search

com-lihaoyi / Ammonite

Scala Scripting
http://ammonite.io
MIT License
2.6k stars 367 forks source link

Remove interpreter dependency on org.scalameta:trees #1177

Open olafurpg opened 3 years ago

olafurpg commented 3 years ago

Currently, the Ammonite interpreter module has a dependency the Scalameta module trees https://github.com/com-lihaoyi/Ammonite/blob/0f0d597f04e62e86cbf76d3bd16deb6965331470/build.sc#L296

This dependency is only used to access internal APIs to read/write protobuf files

https://github.com/com-lihaoyi/Ammonite/blob/0f0d597f04e62e86cbf76d3bd16deb6965331470/amm/interp/src/main/scala/ammonite/interp/script/SemanticdbProcessor.scala#L8

Those APIs are intentionally under scala.meta.internal because they may introduce source+binary breaking changes on every release. The ideal way to consume those classes is to generate your own code from the semanticdb.proto file here https://github.com/scalameta/scalameta/blob/5c308e6452705542a4799a82c627e73c55a0f8a7/semanticdb/semanticdb/semanticdb.proto

This dependency makes it impossible to use newer versions of Scalameta trees with the Ammonite REPL. See https://github.com/scalameta/scalameta/issues/2294 The problem is that Scalameta releases are not forwards binary compatible, they're only backwards compatible with each other. Forwards compatibility means you can't add any new method to the API without breaking the major release.

cc/ @alexarchambault

olafurpg commented 3 years ago

An alternative middleground is that we publish a new module org.scalameta:semanticdb that includes the auto-generated classes that Ammonite can depend on. The generated classes will remain internal and discouraged for public usage, but it would still fix this issue because Ammonite doesn't need the rest of Scalameta (parsers, trees, ...) to work.

olafurpg commented 3 years ago

I opened https://github.com/scalameta/scalameta/pull/2299 moving the auto-generated ScalaPB classes to the module org.scalameta:common that doesn't bring in Scalameta trees/parsers.

alexarchambault commented 3 years ago

@olafurpg Ammonite can be run with the --thin option, hiding many of its internal dependencies:

$ amm
Loading...
Welcome to the Ammonite Repl 2.3.8-65-0f0d597f (Scala 2.13.5 Java 11.0.7)
@ import scala.meta._
import scala.meta._

@ exit

$ amm --thin
Loading...
Welcome to the Ammonite Repl 2.3.8-65-0f0d597f (Scala 2.13.5 Java 11.0.7)
@ import scala.meta._
cmd0.sc:1: object meta is not a member of package scala
did you mean math?
import scala.meta._
             ^
Compilation Failed

@

I believe there shouldn't be conflicts this way.

alexarchambault commented 3 years ago

When run with --thin, Ammonite only exposes the class path of its compiler and repl-api modules:

$ cs resolve com.lihaoyi:ammonite-repl-api_2.13.5:2.3.8-65-0f0d597f com.lihaoyi:ammonite-compiler_2.13.5:2.3.8-65-0f0d597f
com.github.javaparser:javaparser-core:3.2.5:default
com.lihaoyi:ammonite-compiler-interface_2.13.5:2.3.8-65-0f0d597f:default
com.lihaoyi:ammonite-compiler_2.13.5:2.3.8-65-0f0d597f:default
com.lihaoyi:ammonite-interp-api_2.13.5:2.3.8-65-0f0d597f:default
com.lihaoyi:ammonite-ops_2.13:2.3.8-65-0f0d597f:default
com.lihaoyi:ammonite-repl-api_2.13.5:2.3.8-65-0f0d597f:default
com.lihaoyi:ammonite-util_2.13:2.3.8-65-0f0d597f:default
com.lihaoyi:fansi_2.13:0.2.9:default
com.lihaoyi:fastparse_2.13:2.3.0:default
com.lihaoyi:geny_2.13:0.6.2:default
com.lihaoyi:mainargs_2.13:0.2.0:default
com.lihaoyi:os-lib_2.13:0.7.1:default
com.lihaoyi:pprint_2.13:0.6.0:default
com.lihaoyi:scalaparse_2.13:2.3.0:default
com.lihaoyi:sourcecode_2.13:0.2.1:default
io.get-coursier:interface:0.0.21:default
net.java.dev.jna:jna:5.3.1:default
org.javassist:javassist:3.21.0-GA:default
org.jline:jline:3.19.0:default
org.scala-lang:scala-compiler:2.13.5:default
org.scala-lang:scala-library:2.13.5:default
org.scala-lang:scala-reflect:2.13.5:default
org.scala-lang.modules:scala-collection-compat_2.13:2.4.0:default
org.scala-lang.modules:scala-xml_2.13:2.0.0-M3:default
kpbochenek commented 3 years ago

@alexarchambault just curious, what is a reason for not having it as a default thing(--thin)? If something is not needed why to include it by default?

alexarchambault commented 3 years ago

--thin was added https://github.com/com-lihaoyi/Ammonite/pull/941, and we settled on making it opt-in at the time (cc @lihaoyi).

I'd have a slight preference for it to be opt-out too. It leads to less surprising things down the road, such as this issue. Maybe it's time to change that?

alexarchambault commented 3 years ago

Or we could go for a middle ground somehow: hide Ammonite's internals by default (like --thin does), but keep exposing most com.lihaoyi::* libraries.

tgodzik commented 3 years ago

Or we could go for a middle ground somehow: hide Ammonite's internals by default (like --thin does), but keep exposing most com.lihaoyi::* libraries.

That would probably be sensible. Those libraries are pretty nice to have without ivy imports, but things like scalameta doesn't really need to be alongside them.

olafurpg commented 3 years ago

Sounds good. We have merged the PR that moved the SemanticDB classes to the org.scalameta:common module. Once the next release is out, we'll send a PR updating the Ammonite depencency to use that instead of the trees module to avoid the need for the --thin option. Thank you @alexarchambault !

benwaffle commented 2 years ago

Hey, are we able to hide scalameta from ammonite scripts by default now?