Conversation
|
/cc @gkossakowski ;-) |
|
/cc @Duhemm |
|
Thanks -- any pointers to developer docs on which test targets to run before submitting the PR are welcome. |
|
Do you have an example of scenario you want to address? |
|
The scenarios are described using scripted test (see http://eed3si9n.com/testing-sbt-plugins), which is what @gkossakowski showed. To run it for |
|
The command to run the tests is You may also want to try using this: https://github.com/sbt/incrementalcompiler/blob/1.0/internal/incrementalcompiler-core/src/test/scala/sbt/inc/IncrementalTest.scala. It may be easier to make ensure that what you want to have recompiled is actually recompiled. |
|
Also note that for sbt 1.0, we've split incremental compiler into https://github.com/sbt/incrementalcompiler so you might have to send us the PR there too. |
|
This will need to be backported as it's for a customer that I assume is still on 0.13. What's the preferred direction? Submit to 1.0 and backport or other way around? (This patch needs to be sent to them before the end of the month, though that's not super relevant for the immediate discussion.) |
|
The bug I'm trying to fix is spurious recompilation, triggered for classes without self types. This was spotted in the wild, and I don't have a repro, but it looked like the representation of their self type in the extracted API would vary between no type and the type of the class, causing a rebuild. |
|
/cc @retronym |
|
Regarding the intermittent Here's the bug I mentioned in our chat today related to a type causes a loop in |
If 0.13.10 needs to be issued, then we should get it into 0.13 first. |
sbt was reporting warning abouts inconsistent versions of dependencies even if these dependencies didn't have the same configuration (as in `provided` vs `compile`). This commit fixes this problem by comparing the dependencies by organization, artifact name and configuration. Fixes sbt#1933
|
We don't immediately need a new release, though this should end up in one. |
7b05583 to
ef7d3c3
Compare
The signatures of methods that have value classes as arguments or return type change during the erasure phase. Because we only registered signatures before the erasure, we missed some API changes when a class was changed to a value class (or a value class changed to a class). This commit fixes this problem by recording the signatures of method before and after erasure. Fixes sbt#1171
7d85ebb to
c093d42
Compare
The crash manifested as a `NotSerializableException`, because we don't visit the private subtrees of the API graph, which would previously force all Lazy stubs as a side-effect. There was a backstop in `AbstractLazy` (`writeReplace`), but somehow that didn't work. ``` java.io.NotSerializableException: sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl [...] at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:329) at xsbt.api.SourceFormat$.writes(SourceFormat.scala:24) at xsbt.api.SourceFormat$.writes(SourceFormat.scala:15) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:329) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:329) at sbt.inc.TextAnalysisFormat21661anonfun at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.javr$.aggregate(TextAnalysisFormat.scala:18) at sbt.inc.TextAnalysisFormat$.objToString(TextAnalysisFormat.scala:329) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:213) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:213) at sbt.inc.TextAnalysisFormat21661anonfun21661writeMap.apply(TextAnalysisFormat.scala:381) at sbt.inc.TextAnalysisFormat21661anonfun21661writeMap.apply(TextAnalysisFormat.scala:377) at scala.collection.mutable.ResizableArray.foreach(Resizable at sbt.inc.TextAnalysisFormalection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47) at sbt.inc.TextAnalysisFormat$.sbt21661writeMap(TextAnalysisFormat.scala:377) at sbt.inc.TextAnalysisFormat$.write(TextAnalysisFormat.scala:216) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:64) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:64) at sbt.inc.TextAnalysisFormat21661anonfun.apply(TextAnalysisFormat.scala:64) at sbt.inc.FormatTimer$.aggregate(TextAnalysisFo at sbt.inc.TextAnalysisFormat21661anonfun.scala:25) at sbt.inc.TextAnalysisFormat$.write(TextAnalysisFormat.scala:64) at sbt.inc.FileBasedStore21661anon21661anonfun.apply(FileBasedStore.scala:12) at sbt.inc.FileBasedStore21661anon21661anonfun.apply(FileBasedStore.scala:12) at sbt.Using.apply(Using.scala:24) at sbt.inc.FileBasedStore21661anon.set(FileBasedStore.scala:12) at sbt.inc.AnalysisStore21661anon.set(AnalysisStore.scala:16) at sbt.inc.AnalysisStore21661anon.set(AnalysisStore.scala:27) at sbt.Defaults21661anonfun.apply(Defaults.scala:799) at sbt.Defaults21661anonfun at sbt.inc.TextAts.scala:793) at scala.Function121661anonfun.apply(Function1.scala:47) at sbt.21661anonfun21661u2219.apply(TypeFunctions.scala:40) at sbt.std.Transform21661anon.work(System.scala:63) at sbt.Execute21661anonfun21661anonfun.apply(Execute.scala:226) at sbt.Execute21661anonfun21661anonfun.apply(Execute.scala:226) at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17) at sbt.Execute.work(Execute.scala:235) at sbt.Execute21661anonfun.apply(Exe at sbt.Using.apply(Using.scala:24) mit.apply(Execute.scala:226) at sbt.ConcurrentRestrictions21661anon21661anonfun.apply(ConcurrentRestrictions.scala:159) at sbt.CompletionService21661anon.call(CompletionService.scala:28) at java.util.concurrent.FutureTask.innerRun(FutureTask.java:303) at java.util.concurrent.FutureTask.run(FutureTask.java:138) at java.util.concurrent.Executors.call(Executors.java:439) at java.util.concurrent.FutureTask.innerRun(FutureTask.java:303 at sbt.std.Transform21661anon.work(System(FutureTask.java:138) at java.util.concurrent.ThreadPoolExecutor.runTask(ThreadPoolExecutor.java:895) at java.util.concurrent.ThreadPoolExecutor.run(ThreadPoolExecutor.java:918) at java.lang.Thread.run(Thread.java:695) [error] (compile:compile) java.io.NotSerializableException: sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl [error] Total time: 2 s, completed Dec 21, 2015 12:05:19 PM ```
c093d42 to
9f5fd6c
Compare
Fix for regression triggered by sbt#2325 -- apparently, the API visitor would force all the lazy stubs so the `AbstractLazy` `writeReplace` was not exercised. With the private subtrees being ignored, the visitor didn't force the lazy stub, and the `writeProtected` method was not inherited in `SafeLazy.Impl`. From https://docs.oracle.com/javase/7/docs/api/java/io/Serializable.html `writeReplace` must be protected for its override to be inherited. > `Serializable` classes that need to designate an alternative object to be used > when writing an object to the stream should implement this special method with > the exact signature: > `ANY-ACCESS-MODIFIER Object writeReplace() throws ObjectStreamException;` > > This `writeReplace` method is invoked by serialization if the method exists and > it would be accessible from a method defined within the class of the object > being serialized. Thus, the method can have `private`, `protected` and > `package-private` access. > > **Subclass access to this method follows java accessibility rules.** (Thanks to retronym for digging up the docs.) The fix is captured, indirectly, by the scripted test `source-dependencies/java-analysis-serialization-error`.
Call `initialize` in case symbol's `info` hadn't been completed during normal compilation. Also, normalize to the class symbol immediately. Add a TODO regarding only looking at class symbols, and thus ignoring the term symbol for objects, as the corresponding class symbol has all the relevant info.
The only aspect of the self variable that's relevant for incremental compilation is its explicitly declared type, and only when it's different from the type of the class that declares it. Technically, any self type that's a super type of the class could be ignored, as it cannot affect external use (instantiation/subclassing) of the class.
Motivated because we want to make it more robust & configurable.
Original motivation was to diagnose a cyclic type representation,
likely due to an f-bounded existential type, as illustrated by the following:
```
class Dep {
// The API representation for `bla`'s result type contains a cycle
// (an existential's type variable's bound is the existential type itself)
// This results in a stack overflow while showing the API diff.
// Note that the actual result type in the compiler is not cyclic
// (the f-bounded existential for Comparable is truncated)
def bla(c: Boolean) = if (c) new Value else "bla"
}
class Value extends java.lang.Comparable[Value] { def compareTo(that: Value): Int = 1 }
```
Limit nesting (`-Dsbt.inc.apidiff.depth=N`, where N defaults to `2`),
and number of declarations shown for a class/structural type
(via `sbt.inc.apidiff.decls`, which defaults to `0` -- no limit).
Limiting nesting is crucial in keeping the size of api diffs of large programs
within a reasonable amount of RAM...
For example, compiling the Scala library, the API diff with nesting at `4`
exhausts 4G of RAM...
9f5fd6c to
de25c20
Compare
For refinement types, the Structure was already restricted
to declarations (and not inherited members), but all base types
were still included for a refinement's parents, which would
create unwieldy, and even erroneous (cyclic) types by expanding
all constituents of an intersection type to add all base types.
Since the logic already disregarded inherited members, it seems
logical to only include direct parents, and not all ancestor types.
```
class Dep {
def bla(c: Boolean) = if (c) new Value else "bla"
}
class Value extends java.lang.Comparable[Value] { def compareTo(that: Value): Int = 1 }
```
de25c20 to
0d32d1b
Compare
Specialize two implementations for each value of the `inherit` boolean argument. Also use a more direct way of distinguishing declared and inherited members. backwards compat for source-dependencies/inherited-dependencies
Also a bit more complete: handle SelectFromTypeTree, consider the self type an inheritance dependency, and flatten any refinement types in inherited types, to get to the symbols of their parents, instead of the useless symbol of the refinement class. Include inheritance dependencies in regular ones Also, update test to reflect the self type is now seen as an inheritance dependency. self types are local, so don't treat them like inherited types note inheritanceSymbols dealiases, where allSymbols is constructed differently fix NPE in source-dependencies/macro-annotation
Since pickled annotated types and symbols only mention static annotations, whereas compilation from source sees all annotations, we must explicitly filter annotations in the API representation using the same criteria as the pickler, so that we generate the same API when compiling from source as when we're loading classfiles.
Further categorize debug output as api diff ([diff]), and invalidation ([inv]). Since we log a ton of diagnostics, make it a bit easier to find the relevant bits.
0d32d1b to
9969e82
Compare
WIP PR. @eed3si9n, any suggestions on how to polish (/test) it so that it can be accepted?