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

Unexpected inject of scala3-library by the TypelevelKernelPlugin #654

Closed
WojciechMazur opened this issue Oct 24, 2023 · 9 comments · Fixed by #655
Closed

Unexpected inject of scala3-library by the TypelevelKernelPlugin #654

WojciechMazur opened this issue Oct 24, 2023 · 9 comments · Fixed by #655
Labels

Comments

@WojciechMazur
Copy link

In the Open Community Build for typelevel/spotted-leopards I've seen a strange failures when building docs under 3.4.0-NIGHTLY. Build logs.
One of the reasons of failures were duplicated entries of scala3-library. The error message was as following

  object caps does not have a member type Cap while compiling ...

where caps.Cap is the experimental part of Caprese project (Capture Calculus) https://github.com/lampepfl/dotty/blob/15033c7ac8be33a1b94226da3cca3922b53066ce/library/src/scala/caps.scala#L7

Why this symbol was required in building docs is out of the scope of this issue. What's important is the fact the for some reason scala3-library:3.3.0 was present on the classpath when building docs insteead of expected nightly version of stdlib. After poking around I've found the following injected externalDependecies

show coreJVM/unmanagedClasspath
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/typelevel/scalac-compat-annotation_3/0.1.2/scalac-compat-annotation_3-0.1.2.jar)
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/3.3.0/scala3-library_3-3.3.0.jar)
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar)

Inspection of unmanaged classpath pointed me to this project:

inspect coreJVM/unmanagedClasspath
...
[info]  ProjectRef(uri("file:/workspace/spotted-leopards/"), "coreJVM") / Compile / unmanagedClasspath
[info] Defined at:
[info]  (sbt.Classpaths.classpaths) Defaults.scala:2709
[info]  (org.typelevel.sbt.TypelevelKernelPlugin.projectSettings) TypelevelKernelPlugin.scala:62

The scala3-library:3.3.0 seems to be injected in here in TypeLevelKernelPlugin

Compile / unmanagedClasspath ++= update.value.select(configurationFilter(CompileTime.name))

I don't know the purpose of this inject and if it really required. I only want to spot some light on it.
When it comes to failures in the scaladoc I think there might some some bug, because during normal compilation duplicate stdlib entries seems not to be the problems, altough I would try to reproduce it and report to the dotty repo.

@armanbilge
Copy link
Member

Thanks, sorry for the trouble.

We introduced that in #415, which was cargo-culted from typelevel/cats#1889 via sbt/sbt#2503.

Maybe there is a better way to do this. I guess the problem is that eviction is not working properly when we manually mess with the classpath like this.

@WojciechMazur
Copy link
Author

Also one note - it should rather be kept as a low priority. We can add custom sbt options to projects in OpenCB to filter out problematic config entries on our side.

@armanbilge armanbilge added the bug label Oct 24, 2023
@armanbilge
Copy link
Member

armanbilge commented Oct 24, 2023

Thanks, but actually I think that this is a rather high-priority issue. It's not just affecting the community build, it's affecting real builds. For example this is the Cats build:

sbt:root> show kernelJVM/dependencyClasspath
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/typelevel/scalac-compat-annotation_3/0.1.2/scalac-compat-annotation_3-0.1.2.jar)
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/3.3.0/scala3-library_3-3.3.0.jar)
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar)
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/3.3.1/scala3-library_3-3.3.1.jar)
[info] * Attributed(/home/gitpod/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar)

We haven't noticed the problem so far because Scala Library 3.3.1 is forwards- and backwards-compatible with 3.3.0. But it can still be highly confusing to users because they may be non-deterministically using a different version of Scala 3.3.x lib than they intended e.g. an older, bugged version.

The problem becomes even worse when Scala 3.4.0 will be released because that will not be forward-compatible with 3.3.x. The community build is foreshadowing real-world user pain.

@satorg
Copy link
Contributor

satorg commented Oct 25, 2023

Hmm.. It was definitely not anticipated. Do I understand correctly that even though scalac-compat-annotation is still compile-only dependency, but it brings in another version of scala3-library at compile time?

@armanbilge
Copy link
Member

armanbilge commented Oct 25, 2023

Yes, the problem with this code is that we are mindlessly adding scalac-compat-annotation and all of its dependencies (i.e. the scala library, either 2 and/or 3) directly to the classpath. Typically dependencies are first pre-processed through the eviction mechanism to avoid conflicting entries on the classpath but this bypasses that.

Compile / unmanagedClasspath ++= update.value.select(configurationFilter(CompileTime.name))


In light of typelevel/scalac-compat#56 I'm starting to wonder whether it's worth all of this effort to have a "compile-only" dependency. The tooling just doesn't support it well ...

@satorg
Copy link
Contributor

satorg commented Oct 26, 2023

@armanbilge , it's just appeared to me that we could probably ditch the CompileTime config in favor of the standard Provided one. In other words we could simply start adding scalac-compat-annotation like this:

"org.typelevel" %% "scalac-compat-annotation" % "0.1.2" % Provided

I checked it briefly on my local and it seems working in a similar way – allows a downstream using the annotations at compile time but does not push the library further. Neither it pushes any garbage to the downstream classpath – i.e. the eviction is working out of the box.

Wdyt? If you're fine with such a change, I can arrange a PR for that.

@satorg
Copy link
Contributor

satorg commented Oct 26, 2023

In other words I suppose this particular issue can be resolved.
I am not sure about other tooling though, but at least some relief we can get.

@armanbilge
Copy link
Member

armanbilge commented Oct 26, 2023

Yes, in this SO answer Seb suggests using the Provided scope for this sort of thing. I'm not sure why nobody suggested it in sbt/sbt#2503. Thanks for manually checking it! That seems like a reasonable fix.

Should we deprecate CompileTime? Unfortunately it seems to be fundamentally broken due to the eviction issues.

@satorg
Copy link
Contributor

satorg commented Oct 26, 2023

@armanbilge fyi: #655, thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants