There's one test that starts to pass when we enable name hashing. It's
`import-class` which tests whether tracking of dependencies that arise
from imports is properly tracked. The name hashing algorithm uses different
dependency tracking compared to the old algorithm and the new dependency
extraction logic does handle import tree nodes properly so the test passes.
We "mark" the test passing by copying it and enabling the name hashing
flag in it. This is done similarly as in 940f7ff46d.
There are number of scripted tests that fail if we switch to name hashing
being enabled by default. There's no easy way to mark those tests as
pending only when name hashing flag is enabled so I decided to "mark" them
by copying those tests, enabling name hashing in each of them and mark
those copies as pending.
Here's explanation of each failing test:
* `constants` and `java-static` fail due to typer inlining constants
so we can't track dependencies properly (see SI-7173)
* `macro` fails for similar reasons as above: typer expands macros
and we can't track dependencies properly
* `struct` fails because it turns out that we need to handle structural
types in a special way both at declaration and use sites. At the moment
we handle them explicitly at declaration site so `struct-usage` passes
but `struct` fails
Provide implementation of invalidation logic that takes computed
name hashes into account. The implementation is spread amongst two
classes:
1. `IncrementalNameHashing` which implements a variant of
incremental compilation algorithm that computes modified
names and delegates to `MemberReferenceInvalidationStrategy`
when invalidating member reference dependencies
2. `MemberReferenceInvalidationStrategy` which implements the
core logic of dealing with dependencies introduced by member
reference. See documentation of that class for details.
The name hashing optimization is applied when invalidating source files
having both internal and external dependencies (in initial iteration),
check `invalidateByExternal` and `invalidateSource` methods for details.
As seen in implementation of `MemberReferenceInvalidationStrategy`
the name hashing optimization is not applied when implicit members
change.
NOTE: All functionality introduced in this commit is enabled only
when `IncOptions.nameHashing` flag is set to true.
The `source-dependencies/transitive-memberRef` test has been changed
to test name hashing variant of incremental compilation. The change
to invalidated files reflects the difference between the old and the
new algorithm.
Also, there a few new tests added that cover issues previously found
while testing name hashing algorithm and are fixed in this commit.
Each paragraph describes a single test.
Add a test case which shows that detect properly changes to type aliases
in the name hashing algorithm. See gkossakowski/sbt#6 for details.
Add test covering bug with use of symbolic names (issue
gkossakowski/sbt#5).
Add a test which covers the case where we refer to a name that is
declared in the same file. See issue gkossakowski/sbt#3 for details.
Add a test-case that documents current behavior of incremental
compiler when it comes to invalidating dependencies that arise
from inheritance and member references.
This requires a Format[T] to be implicitly available at the call site and requires the task
to be referenced statically (not in a settingDyn call). References to previous task values
in the form of a ScopedKey[Task[T]] + Format[T] are collected at setting load time in the
'references' setting. These are used to know which tasks should be persisted (the ScopedKey)
and how to persist them (the Format).
When checking/delegating previous references, rules are slightly different.
A normal reference from a task t in scope s cannot refer to t in s unless
there is an earlier definition of t in s. However, a previous reference
does not have this restriction. This commit modifies validateReferenced
to allow this.
TODO: user documentation
TODO: stable selection of the Format when there are multiple .previous calls on the same task
TODO: make it usable in InputTasks, specifically Parsers
Previously incremental compiler was extracting source code
dependencies by inspecting `CompilationUnit.depends` set. This set is
constructed by Scala compiler and it contains all symbols that given
compilation unit refers or even saw (in case of implicit search).
There are a few problems with this approach:
* The contract for `CompilationUnit.depend` is not clearly defined
in Scala compiler and there are no tests around it. Read: it's
not an official, maintained API.
* Improvements to incremental compiler require more context
information about given dependency. For example, we want to
distinguish between dependency on a class when you just select
members from it or inherit from it. The other example is that
we might want to know dependencies of a given class instead of
the whole compilation unit to make the invalidation logic more
precise.
That led to the idea of pushing dependency extracting logic to
incremental compiler side so it can evolve indepedently from Scala
compiler releases and can be refined as needed. We extract
dependencies of a compilation unit by walking a type-checked tree
and gathering symbols attached to them.
Specifically, the tree walk is implemented as a separate phase that
runs after pickler and extracts symbols from following tree nodes:
* `Import` so we can track dependencies on unused imports
* `Select` which is used for selecting all terms
* `Ident` used for referring to local terms, package-local terms
and top-level packages
* `TypeTree` which is used for referring to all types
Note that we do not extract just a single symbol assigned to `TypeTree`
node because it might represent a complex type that mentions
several symbols. We collect all those symbols by traversing the type
with CollectTypeTraverser. The implementation of the traverser is inspired
by `CollectTypeCollector` from Scala 2.10. The
`source-dependencies/typeref-only` test covers a scenario where the
dependency is introduced through a TypeRef only.
When the `source-dependencies/inherited-dependencies` test fails we
get a dump of a big collection of all dependencies with absolute
file paths printed. This is not very readable when one needs to
understand the actual difference.
I decided to test dependencies of each source file separately. This way
when assertion exception is thrown we get a stack trace that points
us at the line which tested dependencies of a specific source file.
Also, all files are relative to baseDirectory of the project.
This avoids an additional cause of recursion via the semicolon/multiple command, which fixes#933.
It also provides error messages on the expanded command. This fixes#598.
The fix was made possible by the very helpful information provided by @retronym.
This commit does two key things:
1. changes the owner when splicing original trees into new trees
2. ensures the synthetic trees that get spliced into original trees do not need typechecking
Given this original source (from Defaults.scala):
...
lazy val sourceConfigPaths = Seq(
...
unmanagedSourceDirectories := Seq(scalaSource.value, javaSource.value),
...
)
...
After expansion of .value, this looks something like:
unmanagedSourceDirectories := Seq(
InputWrapper.wrapInit[File](scalaSource),
InputWrapper.wrapInit[File](javaSource)
)
where wrapInit is something like:
def wrapInit[T](a: Any): T
After expansion of := we have (approximately):
unmanagedSourceDirectories <<=
Instance.app( (scalaSource, javaSource) ) {
$p1: (File, File) =>
val $q4: File = $p1._1
val $q3: File = $p1._2
Seq($q3, $q4)
}
So,
a) `scalaSource` and `javaSource` are user trees that are spliced into a tuple constructor after being temporarily held in `InputWrapper.wrapInit`
b) the constructed tuple `(scalaSource, javaSource)` is passed as an argument to another method call (without going through a val or anything) and shouldn't need owner changing
c) the synthetic vals $q3 and $q4 need their owner properly set to the anonymous function
d) the references (Idents) $q3 and $q4 are spliced into the user tree `Seq(..., ...)` and their symbols need to be the Symbol for the referenced vals
e) generally, treeCopy needs to be used when substituting Trees in order to preserve attributes, like Types and Positions
changeOwner is called on the body `Seq($q3, $q4)` with the original owner sourceConfigPaths to be changed to the new anonymous function.
In this example, no owners are actually changed, but when the body contains vals or anonymous functions, they will.
An example of the compiler crash seen when the symbol of the references is not that of the vals:
symbol value $q3 does not exist in sbt.Defaults.sourceConfigPaths$lzycompute
at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:49)
at scala.tools.nsc.Global.abort(Global.scala:254)
at scala.tools.nsc.backend.icode.GenICode$ICodePhase.genLoadIdent$1(GenICode.scala:1038)
at scala.tools.nsc.backend.icode.GenICode$ICodePhase.scala$tools$nsc$backend$icode$GenICode$ICodePhase$$genLoad(GenICode.scala:1044)
at scala.tools.nsc.backend.icode.GenICode$ICodePhase$$anonfun$genLoadArguments$1.apply(GenICode.scala:1246)
at scala.tools.nsc.backend.icode.GenICode$ICodePhase$$anonfun$genLoadArguments$1.apply(GenICode.scala:1244)
...
Other problems with the synthetic tree when it is spliced under the original tree often result in type mismatches or some other compiler error that doesn't result in a crash.
If the owner is not changed correctly on the original tree that gets spliced under a synthetic tree, one way it can crash the compiler is:
java.lang.IllegalArgumentException: Could not find proxy for val $q23: java.io.File in List(value $q23, method apply, anonymous class $anonfun$globalCore$5, value globalCore, object Defaults, package sbt, package <root>) (currentOwner= value dir )
...
while compiling: /home/mark/code/sbt/main/src/main/scala/sbt/Defaults.scala
during phase: global=lambdalift, atPhase=constructors
...
last tree to typer: term $outer
symbol: value $outer (flags: <synthetic> <paramaccessor> <triedcooking> private[this])
symbol definition: private[this] val $outer: sbt.BuildCommon
tpe: <notype>
symbol owners: value $outer -> anonymous class $anonfun$87 -> value x$298 -> method derive -> class BuildCommon$class -> package sbt
context owners: value dir -> value globalCore -> object Defaults -> package sbt
...
The problem here is the difference between context owners and the proxy search chain.
This feature is not activated by default. To enable it set `testForkedParallel` to `true`.
The test-agent then executes the tests in a thread pool.
For now it has a fixed size set to the number of available processors.
The concurrent restrictions configuration should be used.
The #958 describes a scenario where partially successful results are
produced in form of class files written to disk. However, if compilation
fails down the road we do not record any new compilation results (products)
in Analysis object. This leads to Analysis object and disk contents to get
out of sync.
One way to solve this problem is to use transactional ClassfileManager that
commits changes to class files on disk only when entire incremental
compilation session is successful. Otherwise, new class files are rolled
back to previous state.
The other way to solve this problem is to record time stamps of class files
in Analysis object. This way, incremental compiler can detect that class
files and Analysis object got out of sync and recover from that by
recompiling corresponding sources.
This commit uses latter solution which enables simpler (non-transactional)
ClassfileManager to handle scenario from #958.
Fixes#958
The ticket contains detailed description of the problem. The test case
just shows that if we set `incOptions := sbt.inc.IncOptions.Default`
then the incremental compiler doesn't recover from compiler errors
properly.
Fix the problem with unstable names synthesized for existential
types (declared with underscore syntax) by renaming type variables
to a scheme that is guaranteed to be stable no matter where given
the existential type appears.
The sheme we use are De Bruijn-like indices that capture both position
of type variable declarion within single existential type and nesting
level of nested existential type. This way we properly support nested
existential types by avoiding name clashes.
In general, we can perform renamings like that because type variables
declared in existential types are scoped to those types so the renaming
operation is local.
There's a specs2 unit test covering instability of existential types.
The test is included in compiler-interface project and the build
definition has been modified to enable building and executing tests
in compiler-interface project. Some dependencies has been modified:
* compiler-interface project depends on api project for testing
(test makes us of SameAPI)
* dependency on junit has been introduced because it's needed
for `@RunWith` annotation which declares that specs2 unit
test should be ran with JUnitRunner
SameAPI has been modified to expose a method that allows us to
compare two definitions.
This commit also adds `ScalaCompilerForUnitTesting` class that allows
to compile a piece of Scala code and inspect information recorded
callbacks defined in `AnalysisCallback` interface. That class uses
existing ConsoleLogger for logging. I considered doing the same for
ConsoleReporter. There's LoggingReporter defined which would fit our
usecase but it's defined in compile subproject that compiler-interface
doesn't depend on so we roll our own.
ScalaCompilerForUnit testing uses TestCallback from compiler-interface
subproject for recording information passed to callbacks. In order
to be able to access TestCallback from compiler-interface
subproject I had to tweak dependencies between interface and
compiler-interface so test classes from the former are visible in the
latter. I also modified the TestCallback itself to accumulate apis in
a HashMap instead of a buffer of tuples for easier lookup.
An integration test has been added which tests scenario
mentioned in #823.
This commit fixes#823.
* when mvn does a local 'install', it doesn't update the pom.xml last modified time if the pom.xml content hasn't changed
* an ivy.xml includes publicationDate, so an ivy.xml will always be touched even if the other content hasn't changed
* when Ivy checks if a snapshot is uptodate
+ it sees a SNAPSHOT, so it knows the module metadata and artifacts might change
+ it then checks the lastModified time of the metadata
+ if unchanged, it uses the cached information
+ if useOrigin is effectively false (either it is explicitly false or a resource is remote/isLocal=false),
this means that a new artifact won't be retrieved
* the Ivy IBiblioResolver
+ must be used for Maven repositories for proper behavior (no FileResolver, for example)
+ only returns URLResources, even for file: URLs
+ a FileResource is needed in combination with useOrigin to avoid copying artifacts from .m2/repository/
This commit fixes the above by setting a custom URLRepository on a constructed IBiblioResolver.
This URLRepository returns FileResources for file: URLs and standard URLResources for others.
The returned FileResource has isLocal=true and sbt sets useOrigin=true by default, so the artifacts
are used from the origin.
If it turns out a similar situation happens when mvn publishes to remote repositories, it is likely the fix for
that would be to figure out how to disable the lastModified check on the metadata and always download the metadata.
This would be slower, however.
The <type> element was generated for any explicit Artifact with a type other than "jar".
withSources() and withJavadoc() create explicit Artifacts of this kind, but shouldn't
have <type> elements. They are primarily artifacts with classifiers- the type is an
Ivy aspect. So, when writing to a pom, don't use write a type for known classifiers.
The test case compiles a project without and with this
setting and checks that a warning is and isn't emitted
respectively.
It's a multi-project build; this bug didn't seem to turn
up in a single-project build.
This drops older code that picked a single fingerprint within a framework.
This commit ensures only one name is executed across all frameworks.
The precedence is the order that frameworks are declared in Defaults.scala.
JUnit should be last because it is common for specs tests to be annotated
with @RunWith for Eclipse support. If a distinct fingerprint is not
picked, the test will be run by both specs and JUnit.
In addition to this main reason, result processing code assumes one result
per test class name and would have to be fixed if multiple fingerprints
were allowed.
The problem with pickled existential flag has been fixed upstream
in Scala 2.10 (the one sbt is using right now) so let's mark this
test as passing.
Note, that the fix went into Scala 2.9.3 so it's passing with that
version of Scala as well.
Closes#616. We don't need to work-around this on sbt side because
sbt already upgraded to Scala 2.10 where this is fixed.
Closes#623. Since sbt switched to Scala 2.10 we don't need a
specific version of Scala for this test.
Dan Sanduleac
Grzegorz Kossakowski
Mark Harrah
Bruno Bieth
Jason Zaugg
Eugene Yokota
cheeseng