Merge 1.0.x into 1.x #455
Merged
Merge 1.0.x into 1.x #455
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
Follow-up on sbt#314 - I _still_ misinterpreted.. Turns out the ".asInstanceOf[AnyRef].getClass.getName" implementation was the _original_ implementation. Then Mark switched to using bindValue in sbt/sbt@4b8f0f3. Since Scala 2.11.0 (scala/scala#1648 in particular) bindValue was removed. So we'll use NamedParam and quietBind, both which exist since Scala 2.9.0. Fixes sbt/sbt#2884, tested with local releases.
Fix ConsoleInterface binding things properly^2
I get NPE when I extend ManagedLoggedReporter if I don't do this.
neo-sbt-scalafmt and making LoggedReporter extensible
xsbti Java classes were ported into compiler bridge, instead of the compiler interface by mistake. Since there's not code utilizing this interface yet, this was never caught.
Move REPL related xsbti Java classes to the correct module
sealed test used to be implemented with `-> compile` in sbt/sbt but was marked pending in 8e65c00. This changes back to the original state.
This commit proves that the error only exists with 2.11.x.
### background In sbt 0.13 days, we could ignore the relationship between two classes defined in the same `*.scala` source file, because they will be compiled anyway, and the invalidation was done at the source file level. With class-based namehashing, the invalidation is done at the class level, so we can no longer ignore inheritance relationship coming from the same source, but we still have old assumptions scattered around the xsbt-dependency implementation. ### what we see without the fix ``` [info] Compiling 1 Scala source to ... .... [debug] [inv] internalDependencies: [debug] [inv] DependencyByInheritance Relation [ [debug] [inv] xx.B -> gg.table.A [debug] [inv] xx.Foo -> xx.C [debug] [inv] ] [debug] [inv] DependencyByMemberRef Relation [ [debug] [inv] xx.B -> gg.table.A [debug] [inv] xx.Hello -> gg.table.A [debug] [inv] xx.Foo -> xx.C [debug] [inv] ] .... Caused by: java.lang.AbstractMethodError: xx.Foo.buildNonemptyObjects(II)V ``` First, we see that `xx.C -> xx.B DependencyByInheritance` relationship is missing. Second, the error message seen is `java.lang.AbstractMethodError` happening on `xx.Foo`. ### what this changes This change changes two if expressions that was used to filter out dependency info coming from the same source. One might wonder why it's necessary to keep the local inheritance info, if two classes involved are compiled together anyways. The answer is transitive dependencies. Here's likely what was happening: 1. `gg.table.A` was changed, 2. causing `xx.B` to invalidate. 3. However, because of the missing same-source inheritance, it did not invalidate `xx.C`. 4. This meant that neither `xx.Foo` was invalidated. 5. Calling transform method on a new `xx.Foo` causes runtime error. By tracking same-source inheritance, we will now correctly invalidate `xx.C` and `xx.Foo`. I think the assumption that's broken here is that "we don't need to track inheritance that is happening between two classes in a same source." ### Is this 2.11 only issue? No. The simple trait-trait inheritance reproduction alone will not cause problem in Scala 2.12 because of the [compile-to-interface](http://www.scala-lang.org/news/2.12.0/#traits-compile-to-interfaces) traits. However, not all traits will compile to interface. This means that if we want to take advantage of the compile-to-interface traits, we still should keep track of the same-source inheritance, but introduce some more logic to determine whether recompilation is necessary. Fixes sbt#417
`source-dependencies / patMat-scope` passes locally for me. The invalidation on the CI is likely caused by: ``` [debug] Recompiling all 3 sources: invalidated sources (2) exceeded 50.0% of all sources ``` This attempts to workaround that by adding more source. This does not affect the fidelity of the original test.
Fixes undercompilation on inheritance on same source
It looks like scalac encodes access rights of objects in their names. To make sure that we get the right simple names, we need to use `unexpandedName` instead of `name` which will decipher these access rights and return their simple names insted (with all the previous `$$` prefixes stripped out).
Fix sbt#127: Use unexpanded names
These are the improvements that I've added to scripted:
1. Scripted is now parallel and does batch execution.
2. Scripted logs to both a file and the console (if `bufferLog == true`).
All the logs can be inspected locally by going to a directory in
`/tmp`. This directory is shown to the user at the end of the
execution.
3. Scripted UI has been improved.
3.1. Colors are used for `+` and `x`.
3.1. It shows the command that actually failed, not `Command failed {line 1}`.
3.2. It trims the stack traces of the wrapping exceptions
(corresponding to the scripted infrastructure). Only the stack traces
of the causing exceptions are shown (which are the ones we're
interested in and are usually assertion errors).
I think these improvements enhance the current dev workflow
considerably. I invite you to give them a try.
This change combined with sbt#429, gives a really fast execution of
scripted. Testing just one test is under 7 seconds in my machine (note
that in those 7 seconds we have to fetch the bridge, etc).
Fix sbt#436: Remove annoying log4j scripted exception
`Method.getGenericParameterType` may sometimes return `null` for the return types when the method's return type is indeed generic. It's not clear yet where this "sometimes" happens, but it looks like the JDK is not able to tell the return type of a lambda returning the generic class. This can be seen in the `java-lambda-typeparams` scripted test. In this context, lambda metafactory synthesizes a lambda class that returns a class that is parameterized in its return type. In that context, when `ClassToAPI` inspects the synthesized lambda and tries to figure out the full return type of it, the `null` is returned. It looks like the Java reflection API sparingly returns `null`s. We can see that in `ClassToAPI` where we guard against `null`s in lots of other places. So the guard added by this commit is not a novelty, but rather the norm. Fixes sbt#389.
Ignore null in generic lambda tparams
We were not testing what we thought we were testing before, because C used the name "x" which is also the name of the parameter we change in A, so when we changed A we were invalidating C too. Fixed by using a different name than "x" in C, which reveals that there is a bug somewhere since the test doesn't pass anymore.
Quoting from 1e7e99e: If the underlying type of a value class change, its name hash doesn't change, but the name hash of <init> change and since every class uses the name <init>, we don't need to do anything special to trigger recompilations either This was true until aca8dfa where we started giving unique names to constructors. This broke the value-class-underlying type but this wasn't noticed because the test was broken in the same commit (and has now been fixed in the previous commit in this PR).
Fix sbt#442: Name hash of value class should include underlying type
And make it parallel! This patch adds a cache that relies on filesystem metadata to cache hashes for jars that have the same last modified time across different compiler iterations. This is important because until now there was a significant overhead when running `compile` on multi-module builds that have gigantic classpaths. In this scenario, the previous algorithm computed hashes for all jars transitively across all these projects. This patch is conservative; there are several things that are wrong with the status quo of classpath hashing. The most important one is the fact that Zinc has been doing `hashCode` on a SHA-1 checksum, which doesn't make sense. The second one is that we don't need a SHA-1 checksum for the kind of checks we want to do. sbt#371 explains why. The third limitation with this check is that file hashes are implemented internally as `int`s, which is not enough to represent the richness of the checksum. My previous PR also tackles this problem, which will be solved in the long term. Therefore, this pull request only tackles these two things: * Caching of classpath entry hashes. * Parallelize this IO-bound task. Results, on my local machine: - No parallel hashing of the first 500 jars in my ivy cache: 1330ms. - Parallel hashing of the first 500 jars in my ivy cache: 770ms. - Second parallel hashing of the first 500 jars in my ivy cache: 1ms. Fixes sbt#433.
Fix sbt#433: Make classpath hashing more lightweight
This also proves that our sbt-header configuration works.
Add headers and move compiler interface
* add new compiler bridge
…llel Improve and make scripted parallel
Add yourkit acknoledgement in the README
Fixes sbt#395, sbt/sbt#3427 In scala/scala#5903 Scala compiler's REPL-related classes went through some changes, including move to a different package. This implements a new compiler bridge tracking the changes. To verify that the new bridge compiles under 2.13, we need to compile it using sbt 1.0.3, which in turn requires a bridge compatible with Scala 2.13.0-M2. To work around this chicken-egg, I've manually created a bridge and published it to Maven Central as "org.scala-sbt" % "compiler-bridge_2.13.0-M2" % "1.1.0-M1-bootstrap2".
Splitting compiler bridge tests to another subproject because while the bridge itself can be compiled with just compiler-interface, util-interface, and Scala Compiler as dependencies, the testing introduces more (such as IO). This creates problem for new Scala versions where IO or test libraries do not exist yet (e.g. Scala 2.13.0-M2). This also removes the Mima test due to the lack of 2.13 bridge for Zinc 1.0.0. Compiler bridge just needs to compile itself against the interface and Scala compiler, so there's no need to run Mima test.
eed3si9n
previously approved these changes
Nov 22, 2017
"sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0
Closed
|
I'm happy to merge this as soon as the recently merged PR is added to the PR. |
* 1.0.x: (28 commits) Split compiler bridge tests to another subproject Implement compiler bridge for 2.13.0-M2 Add yourkit acknoledgement in the README "sbt '++ 2.13.0-M2!' compile" does not work with sbt 1.0.0 Add header to cached hashing spec Add headers to missing files Fix sbt#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix sbt#442: Name hash of value class should include underlying type source-dependencies/value-class-underlying: fix test Ignore null in generic lambda tparams Improve and make scripted parallel Fix sbt#436: Remove annoying log4j scripted exception Fix sbt#127: Use `unexpanded` name instead of `name` Add pending test case for issue/127 source-dependencies / patMat-scope workaround Fixes undercompilation on inheritance on same source Add real reproduction case for sbt#417 Add trait-trait-212 for Scala 2.12.3 ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala project/build.properties zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala The ClassToAPI conflict is due to: * sbt#393 (a 1.x PR), conflicting with * sbt#446 (a 1.0.x PR). The build.properties conflict is due to different PRs bumping sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (sbt#413, sbt#418, sbt#453). The MixedAnalyzingCompiler conflict is due to: * sbt#427 (a 1.x PR), conflicting with * sbt#452 (a 1.0.x PR).
dwijnand
force-pushed
the
merge-1.0.x-into-1.x
branch
from
8aa95a5
to
e245d95
Compare
|
Ah yes, well remembered. Done. |
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.
Conflicts:
The ClassToAPI conflict is due to:
The build.properties conflict is due to different PRs bumping
sbt.version from 1.0.0 to 1.0.2 to 1.0.3. (#413, #418, #453).
The MixedAnalyzingCompiler conflict is due to: