-
-
Notifications
You must be signed in to change notification settings - Fork 300
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
Implement SIP-51 (unfreezing the Scala library) #3075
Comments
@lefou are you interested in working on this yourself, or should I try to find someone (maybe me, though I know nothing about mill) to take a look? |
@lrytz Good question. First, I need to make myself familiar with what this change actually means. Is there some ETA for when users expect SIP-51 to be usable? |
It took me a long time to get it done for sbt because I didn't anticipate all the interactions I wrote in this ticket, I was only discovering them along the way. Hopefully there should be no surprises when doing the same for mill and the actual diff should not be too big (it's not in sbt). For timing, I don't know what is the timing for sbt 1.10.0 (which could ship the change). Even when the change is released in the build tools, nothing is going to happen until there is actually some Scala release with a forwards incompatible change in the library, which would be 2.13.15 the earliest (late summer 2024). |
I wonder whether upcoming version of |
So, this is interesting. I took your example project, and overrode some targets defined in import mill._
import mill.scalalib._
object proj extends ScalaModule {
def scalaVersion = "2.13.8"
def ivyDeps = Agg(
ivy"com.softwaremill.sttp.client3::core:3.8.3", // depends on scala-library 2.13.10, ws 1.3.10
ivy"com.softwaremill.sttp.shared::ws:1.2.7"
)
def scalaLibraryIvyDeps = Agg(
// same as super, but without the `.force`
ivy"org.scala-lang:scala-library:${scalaVersion()}"
)
def mapDependencies: Task[coursier.Dependency => coursier.Dependency] = T.task {
// ignore super-setup
d: coursier.Dependency => d
}
} With this setup I can unpin the
The compiler is still using the correct
And here is the dependency tree:
This all looks good to me. But I'm not 100% sure since you also wrote:
Is this sbt specific or should I enforce the use of the So, the interesting part starts, if I try to use a version range to let coursier 2.1.9 resolve either a consistent classpath, or fail. def scalaLibraryIvyDeps = Agg(
- ivy"org.scala-lang:scala-library:${scalaVersion()}"
+ ivy"org.scala-lang:scala-library:[${scalaVersion()}]"
) My expectation is, that it fails, since the resolved version range is Instead, it resolves the following compile classpath:
Instead of a proper resolution or an failure I now see a classpath with the same artifact but two different version. I need to inspect if this comes from Mill or if it's an issue in coursier. It's definitely not correct. |
@alexarchambault Do you see anything obvious? To me, it looks like an issue in coursier. We effectively call coursier API with the following parameters: val rootDeps = List(
Dependency(
module = Module(
organization = Organization(value = "com.softwaremill.sttp.client3"),
name = ModuleName(value = "core_2.13"),
),
version = "3.8.3"
),
Dependency(
module = Module(
organization = Organization(value = "com.softwaremill.sttp.shared"),
name = ModuleName(value = "ws_2.13"),
),
version = "1.2.7"
),
Dependency(
module = Module(
organization = Organization(value = "org.scala-lang"),
name = ModuleName(value = "scala-library"),
),
version = "[2.13.8]",
)
)
val forced = Map()
coursier.Resolution()
.withRootDependencies(rootDeps)
.withForceVersions(forced) |
That's an interesting, I wasn't aware (https://maven.apache.org/pom.html#dependency-version-requirement-specification). We'd have to test it and see what happens. For example in a project that has a dependency on |
Yeah, since version ranges in Maven never worked that well in older Maven versions (it wasn't able to correctly find good version candidates IIRC), most users found other ways to avoid them, and I think using |
I think unpinning the scala-library as you did in your experiment is good, it removes a special case. But then mill should also fail the build if the scala-library on the compiletime classpath is newer than the scala compiler (in the case of Scala 2 only). Here's an example that simulates what could happen.
Expanding the macro in the 2.13.1 compiler will crash. |
Is it
|
The compiler could be newer, though in reality this probably won't happen, as setting scalaVersion X adds a dependency to scala-library:X. |
So, if we need to align the resolved and the used scala versions, my preferred implementation would be to use coursier to detect any mismatch. We can catch and recognize the exception and provide a more actionable error message, possibly also a copy-and-paste-able fix. But for this, we need to find and fix the root cause for the incorrect resolution I posted above. |
cc @lolgab, who has more profound knowledge of our Scala.JS and Scala Native implementations. |
In my simple mental model coursier doesn't know about the compiler's compile-time and run-time classpath. IIUC, the build tool invokes coursier's dependency resolution twice:
Coursier is just answering these two independent requests. Isn't that how it works? |
It's exactly as you said!
This is
This is
Sure. But IIUC, you said we need to align the version of the used compiler (e.g. As you said, we add the In Mill jargon this means effectively: def ivyDeps = Agg(
ivy"org.scala-lang:scala-library:[2.13.8],
ivy"com.softwaremill.sttp.client3::core:3.8.3", // depends on scala-library 2.13.10, ws 1.3.10
ivy"com.softwaremill.sttp.shared::ws:1.2.7"
) In therory, this should force coursier to either resolve to
|
|
I see now, that would be an elegant way to implement it. But if coursier's support for that is (currently?) unreliable, you can do the check in the build tool. I did it in the |
@lefou coursier handles fine the conflicts you're expecting above:
IIRC, Mill uses coursier via its low-level API, not the high-level one ( (The high-level API is meant to avoid users having to think about that kind of thing) |
Edit: I deleted a comment about coursier, since these were off-topic here I already experimentally switched parts of Mills courier API to the new |
Here is the error message sbt is showing when the version mismatches:
|
There is an open PR for sbt to implement SIP-51 (unfreezing the Scala library). The topic was discussed at a recent Scala meeting and will be brought up in the next SIP meeting (Friday Mar 15); everyone seems to be supporting it.
Example:
ws
is upgraded to 1.3.10, butscala-library
remains at 2.13.8.Constraints for Scala 2.13
In the example shown above, sbt willl fail the build and require the user to upgrade
scalaVersion
to 2.13.10. The reason is interactions between the compiler's compile-time and run-time classpath.On a high level, the compiler is invoked as
java -cp RUNTIME_CLASSPATH scala.tool.nsc.Main -cp COMPILETIME_CLASSPATH Source.scala
. Both classpaths contain a scala-library.mill console
), REPL lines compiled against a newer library could fail to execute on the older libraryThe implementation for Scala 2.13 in sbt is therefore
scalaVersion
SameVersion
rule to ensure scala-library, scala-reflect and scala-compiler are all at the same version (https://github.com/coursier/sbt-coursier/pull/490/files)For sbt-specific details see the individual commit messages in https://github.com/sbt/sbt/pull/7480/commits.
Scala 3
For Scala 3, unpinning scala-library and updating it in dependency resolution like other libraries is the right solution. Currently mill pins
scala-library
according to the Scala version:Just as in Scala 2, to support macros and the REPL correctly, the runtime classpath of the Scala 3 compiler needs to have the updated scala-library. A macro on the dependency classpath can be compiled against some new Scala library; expanding it in the compiler with an older Scala library can fail.
Here's a test case for this scenario (it exploits some package private addition that was done in 2.13.9, as an example of what will happen after unfreezing the standard library):
Scala 2.12
Nothing should change for projects using Scala 2.12 or older.
The text was updated successfully, but these errors were encountered: