Skip to content

sbt bootstrap Part 2: Revenge of the sbt bootstrap #2312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
May 1, 2017

Conversation

smarter
Copy link
Member

@smarter smarter commented Apr 26, 2017

This is a major overhaul of how the sbt bootstrap works, the main goal is to be able to publish a usable bootstrapped compiler, this mean having artefacts with names like dotty-*_0.1 instead of dotty-*-bootstrapped_2.11, this is important to be able to cross-compile with dotty like any other Scala version (I will rewrite sbt-dotty to take advantage of this soon. The other important change in this PR is that the non-bootstrapped compiler no longer needs to be published to compile the bootstrapped compiler, only dotty-sbt-bridge needs to be published (I haven't investigated whether this is something we could avoid too). It is still possible to bootstrap from published artefacts using the bootstrapFromPublishedJars setting I added, see the commit messages.

@smarter smarter force-pushed the publish-bootstrap branch 2 times, most recently from a3e0d36 to 0bed0bd Compare April 26, 2017 17:35
@smarter smarter requested review from sjrd and felixmulder April 26, 2017 17:55
Copy link
Contributor

@felixmulder felixmulder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

I have some reservations with merging this however. If I've understood correctly this will make the distributed version of Dotty bootstrapped as well.

Has the infinite loop issue been resolved for the REPL in this case? If not, I would prefer if we fix that first - otherwise people who try out Dotty will be met with a REPL that does not work. Not very good...

@smarter
Copy link
Member Author

smarter commented Apr 27, 2017

Has the infinite loop issue been resolved for the REPL in this case? If not, I would prefer if we fix that first - otherwise people who try out Dotty will be met with a REPL that does not work. Not very good...

I've asked @OlivierBlanvillain to look at it ;).

@smarter smarter force-pushed the publish-bootstrap branch from 0bed0bd to a2f3b9e Compare April 27, 2017 15:56
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice overall. Just a couple minor details.

build.sbt Outdated
@@ -16,5 +16,3 @@ val `scala-library` = Build.`scala-library`
val `scala-compiler` = Build.`scala-compiler`
val `scala-reflect` = Build.`scala-reflect`
val scalap = Build.scalap

inThisBuild(Build.thisBuildSettings)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be in the wrong commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, will fix

settings(commonBootstrappedSettings).
settings(
publishArtifact := false
moduleName := "dotty"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can automate this by having the following in commonBootstrappedSettings:

moduleName ~= { _.stripSuffix("-bootstrapped") }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

import dotty.tools.dotc._
import core.Contexts._

class TestDriver extends Driver {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need to make this public? Can't in private inside the companion object of ScalaCompilerForUnitTesting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but it's a test so it doesn't matter much, and I'd like to avoid changing ScalaCompilerForUnitTesting too much since it's adapted from sbt and I synchronize it with the sbt version sometimes.

@smarter
Copy link
Member Author

smarter commented Apr 30, 2017

I've opened #2335 to track the bootstrapped REPL issue.

smarter added 17 commits May 1, 2017 17:38
…ettings

No need to have multiple ways to define settings common to all projects,
and thisBuildSettings doesn't work for keys which are defined in every
project but not globally.
By default, sbt will try to rewrite dependencies on
scala-{library,compiler,reflect} and scalap artifacts.
This can have very weird consequences for us since we define our own
versions of these projects.
Not having publishSettings set does not prevent publishing, we should
set publishArtifact := false on such projects instead.
It is automatically set correctly for dotty by sbt 0.13.15
The regular dotty-sbt-bridge depends on `dotty-compiler` but we'd like
to be able to publish only bootstrapped artifacts without depending on
any non-bootstrapped artifacts.
There are four kinds of projects in our build:
- Projects compiled only with Scala 2 (dotty-bot, as well as
  dotty-interfaces since it's Java-only)
- Projects that are part of dotty and compiled with Scala 2
- Projects that are part of dotty and compiled with a non-bootstrapped
  dotty
- Dummy projects for scala{-library,-compiler,-reflect,p}, see the
  comment above `commonDummySettings`

They are now distinguished using four sets of settings. The first and
second set are identical but this will change with the next commit.
This make them easier to distinguish and harder to accidentally use.
This also means that in the future we could compile dotty with
`crossPath := false`, but this will require changing the hardcoded dotty
dependencies in sbt.
- The bootstrapped dotty artifacts are now named dotty-*_0.1 instead of
dotty-*-bootstrapped_2.11, which means they can be used in sbt projects.

- The dummy scala{-library,-reflect,-compiler,p} aritfacts are now made
  to work with the bootstrapped dotty.

- Compiling the bootstrapped dotty no longer requires publishing the
  non-bootstrapped dotty, only dotty-sbt-bridge needs to be published,
  so you can do:
  ;dotty-sbt-bridge/publishLocal;dotty-bootstrapped/test
Since the previous commit, we bootstrap dotty from the current build
output of the non-bootstrapped compiler, but when hacking on the
compiler it might be useful to bootstrap from a stable artefact, this is
now possible by setting `bootstrapFromPublishedJars` to true.

Usage:
\# Publish the non-bootstrapped compiler
> dotty/publishLocal
\# Enable the option
> set bootstrapFromPublishedJars in ThisBuild := true
\# Bootstrap the compiler
> ;dotty-bootstrapped/clean;dotty-bootstrapped/compile
Otherwise the test won't compile with dotty.
Dotty crashes when trying to compile them currently.
Also move them to their own project `dotty-sbt-scripted-tests`, because
they need to be in a Scala 2.10 project to work correctly, since sbt
uses 2.10.

The scripted tests can now be run with:
> dotty-sbt-scripted-tests/scripted

A few of them are broken right now, I'll fix them later (we don't run
them on the CI yet).
This is needed so that dotty-bootstrapped/publishLocal publishes
everything needed to use dotty.
These `publishLocal` calls should be run _before_ `scriptedTask`, but
before this commit they were run in parallel, leading to horrible failures.
Before this commit, `dotty-sbt-bridge` had to be published locally to be
able to compile bootstrapped projects. This is no longer the case,
instead the local `dotty-sbt-bridge` is used, this also requires calling
`cleanSbtBridge` before packaging `dotty-sbt-bridge` instead of before
publishing it.
@smarter smarter force-pushed the publish-bootstrap branch from d9157b6 to de5d34a Compare May 1, 2017 18:12
@smarter
Copy link
Member Author

smarter commented May 1, 2017

I managed to get the bootstrapped dotty compilable without publishing anything! See last commit. I'll merge this soon now that the REPL is fixed because this is a blocker for other things, but I'll still take into account further comments of course.

@smarter smarter merged commit b19d1fb into scala:master May 1, 2017
@smarter
Copy link
Member Author

smarter commented May 1, 2017

@DarkDimius @felixmulder I've updated our Jenkins nightly build to run dotty-bootstrapped/publishSigned and successfully ran it once.

smarter added a commit to scala/scala3-example-project that referenced this pull request May 1, 2017
Dotty is now published with scalaBinaryVersion 0.1, see
scala/scala3#2312
@allanrenucci allanrenucci deleted the publish-bootstrap branch December 14, 2017 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants