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

Add support for SIP-51 (unfreezing the Scala library) #7480

Merged
merged 9 commits into from
Apr 8, 2024

Conversation

lrytz
Copy link
Contributor

@lrytz lrytz commented Jan 23, 2024

This implements the changes needed for SIP-51, to drop forwards binary compatibility for the Scala library.

Upstream PRs have been merged and released (sbt/zinc#1331 and coursier/sbt-coursier#490).

See the individual commit messages for details.

Summary, for Scala 2.13:

  • The Scala library on the classpath is no longer pinned to scalaVersion, but resolved like any other library
  • The scalaVersion cannot be older than the Scala library on the classpath, otherwise fail the build
  • csrSameVersions is used to ensure all Scala artifacts (scala-library, scala-reflect, scala-compiler, scalap) are kept at the same version. This is necessary because they are compiled with the inliner enabled, mixing versions is not binary compatible.

For Scala 3

  • The scala-library.jar is already resolved like other libraries in current sbt, but only on the dependency classpath, not on the runtime classpath used when executing the compiler
  • This can lead to NoSuchMethodExceptions when expanding macros
  • This PR changes this by replacing the scala-library in the scala-tool configuration by the version in the compile configuration

@lrytz
Copy link
Contributor Author

lrytz commented Feb 22, 2024

The zinc and sbt-coursier PRs are merged. Once there are releases of these two libraries, this PR can be updated to pass CI. @eed3si9n / @alexarchambault could you take a look, are any releases planned?

@eed3si9n
Copy link
Member

Yea, I can kick off releases for both soon.

@eed3si9n
Copy link
Member

No longer pass `-bootclasspath /path/to/scala-library.jar` to the Scala
compiler, put the library on the ordinary classpath.
There are a couple of settings / configs that affect this, summary
below. The change in this PR seems to be the most narrow.

`scalaModuleInfo.value.overrideScalaVersion` in sbt
  - affects how sbt to sets coursier's `forceScalaVersion` (see below)
  - used by librarymanagement.ivy. If true, add a OverrideScalaMediator
    See sbt#2634. Probably not relevant when using coursier.

`autoScalaLibrary` setting in sbt
  - automatically add `scala-library` (or `scala3-library`) as a project
    dependency
  - also used for `forceScalaVersion` (see below)

`CoursierConfiguration.autoScalaLibrary`
  - if `true` then Coursier `ResolutionParams.forceScalaVersion` is set
    to to `true`
  - initialized by sbt to
    `autoScalaLibrary.value &&
     !ScalaArtifacts.isScala3(sv) &&
     !Classpaths.isScala213(sv) &&        // added in this commit
     scalaModuleInfo.forall(_.overrideScalaVersion)`

coursier `ResolutionParams.forceScalaVersion`
  - if true, `scala-library` / `scala-reflect` / `scala-compiler` /
    `scalap` are forced to the scala version, not actually resolved
  - for Scala 3, the `scala3-library` and `scala3-compiler` versions
    are forced
When expanding a macro compiled against a new Scala library, the
runtime classpath of the compiler should not contain an older library.
Otherwise a NoSuchMethodException can occur.

A similar issue is present when running the Scala repl through sbt.
An input line compiled against a new library could fail to run if
the repl's runtime classpath is on an old version.
The `csrSameVersions` setting can be used to keep dependencies at the
same version.

By default it's used for Scala artifacts. They need to be kept at the
same version because the compiler / reflect are built with
cross-artifact inlining enabled.

`csrSameVersions := Seq(Set(scala-library, scala-reflect, scala-compiler, scalap))`

Users can make use of the new setting in the following way:
  - `csrSameVersions += Set[InclExclRule]("com.corp" % "lib", "com.corp" % "lub")`
  - `csrSameVersions += Set[InclExclRule]("com.corp" % "lib-family-*")`
When a macro was compiled against a new scala-library (say 2.13.15),
the runtime classpath of the Scala compiler should not contain an older
scala-library. Otherwise the macro can cause a NoSuchMethodException
during expansion.

This commit updates scala-library in the scalaInstance to the version
on the projects dependency classpath.
dependency-graph/toFileSubTask failed for me locally
Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Let's land this.

@eed3si9n eed3si9n merged commit ec02bf3 into sbt:1.10.x Apr 8, 2024
10 checks passed
@eed3si9n eed3si9n added this to the 1.10.0 milestone Apr 8, 2024
@tgodzik
Copy link

tgodzik commented Jun 12, 2024

"The scalaVersion cannot be older than the Scala library on the classpath, otherwise fail the build"

How do we cross publish for different Scala versions now actually? Is it even sensibly possible? We have an issue where we publish semanticdb plugin for older versions, but since we use scalapb which uses a newer Scala version then we are no longer able to build for a version older than the scalapb's scala version.

Is the only option to have a compatibility map for every dependency?

@lefou
Copy link

lefou commented Jun 12, 2024

Not a real answer, but I think one root issue in Scala/JVM land is the missing distinction between compile-time and runtime-dependencies. Libraries should always compile against the lowest supported version, but run with the highest (if possible, to address improvements and fixes). There should be no random version bumps in compile-dependencies of libraries, since this essentially drops support for all versions below that version. But this requires a mind shift on the library user side, since downstream users need to watch and bump transitive dependencies themselves. Since almost no library I can find follows this semantics, library authors that also depends on other libraries will have a hard time to provide a wide range of Scala versions.

@lrytz
Copy link
Contributor Author

lrytz commented Jun 12, 2024

How do we cross publish for different Scala versions now actually?

This is relevant only for compiler plugins, right?

But maybe there's actually nothing to do here. If the new semanticdb depends on a scalapb which itself depends on 2.13.14, then releasing semanticdb for 2.13.13 is pointless. The same scalapb dependency will be pulled in for users and require them to also upgrade to 2.13.14.

@tgodzik
Copy link

tgodzik commented Jun 12, 2024

How do we cross publish for different Scala versions now actually?

This is relevant only for compiler plugins, right?

But maybe there's actually nothing to do here. If the new semanticdb depends on a scalapb which itself depends on 2.13.14, then releasing semanticdb for 2.13.13 is pointless. The same scalapb dependency will be pulled in for users and require them to also upgrade to 2.13.14.

I guess so, but then we can't stop publishing for more than one latest version, since that might be too strict :/ Also we might need to backpublish support for some version from time to time. It just got harder to do so :/

@tgodzik
Copy link

tgodzik commented Jun 12, 2024

Looks like I managed to make it work scalameta/scalameta#3722

bjaglin added a commit to scalacenter/scalafix that referenced this pull request Jun 23, 2024
bjaglin added a commit to bjaglin/scalafix that referenced this pull request Jul 29, 2024
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.

None yet

4 participants