Prior to this commit, it was difficult to prevent the sbt metabuild
classpath from leaking into the runtime and test classpaths. The biggest
issue is that the test-inferface jar was located in the metabuild
classpath. We tried to prevent leakage using the DualClassLoader, but
this was an ugly solution that did not seem to work reliably. The fix is
to modify the actual sbt metabuild classloader provided by the sbt
launcher.
To do this, I add a new classloader SbtMetaClassLoader that isolates the
test-interface jar from the rest of the classpath. I modify xMain to
create a new AppConfiguration that uses this new classloader and
use reflection to invoke the sbt main method using the new classloader.
Not only do I think that this is a much saner solution than DualLoaders,
I accidentally fixed#4575 with this change.
It isn't possible to share the runtime and test layers correctly with
bgCopyClasspath is used because the runtime classpath uses the
dependencies copied to the boot directory while the test classpath uses
the classes in target and .ivy2. Since this is not the default and users
have to opt in to
ClassLoaderLayeringStrategy.ShareRuntimeDependenciesLayerWithTestDependencies,
I think this is fine.
It is possible with the new layering strategies that tests may fail if a
java package private class is accessed across classloader layers. This
will result in an IllegalAccessError that is hard to debug. With this
commit, I add an error message that will be displayed if run throws an
IllegalAccessError that suggests that the user try the
ScalaInstance layering strategy or the flat layering strategy.
I noticed that sometimes multiple ClassLoaderCache instances were
created in each configuraiton. I believe this was due to the use of
inConfig(...)(...) causing multiple caches to be created. Long term, I'm
not sure that taskRepository and classLoaderCache are the right
solutions so I made classLoaderCache private[sbt] as well.
I have noticed on linux that the file cache updates aren't fast enough
for ExternalHooks. Say you have project b that depends on project a.
With a clean build, if you run b/compile, the file cache may not yet see
the changes to *.class files generated by project a. There are multiple
ways to fix this:
* don't use the file cache for binary products
* use the analysis results to invalidate the cache
* switch over to my hypothetical replacement file system
In the meantime, we should stop spamming users by default.
I wrote this check in a rush and realized that it didn't quite match the
correct glob semantics. The depth parameter is effectively the index of
the array of sorted child directories of the base. That index is
computed with getNameCount - 1, not getNameCount. It is also inclusive,
not exclusive, hence the switch from `<` to `<=`.
This change was motivated by my reviewing the initial change in the
context of the fix to https://github.com/sbt/sbt/issues/4591.
This commit change the default FileTree.Repository to always use a polling file
repository but one that validates the current file system results
against the cache results. On windows, we do not validate the cache
because the cache can cause io contention in scripted tests. The
cache does seem to work ok on my VM, but not on appveyor for whatever
reason. Validating the cache by default was suggested by @smarter in a
comment in https://github.com/sbt/sbt/issues/4543.
This commit reworks the watch start message so that instead of printing
something like:
[info] [watch] 1. Waiting for source changes... (press 'r' to re-run the command, 'x' to exit sbt or 'enter' to return to the shell)
it instead prints something like:
[info] 1. Monitoring source files for updates...
[info] Project: filesJVM
[info] Command: compile
[info] Options:
[info] <enter>: return to the shell
[info] 'r': repeat the current command
[info] 'x': exit sbt
It will also print which path triggered the build.
Prior to this commit, it was necessary to add breadcrumbs for every
input that is used within a dynamic task. In this commit, I rework the
watch setup so that we can track the dynamic inputs that are used. To
simplify the discussion, I'm going to ignore aggregation and
multi-commands, but they are both supported. To implement this change, I
update the GlobLister.all method to take a second implicit argument:
DynamicInputs. This is effectively a mutable Set of Globs that is
updated every time a task looks up files from a glob. The repository.get
method should already register the glob with the repository. The set of
globs are necessary because the repository may not do any file filtering
so the file event monitor needs to check the input globs to ensure that
the file event is for a file that actually requested by a task during
evaluation.
* Long term, I plan to add support for lifting tasks into a dynamic task
in a way that records _all_ of the possible dependencies for the task
through each of the dynamic code paths. We should revisit this change to
determine if its still necessary after that change.
I had needed to add proxy classes for the global FileTreeRepository so
that tasks that called the close method wouldn't actually stop the
monitoring done by the global repository. I realized that it makes a lot
more sense to just not provide direct access to the underlying file tree
repository and let the registerGlobalCaches manage its life cycle
instead.
This commit cleans up the approach for transforming the sbt state upon
completion of a task returning State. I add a new approach where a task
can return an instance of StateTransform, which is just a wrapper around
State. I then update EvaluateTask to apply this stateTransform rather
than the (optional) state transformation that may be stored in the Task
info parameter. By requiring that the user return StateTransform rather
than State directly, we ensure that no existing tasks that depend on the
state transformation function embedded in the Task info break. In sbt 2,
I could see the possibility of making this automatic (and probably
removing the state transformation function via attribute).
The problem with using the transformState attribute key is that it is
applied non-deterministically. This means that if you decorate a task
returning State, then the state transformation may or may not be
correctly applied.
I tracked this non-determinism down to the stateTransform
method in EvaluateTask. It iterates through the task result map and
chains all of the defined transformState attribute values. Because the
result is a map, this order is not specified. This chaining is arguably
a bad design because State => State does not imply commutivity. Indeed,
the problem here was that my state transformation functions were
constant functions, which are obviously non-commutative. I believe that
this logic likely written under the assumption that there would be no
more than one of these tranformations in a given result map.
I decided that it makes sense to move all of the new watch code out of
the Watched companion object since the Watched trait itself is now
deprecated. I don't really like having the new code in Watched.scala
mixed with the legacy code, so I pulled it all out and moved it into the
Watch object. Since we have to put all of the logic for the Continuous
object in main in order to access the sbt.Keys object, it makes sense to
move the logic out of main-command and into command so that most of the
watch related logic is in the same subproject.
This is a huge refactor of Watched. I produced this through multiple
rewrite iterations and it was too difficult to separate all of the
changes into small individual commits so I, unfortunately, had to make a
massive commit. In general, I have tried to document the source code
extensively both to facilitate reading this commit and to help with
future maintenance.
These changes are quite complicated because they provided a built-in
like api to a feature that is implemented like a plugin. In particular,
we have to manually do a lot of parsing as well as roll our own
task/setting evaluation because we cannot infer the watch settings at
project build time because we do not know a priori what commands the
user may watch in a given session. The dynamic setting and task
evaluation is mostly confined to the WatchSettings class in Continuous.
It feels dirty to do all of this extraction by hand, but it does seem to
work correctly with scopes.
At a high level this commit does four things:
1) migrate the watch implementation to using the InputGraph to collect
the globs that it needs to monitor during the watch
2) simplify WatchConfig to make it easier for plugin authors to write
their own custom watch implementations
3) allow configuration of the watch settings based on the task(s) that
is/are being run
4) adds an InputTask implemenation of watch.
Point #1 is mostly handled by Point #3 since I had to overhaul how _all_
of the watch settings are generated. InputGraph already handles both
transitive inputs and triggers as well as legacy watchSources so not
much additional logic is needed beyond passing the correct scoped keys
into InputGraph.
Point #3 require some structural changes. The watch settings cannot in
general be defined statically because we don't know a priori what tasks
the user will try and watch. To address this, I added code that will
extract the task keys for all of the commands that we are running. I
then manually extract the relevant settings for each command. Finally, I
aggregate those settings into a single WatchConfig that can be used to
actually implement the watch. The aggregation is generally
straightforward: we run all of the callbacks for each task and choose
the next watch state based on the highest priority Action that is
returned by any of the callbacks.
Because I needed Extracted to pull out the necessary settings, I was
forced to move a lot of logic out of Watched and into a new singleton,
Continuous, that exists in the main project (Watched is in the command
project). The public footprint of Continuous is tiny. Even though I want
to make the watch feature flexible for plugin authors, the
implementation and api remain a moving target so I do not want to be
limited by future binary compatibility requirements. Anyone who wants to
live dangerously can access the private[sbt] apis via reflection or by
adding custom code to the sbt package in their plugin (a technique I've
used in CloseWatch).
Point #2 is addressed by removing the count and lastStatus from the
WatchConfig callbacks. While these parameters can be useful, they are
not necessary to implement the semantics of a watch. Moreover, a status
boolean isn't really that useful and the sbt task engine makes it very
difficult to actually extract the previous result of the tasks that were
run. After this refactor, WatchConfig has a simpler api. There are fewer
callbacks to implement and the signatures are simpler. To preserve the
_functionality_ of making the count accessible to the user specifiable
callbacks, I still provided settings like watchOnInputEvent that accept
a count parameter, but the count is actually tracked externally to
Watched.watch and incremented every time the task is run.
Moreover, there are a few parameters of the watch: the logger and
transitive globs, that cannot be provided via settings. I provide
callback settings like watchOnStart that mirror the WatchConfig
callbacks except that they return a function from Continuous.Arguments
to the needed callback. The Continuous.aggregate function will check if
the watchOnStart setting is set and if it is, will pass in the needed
arguments. Otherwise it will use the default watchOnStart implementation
which simulates the existing behavior by tracking the iteration count in
an AtomicInteger and passing the current count into the user provided
callback. In this way, we are able to provide a number of apis to the
watch process while preserving the default behavior.
To implement #4, I had to change the label of the `watch` attribute key
from "watch" to "watched". This allows `watch compile` to work at the
sbt command line even thought it maps to the watchTasks key. The actual
implementation is almost trivial. The difference between an
InputTask[Unit] and a command is very small. The tricky part is that the
actual implementation requires applying mapTask to a delegate task that
overrides the Task's info.postTransform value (which is used to
transform the state after task evaluation). The actual postTransform
function can be shared by the continuous task and continuous command.
There is just a slightly different mechanism for getting to the state
transformation function.
This commit adds functionality to traverse the settings graph to find
all of the Inputs settings values for the transitive dependencies of the
task. We can use this to build up the list of globs that we must watch
when we are in a continuous build. Because the Inputs key is a setting,
it is actually quite fast to fetch all the values once the compiled map
is generated (O(2ms) in the scripted tests, though I did find that it
took O(20ms) to generate the compiled map).
One complicating factor is that dynamic tasks do not track any of
their dynamic dependencies. To work around this, I added the
transitiveDependencies key. If one does something like:
foo := {
val _ = bar / transitiveDependencies
val _ = baz / transitiveDependencies
if (System.getProperty("some.prop", "false") == "true") Def.task(bar.value)
else Def.task(baz.value)
}
then (foo / transitiveDependencies).value will return all of the inputs
and triggers for bar and baz as well as for foo.
To implement transitiveDependencies, I did something fairly similar to
streams where if the setting is referenced, I add a default
implementation. If the default implementation is not present, I fall
back on trying to extract the key from the commandLine. This allows the
user to run `show bar / transitiveDependencies` from the command line
even if `bar / transitiveDependencies` is not defined in the project.
It might be possible to coax transitiveDependencies into a setting, but
then it would have to be eagerly evaluated at project definition time
which might increase start up time too much. Alternatively, we could
just define this task for every task in the build, but I'm not sure how
expensive that would be. At any rate, it should be straightforward to
make that change without breaking binary compatibility if need be. This
is something to possibly explore before the 1.3 release if there is any
spare time (unlikely).
In order to walk the full dependency graph of a task, we need to know
the internal class path dependency configurations. Suppose that we have
projects a and b where b depends on *->compile in a. If we want to find
all of the inputs for b, then if we find that there is a dependency on
b/Compile/internalDependencyClasspath, then we must add a / Compile /
internalDependencyClasspath to the list of dependencies for the task.
I copied the setup of one of the other scripted tests that was
introduced to test the track-internal-dependencies feature to write a
basic scripted test for this new key and implementation.
This adds two new tasks: fileInputs and watchTriggers, that will be used by sbt
both to fetch files within a task as well as to create watch sources for
continuous builds. In a subsequent commit, I will add a task for a
command that will traverse the task dependency graph to find all of the
input task dependency scopes. The idea is to make it possible to easily
and accurately specify the watch sources for a task. For example, we'd
be able to write something like:
val foo = taskKey[Unit]("print text file contents")
foo / fileInputs := baseDirectory.value ** "*.txt"
foo := {
(foo / fileInputs).value.all.foreach(f =>
println(s"$f:\n${new String(java.nio.Files.readAllBytes(f.toPath))}"))
}
If the user then runs `~foo`, then the task should trigger if the user
modifies any file with the "txt" extension in the project directory.
Today, the user would have to do something like:
val fooInputs = settingKey[Seq[Source]]("the input files for foo")
fooInputs := baseDirectory.value ** "*.txt"
val foo = taskKey[Unit]("print text file contents")
foo := {
fooInputs.value.all.foreach(f =>
println(s"$f:\n${new String(java.nio.Files.readAllBytes(f.toPath))}"))
}
watchSources ++= fooInputs.value
or even worse:
val foo = taskKey[Unit]("print text file contents")
foo := {
(baseDirectory.value ** "*.txt").all.foreach(f =>
println(s"$f:\n${new String(java.nio.Files.readAllBytes(f.toPath))}"))
}
watchSources ++= baseDirectory.value ** "*.txt"
which makes it possible for the watchSources and the task sources to get
out of sync.
For consistency, I also renamed the `outputs` key `fileOutputs`.
The supershell output is distracting in CI. I added a system property,
sbt.ci, to explicitly set whether or not sbt is running in a ci build.
It was not at all obvious to me if the BUILD_NUMBER or CI environment
variables were set on travis or appveyor.
Fixes#4582#4443 introduced a perf enhacement of exluding sbt out of the metabuild, and instead appending the list of JARs resolved by the sbt launcher to the classpath.
This strategy worked in most cases, but it seems like some plugins are explicitly depending on IO module. In those cases, old IO would come before the new IO in the classpath ordering, resulting to "Symbol X is missing from classpath" error.
This fixes the issue by excluding all modules whose organization is `org.scala-sbt`.
As an escape hatch, I am adding a new key `reresolveSbtArtifacts`, which can be used to opt out of this behavior.
The usingTerminal method synchronizes the JLine object which can lead to
deadlock if multiple threads call it. When we just to want to read the
attributes of the terminal, but not modify it, there doesn't seem to be
any reason to use a lock.
I was seeing a number of compiler warnings about the type parameter
Scope:
Settings.scala:55:12: type parameter Scope defined in trait Init shadows class Scope defined in package util. You may want to rename your type parameter, or possibly remove it.
I'm not sure why I wasn't seeing these before, but the fix is simple.
Fixes#4574
This defines the `classLoaderLayeringStrategy` key at `Global` and `Zero / Test` level, and uses the scope delegation to pick them out from `test`.
Java has dropped the leading "1." from the specification version in
later versions. No one really refers to java 1.8, so it makes sense to
strip the "1." from the suggested version.
Ethan Atkins
eugene yokota
Dale Wijnand
Dale Wijnand
Ethan Atkins
Jason Zaugg