Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

This line seems to contain a bug: if classpathEntry turns out to be a directory, this comparison unsurprisingly always fails: #1140

Closed
eed3si9n opened this issue Oct 24, 2022 · 3 comments · Fixed by #1382
Closed

This line seems to contain a bug: if classpathEntry turns out to be a directory, this comparison unsurprisingly always fails: #1140

eed3si9n opened this issue Oct 24, 2022 · 3 comments · Fixed by #1382

Comments

@eed3si9n
Copy link
Member

eed3si9n commented Oct 24, 2022 •
edited
Loading

original description

    This line seems to contain a bug: if classpathEntry turns out to be a directory, this comparison unsurprisingly always fails:

Invalidating '.../build/classes/scala/main/.../Foo.class' because
 .../build/classes/scala/main/....class (farm(77c283a23fed88f2)) != .../build/classes/scala/main (absent)
                                                                         ^^^ RHS is a ***directory***.

The resolve step was completely dropped in this PR.

Originally posted by @mernst-github in #712 (comment)
@tgodzik tgodzik mentioned this issue Oct 27, 2022
Closed
@mernst-github mernst-github mentioned this issue Nov 18, 2022
Merged
bot-gradle added a commit to gradle/gradle that referenced this issue Dec 14, 2022
@bot-gradle @alllex
Merge pull request #22786 Scala Plugin: remove ExternalBinariesLookup
bf44f79 
Since Gradle 7.x, Gradle's implementation of `ExternalLookup` has become out of sync with Zinc's file stamping and always triggers full recompilation.

Since an implementation of `ExternalLookup` is optional and [Zinc's default](https://github.com/sbt/zinc/blob/develop/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala#L734) already does what Gradle is trying to do here, the Gradle-customization can be completely removed.

Passes all existing tests under `./gradlew scala:build` which are all *intra-project* incremental compilation tests, so we know this change doesn't break anything there. It would probably be good to have (had) an *inter-project* incremental compilation test as well to ensure such a regression doesn't happen again, but that is beyond the scope of what I can contribute at this point. @justinb99's reproducer in #20101 would certainly be a good starting point.

Note that this does not address the interplay of the java-library plugin and sbt/zinc#1140 , i.e. only works for jar dependencies. The [workaround](https://github.com/sbt/zinc/blob/develop/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala#L44) in zinc might be a short-term solution for that.

Signed-off-by: matthias_ernst <matthias_ernst@apple.com>

Fixes #20101
Fixes #22964

Co-authored-by: Alex Semin <asemin@gradle.com>
@tnielens-centrica
Copy link

We are facing productivity issues in a gradle multi-project repo with several small gradle subprojects with the plugin java-library applied. @mernst-github @tgodzik, would you know a workaround for this? Would a zinc downgrade help?

@mernst-github
Copy link

Try enabling https://github.com/sbt/zinc/blob/develop/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala#L46 in the compileScala.scalaCompileOptions.forkOptions.

@tnielens-centrica
Copy link

Seems to be working for us. Thanks 🙏

@adpi2 adpi2 mentioned this issue Aug 7, 2024
Merged
@tnielens tnielens mentioned this issue Oct 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #1140: overcompilation when using a class directory as a library adpi2/zinc
3 participants
@eed3si9n @mernst-github @tnielens-centrica