Groundwork for Scala version decoupling #1133
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexarchambault
force-pushed
the
groundwork
branch
2 times, most recently
from
352854d to
68bdbe7
Compare
That makes it easier later on to extract a Java interface for CompilerLifecycleManager
Either has less cases to handle, which makes the extraction of a Java interface for CompilerLifecycleManager more straightforward.
Having it rely on Frame from ammonite.repl.api, rather than a concrete Frame implementation, makes it easier to extract an interface for CompilerLifecycleManager later on.
This change doesn't change the semantic of this logic. It makes easier to adapt this method to the upcoming Java-based Preprocessor API.
…and remove Compiler.parse This allows to strip the Compiler public API of references to scala.tools.nsc. Later on, makes it easier to extract a Java interface for Compiler.
This makes it more straightforward to extract a Java API for parsing later on.
Just like the previous commit, this facilitates extracting a Java API for parsing later on.
colors is only useful from the REPL, not from scripts. It makes more sense that it lives in ReplAPI.
util will be a shared dependency of runtime / interp on the one hand, and the upcoming compiler module on the other. Moving Classpath here allows it to be used by both.
The clipboard stuff can live entirely in user space, there's no need to have its calls cross the userspace <-> Ammonite internals barrier. Later on, this is helpful to Java-ize ReplAPI.
That makes it easier to Java-ize InterAPI. It should be do-able to add back os-lib stuff later on.
util should need to be cross-compiled for all supported Scala versions, including the upcoming Scala 3. Dropping the ops dependency allows not to have to cross-compile ops.
It seems it's unused.
Just like for util <-> ops, repl-api is meant to be cross-compiled to all supported Scala versions. Removing the dependency towards ops allows not to cross-compile ops.
That makes it more straightforward to Java-ize InterpAPI later on.
That makes it easier to Java-ize ReplAPI later on.
When creating a compiler instance, Ammonite passes it two class paths: the "initial" one, and the main one. The former is subject to whitelisting when --thin is passed, not the latter. Later on, when adding --scala-version, there can be JARs used internally by Ammonite that are in both paths. Their classes need to be blacklisted, but these JARs should still be loaded via the main class path, as any other JAR added by users. Before this commit, we were naively removing from the main class path any JAR already in the initial class path, which was a problem for the JARs in both paths mentioned above.
ForkClassLoader injects resources from another ClassLoader. When --thin is passed, it could have injected resources that should have been blacklisted. Alternatively, maybe we could have adde a whitelisting layer on top of it too…
A few things are not whitelisted anymore after removing the ops dependency here or there. Whitelisting in these tests will be added back when decoupling the Ammonite and user-facing Scala versions.
alexarchambault
force-pushed
the
groundwork
branch
from
68bdbe7 to
68c4fb7
Compare
alexarchambault
force-pushed
the
groundwork
branch
from
692dc75 to
5e81a3f
Compare
|
Closing this. Going to open new PRs with less changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR prepares the ground for Scala version decoupling.
It mainly
--thinrelated things, that helps for the upcoming--scala …optionAbout the second last point, I've been realizing that os-lib and ops module stuff may be added back later on, in the upcoming PR decoupling Scala versions. I'm finishing cleaning up the PR decoupling Scala versions, and that may allow to drop some of these os-lib and ops module related changes from here. I'm opening this PR to check what the CI says and for you to see, but in any case, let's wait until the upcoming Scala version decoupling PR is opened to merge this.