Skip to content

Upgrade to sbt 1.3.6, release sbt-dotty 0.4.0 #7953

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 11 commits into from
Jan 16, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Jan 9, 2020

No description provided.

@smarter smarter force-pushed the upgrade/sbt-1.3.6 branch 2 times, most recently from 0c03eb0 to 8f5f4d3 Compare January 10, 2020 15:26
@smarter smarter marked this pull request as ready for review January 10, 2020 15:38
@smarter smarter requested a review from nicolasstucki January 10, 2020 15:38
@nicolasstucki
Copy link
Contributor

@smarter sjsJUnitTests is failing

@gzoller
Copy link
Contributor

gzoller commented Jan 10, 2020

Hello! The classpath discovery fix needs to also go in dotty.tools.dotc.consumetasty.ConsumeTasty. The original code is the same there as in QuoteDriver.

@smarter smarter force-pushed the upgrade/sbt-1.3.6 branch 3 times, most recently from 604c9ce to 1cc2379 Compare January 14, 2020 13:48
@smarter

This comment has been minimized.

@smarter

This comment has been minimized.

@smarter smarter requested a review from nicolasstucki January 15, 2020 16:05
@smarter smarter assigned nicolasstucki and unassigned smarter Jan 15, 2020
Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

For the same reason that it was removed from sbt:
sbt/sbt@ffa69ea
dotty-library-bootstrappedJS scalacOptions handling was broken, I fixed
it by upgrading the reference compiler to simplify the handling of
-Yerased-terms, but I also removed all scalajs projects from IDE startup
as explained in the comment.
smarter and others added 6 commits January 16, 2020 09:46
tasty-core is a dotty project, so when bootstrapping it needs to be
recompiled and cannot be reused as is, the test did not fail so far
since we didn't break anything, but it could be made to break by bumping
the tasty version in TastyFormat.

Also use change the names of the output directories of lib, dotty1 and
dotty2 to match the group name.
Switching to sbt 1.3 allows us to pass both the scala-library and
dotty-library jars as standard library jars to `ScalaInstance` whereas
before we could only pass one of them which could have weird
consequences.
This fixes the sbt-dotty/quoted-example-project scripted test after the
previous commit.

Getting a classpath from a classloader is impossible in general, so we
really shouldn't be relying on it, but I fixed the thing we currently
use to at least work with sbt >= 1.3, sbt now uses multiple layers of
classloaders so we have to recurse on the parent of the classloader to
find all the URLs.

I also made it more correct by not appending
System.getProperty("java.class.path") to the classpath, this is
incorrect in general and can lead to classpath pollution (e.g., when
launched from sbt where java.class.path will contain the jars used by
sbt itself, which might include a different version of scala-library
than the one we use).
The tests ensure that we've fixed scala#7897.

Co-Authored-By: Guillaume Martres <[email protected]>
- Move build-no-fork.sbt in a subdirectory, otherwise it would be loaded
  by sbt.
- Call reload after changing build.sbt, otherwise nothing would actually
  change.
- Remove build.properties, no longer needed since we now use sbt 1.3.6
  by default.
This was broken after I stopped appending
`System.getProperty("java.class.path")` to the constructed classpath.
Fixed with a hack, but this is still better than looking at
java.class.path.
@nicolasstucki nicolasstucki merged commit 0af8c1e into scala:master Jan 16, 2020
@nicolasstucki nicolasstucki deleted the upgrade/sbt-1.3.6 branch January 16, 2020 10:50
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