com-lihaoyi / Ammonite

Scala Scripting
http://ammonite.io
MIT License
2.61k stars 369 forks source link

Scalaz that isn't 7.2.x breaks #644

Closed thurstonsand closed 3 years ago

thurstonsand commented 7 years ago

It looks like Coursier introduced a dependency on scalaz 7.2.13 that leaks into the user env. This means that anyone trying to use a different version of scalaz can get a compilation error. I encountered this problem while trying to use http4s:

(note http4s 0.15.13 uses scalaz 7.1.x, while http4s 0.15.13a uses scalaz 7.2.x)

$ amm                                                                                                                                                      ○
Picked up _JAVA_OPTIONS: -Xms2048m -Xmx4096m
Loading...
Welcome to the Ammonite Repl 0.9.5
(Scala 2.12.2 Java 1.8.0_102)
If you like Ammonite, please support our development at www.patreon.com/lihaoyi
@ import $ivy.`org.http4s::http4s-core:0.15.13`
import $ivy.$
@ org.http4s.Uri.uri("https://example.com")
cmd1.sc:1: exception during macro expansion:
java.lang.NoSuchMethodError: scalaz.syntax.std.package$option$.ToOptionIdOps(Ljava/lang/Object;)Lscalaz/syntax/std/OptionIdOps;
    at org.http4s.parser.Rfc3986Parser.HierPart(Rfc3986Parser.scala:33)
    at org.http4s.parser.Rfc3986Parser.HierPart$(Rfc3986Parser.scala:32)
    at org.http4s.parser.RequestUriParser.HierPart(RequestUriParser.scala:9)
    at org.http4s.parser.Rfc3986Parser.AbsoluteUri(Rfc3986Parser.scala:21)
    at org.http4s.parser.Rfc3986Parser.AbsoluteUri$(Rfc3986Parser.scala:20)
    at org.http4s.parser.RequestUriParser.AbsoluteUri(RequestUriParser.scala:9)
    at org.http4s.parser.Rfc3986Parser.Uri(Rfc3986Parser.scala:18)
    at org.http4s.parser.Rfc3986Parser.Uri$(Rfc3986Parser.scala:18)
    at org.http4s.parser.RequestUriParser.Uri(RequestUriParser.scala:9)
    at org.http4s.Uri$.$anonfun$fromString$1(Uri.scala:134)
    at org.parboiled2.Parser.runRule$1(Parser.scala:142)
    at org.parboiled2.Parser.phase0_initialRun$1(Parser.scala:150)
    at org.parboiled2.Parser.__run(Parser.scala:203)
    at org.http4s.Uri$.fromString(Uri.scala:134)
    at org.http4s.Uri$Macros.uriLiteral(Uri.scala:122)

val res1 = org.http4s.Uri.uri("https://example.com")
                             ^
Compilation Failed

vs

$ amm                                                                                                                                                      
Picked up _JAVA_OPTIONS: -Xms2048m -Xmx4096m
Loading...
Welcome to the Ammonite Repl 0.9.5
(Scala 2.12.2 Java 1.8.0_102)
If you like Ammonite, please support our development at www.patreon.com/lihaoyi
@ import $ivy.`org.http4s::http4s-core:0.15.13a`
import $ivy.$
@ org.http4s.Uri.uri("https://example.com")
res1: org.http4s.Uri = Uri(Some(https), Some(Authority(None, RegName(example.com), None)), "", Query(), None)
thurstonsand commented 7 years ago

I also confirmed that this does not occur on amm versions before the switch over to coursier

lihaoyi commented 7 years ago

I knew this day would come https://github.com/lihaoyi/Ammonite/pull/604 https://github.com/coursier/coursier/issues/25 https://github.com/coursier/coursier/issues/175

Unfortunately, now we're exposing Coursier as part of the user-facing API, and Coursier itself exposes Scalaz as part of it's user-facing API, though I'm not sure if Scalaz is used in the APIs that Ammonite users would need. Thus I'm a bit reluctant to try and shade Scalaz and friends, as @alexarchambault had offered to do in #604. I think the correct way forward is to work towards removing the Scalaz dependency in Coursier https://github.com/coursier/coursier/issues/25 https://github.com/coursier/coursier/issues/175

alexarchambault commented 7 years ago

As soon as coursier 1.0 final will be out, I'll likely remove the dependency towards scalaz (and not depend on cats either). From the core of coursier for sure, and from its cache module too hopefully, if it's possible to have it rely on std lib futures.

lihaoyi commented 6 years ago

@alexarchambault coursier 1.0 final is out! :D :D :D

anowak commented 6 years ago

I see that Coursier 1.0 still has scalaz as its dependency and this problem occurs with Ammonite 1.1.2.

As Ammonite now uses Mill for building, I don't know if shading is possible (like here: https://github.com/ensime/ensime-sbt/pull/312). I wonder what could be a workaround other than using Ammonite < 0.9.0.

I would generally expect that Ammonite dependencies don't 'leak' into my script execution environment, because then we need to account for all the transitive dependencies, creating a snowball effect.

In other words, it would seem that here, we would like to use SystemClassLoader as the parent:

https://github.com/lihaoyi/Ammonite/blob/27842d90a5fa9c70dfa6d1e79f9f1b6f9ac2cee8/amm/runtime/src/main/scala/ammonite/runtime/ClassLoaders.scala#L77

And then load Ammonite-related classes in a predef. This way, people could solve the class loading conflicts by using a customized predef. Does it make any sense? 😃

alexarchambault commented 6 years ago

Just fyi, the current master of coursier doesn't depend on scalaz anymore. Switching to e.g. coursier 1.1.0-M4 should solve that issue. The only problem is that the API of it may change quite a bit before 1.1.0 final, so subsequent version bumps might not be too straightforward.

anowak commented 6 years ago

Thanks! I've created #830 for this and I can confirm that it solves the original issue.

However, my question about having an isolated or parent-last classloader for the interpreter still holds, as a similar problem can arise for other dependencies.

thurstonsand commented 6 years ago

glad to know people have kept this in the back of their mind all this time

sake92 commented 3 years ago

@alexarchambault close?