Skip to content
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

Manage Scala 3 instances and fetch compiler bridges #6206

Merged
merged 16 commits into from
Dec 24, 2020

Conversation

adpi2
Copy link
Member

@adpi2 adpi2 commented Dec 16, 2020

Part of #6080
Depends on sbt/librarymanagement#350

Manage Scala 3 instances

When Scala version is Scala 3:

  • the scala3-compiler artifact is added to the list of tool dependencies. scala3-compiler transitively depends on scala3-library and scala-library
  • if autoScalaLibrary := true, the scala3-library is added to the list of library dependencies

Coursier does not handle autoScalaLibrary properly with Scala 3. But we don't really need to set autoScalaLibrary = true in the Coursier configuration because sbt already add the proper scala library to the list of dependencies.

Fetch the Scala 3 compiler bridge binaries

Contrary to Scala 2, the Scala 3 compiler bridges are tied to the compiler versions. There is a single compiler bridge for each compiler version. Also they are pure java so binary compatible with all Scala versions.

Consequently, we can fetch the binary module directly without using the ZincCompilerBridgeProvider.

Manage the .tasty files

This PR introduces an auxiliaryClassFiles setting that is passed to Zinc via incOptions.
When the Scala version is Scala 3, the auxiliaryClassFiles setting contains the TastyFiles singleton. Otherwise it is empty.

One can add more auxiliaryClassFiles with, for example, auxiliaryClassFiles += ScalaJSFiles.instance

@adpi2
Copy link
Member Author

adpi2 commented Dec 18, 2020

@eed3si9n How can I get the CI to run against the latest librarymanagement?
(without releasing librarymanagement because I may have some changes do to around IvyDependencyResoltuion about overrideScalaVersion)

@eed3si9n
Copy link
Member

@adpi2 Not possible now, but it shouldn't be too hard to tweak https://github.com/sbt/sbt/blob/develop/.github/workflows/nightly.yml or copy-paste it and adding a workflow_dispatch similar to https://github.com/sbt/sbt-dist/blob/master/.github/workflows/distro.yml

@adpi2 adpi2 force-pushed the sbt-dotty branch 5 times, most recently from d28867d to 5b886cc Compare December 19, 2020 16:48
@eed3si9n
Copy link
Member

@adpi2
Copy link
Member Author

adpi2 commented Dec 21, 2020

@eed3si9n Thanks for publishing librarymanagement. I will also need latest zinc before merging this PR.

I changed the CI of this PR to make it run on fresh io, zinc and librarymanagement 1.5.0-SNAPSHOT, published locally.
But the scripted tests cannot resolve the compiler-bridge 1.5.0-SNAPSHOT because they don't use the user ivy-cache:

  • The compiler-bridge is published to /home/runner/.ivy2/local/org.scala-sbt/compiler-bridge_2.12/1.5.0-SNAPSHOT/poms/compiler-bridge_2.12.pom
  • But the scripted test try to resolve it from:
[info] [warn] 	module not found: org.scala-sbt#compiler-bridge_2.12;1.5.0-SNAPSHOT
[info] [warn] ==== local: tried
[info] [warn]   /tmp/sbt_9611947f/target/ivy-cache/local/org.scala-sbt/compiler-bridge_2.12/1.5.0-SNAPSHOT/ivys/ivy.xml

Do you know how I can force the scripted tests to use the user ivy cache?

@adpi2
Copy link
Member Author

adpi2 commented Dec 21, 2020

In fact, it seems that my ivy cache problem is due to the settings of some particular scripted test and not to scripted tests in general:

For instance in dependency-management/artifact/build.sbt

ivyPaths := IvyPaths(baseDirectory.value, Some(target.value / "ivy-cache")),

overrides the user global cache.

@adpi2
Copy link
Member Author

adpi2 commented Dec 21, 2020

I adapted those tests with

scalaCompilerBridgeResolvers += userLocalFileResolver(appConfiguration.value)

// use the user local resolver to fetch the SNAPSHOT version of the compiler-bridge
def userLocalFileResolver(appConfig: AppConfiguration): Resolver = {
  val ivyHome = appConfig.provider.scalaProvider.launcher.ivyHome
  Resolver.file("User Local", ivyHome / "local")(Resolver.defaultIvyPatterns)
}

and it works!

@adpi2
Copy link
Member Author

adpi2 commented Dec 21, 2020

This is ready for review!

The autoScalaLib = false, in LMCoursier configuration may not be ideal but it doesn't seem to cause much trouble. Hopefully I'll be able to adapt Coursier to Scala 3 artifacts but it does not seem so simple: autoScalaLib is not an lmcoursier only param, it goes deep into Coursier code itself.

@adpi2 adpi2 requested a review from eed3si9n December 21, 2020 15:55
@adpi2 adpi2 marked this pull request as ready for review December 21, 2020 15:56
@adpi2
Copy link
Member Author

adpi2 commented Dec 21, 2020

For now I have just created an issue in the Coursier repository about autoScalaLib and forceScalaVersion: coursier/coursier#1933

@smarter
Copy link
Contributor

smarter commented Dec 22, 2020

Coursier does not handle autoScalaLibrary properly with Scala 3. But we don't really need to set autoScalaLibrary = true in the Coursier configuration because sbt already add the proper scala library to the list of dependencies.

I'm not sure I understand what that means, will the sbt setting autoScalaLibrary := false still work correctly in scala 3 projects? If sbt deals with adding the library itself, then could the parameter passed to coursier always be autoScalaLibrary = false or would that do the wrong thing in Scala 2? In any case it sounds like we should open an issue against coursier /cc @alexarchambault

@smarter
Copy link
Contributor

smarter commented Dec 22, 2020

Oh oops, I read the description but missed the comment where you mentioned coursier/coursier#1933 which I guess is the follow-up.

@adpi2
Copy link
Member Author

adpi2 commented Dec 22, 2020

I'm not sure I understand what that means, will the sbt setting autoScalaLibrary := false still work correctly in scala 3 projects? If sbt deals with adding the library itself, then could the parameter passed to coursier always be autoScalaLibrary = false or would that do the wrong thing in Scala 2?

So yes the autoScalaLibrary := flase will work correctly in Scala 3 and Scala 2 projects. Also autoScalaLibrary := true still works perfectly with Scala 2.

The only thing that is different with autoScalaLibrary := true and Scala 3 is that Coursier will not try to override the Scala version of the dependencies to Scala artifacts. I guess that it is completely harmless in the very large majority of cases.

Also I think that choosing a custom scalaOrganization does not work with Coursier. Is it even supported with Scala 2? I don't think so. But clearly in Scala 3 we don't need that yet, and it works with Ivy.

@smarter
Copy link
Contributor

smarter commented Dec 22, 2020

The only thing that is different with autoScalaLibrary := true and Scala 3 is that Coursier will not try to override the Scala version of the dependencies to Scala artifacts. I guess that it is completely harmless in the very large majority of cases.

It's something we'll have to figure out at some point, there's an issue open on that subject: scala/scala3#10146

@eed3si9n
Copy link
Member

Do we know why things are failing on AppVeyor?

[error] C:\projects\sbt\main\src\main\scala\sbt\Defaults.scala:91:8: object TastyFiles is not a member of package xsbti.compile
[error] import xsbti.compile.TastyFiles
[error]        ^
[error] C:\projects\sbt\main\src\main\scala\sbt\Keys.scala:251:41: not found: type AuxiliaryClassFiles
[error]   val auxiliaryClassFiles = taskKey[Seq[AuxiliaryClassFiles]]("The auxiliary class files that must be managed by Zinc (for instance the TASTy files)")
[error]                                         ^
[error] C:\projects\sbt\main\src\main\scala\sbt\Defaults.scala:889:10: value withAuxiliaryClassFiles is not a member of xsbti.compile.IncOptions
[error] possible cause: maybe a semicolon is missing before `value withAuxiliaryClassFiles'?
[error]         .withAuxiliaryClassFiles(auxiliaryClassFiles.value.toArray)
[error]          ^
[error] three errors found
[error] (mainProj / Compile / compileIncremental) Compilation failed
[error] Total time: 147 s (02:27), completed Dec 23, 2020 10:17:18 AM

@adpi2
Copy link
Member Author

adpi2 commented Dec 23, 2020

@eed3si9n I need a release of zinc that contains sbt/zinc#956

@eed3si9n
Copy link
Member

@adpi2 Let me get on that.

@eed3si9n eed3si9n merged commit 9ca5a46 into sbt:develop Dec 24, 2020
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