Allow clients to override hashing classpath. #427

romanowski · 2017-10-10T22:42:44Z

This is useful in big builds with 100+ projects with 100+ jars on classpath (mostly the same).

Fixes #400

jvican · 2017-10-11T22:47:17Z

@romanowski Formatting error on CI. I also expect this to report a MiMa error. Make sure to rebase this PR with the latest change in master and add the forward mima issue to the whitelist.

Thank you Kryzstoff.

jvican · 2017-10-13T19:08:47Z

Can someone in the sbt team review this?

eed3si9n · 2017-10-14T01:23:22Z

internal/compiler-interface/src/main/java/xsbti/compile/ExternalHooks.java

@@ -51,6 +51,8 @@
         * @return API changes
         */
        boolean shouldDoIncrementalCompilation(Set<String> changedClasses, CompileAnalysis previousAnalysis);
+
+        Optional<FileHash[]> hashClasspath(File[] classpath);


Kind of curious. Does it matter if this takes File[] or just hash(File)?

This way you can add e.g. paralelizm but with hash(File) you are not.

ok. Sounds good to me.

eed3si9n

Added a question.

fommil · 2017-10-14T16:27:24Z

zinc/src/main/scala/sbt/internal/inc/LookupImpl.scala

@@ -64,4 +62,7 @@ class LookupImpl(compileConfiguration: CompileConfiguration, previousSetup: Opti
  override def shouldDoIncrementalCompilation(changedClasses: Set[String],
                                              analysis: CompileAnalysis): Boolean =
    externalLookup.forall(_.shouldDoIncrementalCompilation(changedClasses, analysis))
+
+  override def hashClasspath(classpath: Array[File]): Optional[Array[FileHash]] =


nice, this allows the user to provide a parallel implementation.

fommil · 2017-10-14T16:44:24Z

zinc/src/main/scala/sbt/internal/inc/LookupImpl.scala

-      case a if a.relations.productClassName._2s.contains(binaryClassName) => a
-    }
-  }
+  override def lookupAnalysis(binaryClassName: String): Option[CompileAnalysis] =


why not make this non-optional and provide a default implementation?

Empty result here has a meaning - that we have to treat this class as Library dependency instead of External one (so names will be invalidated on any change in *.class or *.jar file).

This change was just optimization since find is faster then collectFirst (and code looks also better)

I thought the empty meant that it falls back to the default impl of blindly using SHA-1. Couldn't that default impl just be moved to be the default?

jvican · 2017-10-16T08:51:43Z

I'd like to do some work on this area. Can we approve and merge this? /cc @sbt/zinc-team

jvican

LGTM.

jvican · 2017-10-24T09:31:13Z

Updating branch. If it succeeds, I'll merge.

romanowski · 2017-10-26T11:18:33Z

@jvican all is green

* 1.0.x: (25 commits) Add yourkit acknoledgement in the README 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 Fix source-dependencies/sealed Import statement no longer needed Move mima exclusions to its own file ... Conflicts: internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala 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 MixedAnalyzingCompiler conflict is due to: * sbt#427 (a 1.x PR), conflicting with * sbt#452 (a 1.0.x 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).

* 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 #332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix #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 #436: Remove annoying log4j scripted exception Fix #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/zinc#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/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#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. (#413, #418, #453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x 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/zinc#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/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#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/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x 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 scala#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix scala#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 scala#436: Remove annoying log4j scripted exception Fix scala#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/zinc#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/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#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. (scala#413, scala#418, scala#453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x 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 scala#332: Add sbt-header back to the build Update sbt-scalafmt to 1.12 Make classpath hashing more lightweight Fix scala#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 scala#436: Remove annoying log4j scripted exception Fix scala#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/zinc#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/zinc#393 (a 1.x PR), conflicting with * sbt/zinc#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. (scala#413, scala#418, scala#453). The MixedAnalyzingCompiler conflict is due to: * sbt/zinc#427 (a 1.x PR), conflicting with * sbt/zinc#452 (a 1.0.x PR). Rewritten from sbt/zinc@e245d95

romanowski force-pushed the hashing-classpath-proposal branch from a1f6814 to f8b5010 Compare October 10, 2017 22:48

Add hook to override classpath hashing

615351d

romanowski force-pushed the hashing-classpath-proposal branch from f8b5010 to 615351d Compare October 12, 2017 07:22

eed3si9n reviewed Oct 14, 2017

View reviewed changes

eed3si9n requested changes Oct 14, 2017

View reviewed changes

romanowski mentioned this pull request Oct 14, 2017

Timestamp hashes #434

Closed

fommil reviewed Oct 14, 2017

View reviewed changes

fommil mentioned this pull request Oct 14, 2017

Hash shortcut proof of concept [DO NOT MERGE] sbt/io#80

Closed

fommil reviewed Oct 14, 2017

View reviewed changes

jvican added binary api community feature labels Oct 16, 2017

jvican approved these changes Oct 16, 2017

View reviewed changes

eed3si9n approved these changes Oct 24, 2017

View reviewed changes

Merge branch '1.x' into hashing-classpath-proposal

a7facec

jvican merged commit 8c42c1f into sbt:1.x Oct 26, 2017

stuhood mentioned this pull request Nov 9, 2017

Fix #433: Make classpath hashing more lightweight #452

Merged

dwijnand mentioned this pull request Nov 22, 2017

Merge 1.0.x into 1.x #455

Merged

dwijnand mentioned this pull request May 19, 2020

Epic: Integrate changes in mspnf/zinc #777

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow clients to override hashing classpath. #427

Allow clients to override hashing classpath. #427

romanowski commented Oct 10, 2017

jvican commented Oct 11, 2017

jvican commented Oct 13, 2017 •

edited

Loading

eed3si9n Oct 14, 2017

romanowski Oct 14, 2017

eed3si9n Oct 24, 2017

eed3si9n left a comment

fommil Oct 14, 2017

fommil Oct 14, 2017

romanowski Oct 14, 2017

fommil Oct 14, 2017

jvican commented Oct 16, 2017

jvican left a comment

jvican commented Oct 24, 2017

romanowski commented Oct 26, 2017

Allow clients to override hashing classpath. #427

Allow clients to override hashing classpath. #427

Conversation

romanowski commented Oct 10, 2017

jvican commented Oct 11, 2017

jvican commented Oct 13, 2017 • edited Loading

eed3si9n Oct 14, 2017

Choose a reason for hiding this comment

romanowski Oct 14, 2017

Choose a reason for hiding this comment

eed3si9n Oct 24, 2017

Choose a reason for hiding this comment

eed3si9n left a comment

Choose a reason for hiding this comment

fommil Oct 14, 2017

Choose a reason for hiding this comment

fommil Oct 14, 2017

Choose a reason for hiding this comment

romanowski Oct 14, 2017

Choose a reason for hiding this comment

fommil Oct 14, 2017

Choose a reason for hiding this comment

jvican commented Oct 16, 2017

jvican left a comment

Choose a reason for hiding this comment

jvican commented Oct 24, 2017

romanowski commented Oct 26, 2017

jvican commented Oct 13, 2017 •

edited

Loading