This commit has two goals:
* Simplify the `load` API endpoints, removing the unused ones to shorten
the surface of the API.
* Add documentation to the main `load` methods.
Sbt has a feature to show timed logs for every operation at startup.
However, its output is cluttered and users cannot read how much time
single methods consume nor if they call other methods.
This commit improves the status quo by adding indentation.
This commit reduces the complexity around `loadPluginDefinition` et al.
`pluginDefinitionLoader` is not used anywhere in sbt, so the extra
definitions are removed.
Both the implementation of `loadPluginDefinition` and
`pluginDefinitionLoader` are reduced to a bare minimum where the
components at hand (definition classpath, dependency classpath) are
properly defined.
Documentation to the three methods has been added.
It mainly does three things:
* Clean up the implementation, removing unused values like
`globalPluginDefs`.
* Make the implementation more readable.
* And the big one: Remove the creation of a classloader that we were
instantiating but whose value we were throwing away. This has an impact
in performance, but I'm yet to benchmark how much it is.
This commit fixes most of the red squiggles created by the lack of
analysis of Contraband-generated Scala files. It does not end with all
the red squiggles but it substantially decreases the amount of them.
Previous commit used `synchronized` to ensure that the global reporter
was not reporting errors from other parsing sessions. Theoretically,
though, sbt could invoke parsing in parallel, so it's better to ensure
we remove the `synchronized` block, which could also be preventing some
JVM optimizations.
The following commit solves the issue by introducing a reporter id.
A reporter id is a unique identifier that is mapped to a reporter. Every
parsing session gets its own identifier, which then is reused for
recursive parsing. Error reports between recursive parses cannot collide
because the reporter is cleaned in `parse`.
The previous implementation was instantiating a toolbox to parse every
time it parsed a sbt file (and even recursively!!!).
This is inefficient and translates to instantiating a `ReflectGlobal`
every time we want to parse something.
This commit takes another approach:
1. It removes the dependency on `ReflectGlobal`.
2. It reuses the same `Global` and `Run` instances for parsing.
This is an efficient as it can get without doing a whole overhaul of it.
I think that in the future we may want to reimplement it to avoid the
recursive parsing to work around Scalac's bug.
This change was proposed by Jason in case that the new parsing mechanism
implemented later on has to be reverted. This change provides a good
baseline, but it's far from ideal with regard to readability of the
parser and performance.
The previous implementation was using the Scala runtime universe to
check whether a plugin had or not an `autoImport` member. This is a bad
idea for the following reasons:
* The first time you use it, you class load the whole Scalac compiler
universe. Not efficient. Measurements say this is about a second.
* There is a small overhead of going through the reflection API.
There exists a better approach that consists in checking if `autoImport`
exists with pure Java reflection. Since the class is already class
loaded, we check for:
* A class file named after the plugin FQN that includes `autoImport$` at
the end, which means that an object named `autoImport` exists.
* A field in the plugin class that is named `autoImport`.
This complies with the plugin sbt specification:
http://www.scala-sbt.org/1.0/docs/Plugins.html#Controlling+the+import+with+autoImport
If the POM is in cache, but not the directory listing (nor a .error file for it), the LocalUpdate and LocalUpdateChanging policies make MavenRepository do as if the directory listing is missing - it is not even checked with the subsequent cache policies if any (because the POM was found with LocalUpdate / LocalUpdateChanging in the first place).
This PR fixes that - getting the POM will fail if the directory listing or an error file for it is missing from cache. That way, subsequent cache policies can fetch the directory listing.
As this only happens depending on what's in cache, it's a bit cumbersome to test as is. Relying fully on NIO2 in subsequent coursier versions should make it easier to add test cases for that (by using a virtual fs like jimfs to test what happens depending on what's in cache).
The test should basically do:
$ coursier fetch org.apache.maven:apache-maven:3.3.9 # fill cache
$ rm -f $CACHE_PATH/https/repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/.directory
$ rm -f $CACHE_PATH/https/repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/..directory.checked
$ coursier fetch org.apache.maven:apache-maven:3.3.9
The second fetch should work fine, fetching the directory listing, seeing that it lists no JAR, and then not attempting to download one. Before this commit, the second attempt would assume that the directory listing is not available, do without it, so assume that a JAR exists, and fail to download it.
I'm now going to use `CacheStore.apply` in `JsonUtil` (used by cached resolution). This gets rid of `fileToStore` parameter from a bunch of classes and simplifies the setup around librarymanagement.
sbt/util#45 implemented caching using sjson-new. Now many of the functions take `CacheStore` that abstracts the caching ability.
sbt/sbt#3109 demonstrates that setting up CacheStore requires boilerplate involving concepts introduced in sbt 1.
This change adds back overrides using File by making assumption that majority of the time we would want standard JSON converter.
We need to communicate the error states in the thread, so I added a `Future[Unit]` called `ready`.
If something goes wrong during the startup, like if the port is already taken, this can be used to communicate back to the main thread, and display the error accordingly.
Eugene Yokota
jvican
Dale Wijnand
Alexandre Archambault
Alexandre Archambault