Update Zinc used by the Scala plugin to 1.6.1 #19864

tgodzik · 2022-02-09T17:15:34Z

Context

The currently used version of zinc is havily outdated and missing a lot of new fixes. It's also being depracated and will most likely stop working at some point with Scala 3

Contributor Checklist

Review Contribution Guidelines
Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
Provide unit tests (under <subproject>/src/test) to verify logic
Update User Guide, DSL Reference, and Javadoc for public-facing changes
Ensure that tests pass sanity check: ./gradlew sanityCheck
Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest

Gradle Core Team Checklist

Verify design and implementation
Verify test coverage and CI build status
Verify documentation
Recognize contributor in release notes

gradle/verification-keyring.keys

...ala/src/integTest/groovy/org/gradle/scala/compile/UpToDateScalaCompileIntegrationTest.groovy

tgodzik · 2022-02-09T17:59:53Z

@ljacomet could you take a look at it?

dubinsky

Thanks!!!

subprojects/scala/build.gradle.kts

ljacomet

Thanks a lot for helping out Gradle Scala users!

See below for a couple comments on dependency verification.

Most important though is the compatibility conversation. I believe we had one already when Zinc 1.5 was released, but I cannot find it ...

gradle/verification-keyring.keys

gradle/verification-metadata.xml

...ala/src/integTest/groovy/org/gradle/scala/compile/UpToDateScalaCompileIntegrationTest.groovy

dubinsky · 2022-03-23T18:38:18Z

@tgodzik Any updates on this? Thanks!

ljacomet

First of all, thanks for the contribution.
And sorry it took so long to get a decision for having this in a minor.

But the answer is now yes, so let's see about getting it in Gradle 7.5.

Would you be able to also do some documentation changes:

Update the Zinc compatibility table
Add an entry into the potential breaking changes of the upgrade guide - file upgrading_version_7.adoc

ljacomet · 2022-04-06T14:17:28Z

Looks like running all tests on CI raises a number of problems.
Can you reproduce the issues in org.gradle.scala.compile.* tests?

tgodzik · 2022-04-07T10:16:22Z

Looks like running all tests on CI raises a number of problems. Can you reproduce the issues in org.gradle.scala.compile.* tests?

I will take a look as soon as possible, just got back from vacation so that might take a bit, but hopefully I should get it fixed by next week.

tgodzik · 2022-04-15T16:45:47Z

Looks like running all tests on CI raises a number of problems. Can you reproduce the issues in org.gradle.scala.compile.* tests?

@ljacomet The tests should be fixed, I added a section for the Zinc compatibility table and added a section for potential breaking changes

tgodzik · 2022-04-25T15:06:31Z

@ljacomet I haven't noticed that some of the tests were failing, but they should be fixed now, protobuf was dropped from zinc (shaded version exists). It is still in Scala 3, but that is most likely related to the fact that the bridge resides in the compiler itself instead of Zinc,

ljacomet · 2022-04-25T18:20:45Z

@tgodzik Could you rebase this against the release branch?
I would like to try to get it in 7.5.

You will then need to change the target branch to release as well.

Signed-off-by: Tomasz Godzik <tomek.godzik@gmail.com>

Signed-off-by: Tomasz Godzik <tgodzik@virtuslab.com>

The issue is most likely related to sbt/zinc#799 Signed-off-by: Tomasz Godzik <tgodzik@virtuslab.com>

ljacomet · 2022-04-26T07:41:01Z

I have done the rebase and change of base branch.

Issue gradle#15491

ljacomet · 2022-04-26T09:01:04Z

@bot-gradle test this

bot-gradle · 2022-04-26T09:01:12Z

OK, I've already triggered the following builds for you:

PullRequestFeedback build

tgodzik · 2022-04-26T09:04:44Z

I have done the rebase and change of base branch.

Thanks, sorry that I didn't get to it right away!

ljacomet · 2022-04-26T09:10:28Z

No worry, my ability to help steer this was quite spotty 😉

ljacomet

Green build!

Thanks @tgodzik for proposing this and following up even after time passed!

ljacomet · 2022-04-26T12:21:26Z

@bot-gradle test and merge

bot-gradle · 2022-04-26T12:21:43Z

OK, I've already triggered a build for you.

mernst-github · 2022-10-23T18:53:48Z

subprojects/scala/src/main/java/org/gradle/api/internal/tasks/scala/ZincScalaCompiler.java

-                if (!e.getKey().exists() || !e.getValue().equals(Stamper.forLastModified().apply(e.getKey()))) {
+            for (Map.Entry<VirtualFileRef, Stamp> e : previousAnalysis.readStamps().getAllLibraryStamps().entrySet()) {
+                File path = CONVERTER.toPath(e.getKey()).toFile();
+                if (!path.exists() || !e.getValue().equals(Stamper.forLastModifiedInRootPaths(CONVERTER).apply(e.getKey()))) {


Would like to raise attention to this line which I think is incorrect and seems to have caused #20101
e.getValue seems to be a FarmHash, not a LastModified.

TBH I'm not even sure why Gradle provides the ExternalBinariesLookup when it effectively could rely on zinc's change detection in the getOrElse case here: https://github.com/sbt/zinc/blob/develop/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala#L407

tgodzik commented Feb 9, 2022

View reviewed changes

gradle/verification-keyring.keys Show resolved Hide resolved

tgodzik commented Feb 9, 2022

View reviewed changes

...ala/src/integTest/groovy/org/gradle/scala/compile/UpToDateScalaCompileIntegrationTest.groovy Show resolved Hide resolved

tgodzik force-pushed the update-zinc-1.6.1 branch from 885119e to d07d514 Compare February 9, 2022 17:26

tgodzik mentioned this pull request Feb 9, 2022

Scala 2.13 with Zinc 1.4.3 fails with NoSuchMethodError #15491

Closed

dubinsky approved these changes Feb 9, 2022

View reviewed changes

ljacomet self-assigned this Feb 11, 2022

ijuma reviewed Feb 13, 2022

View reviewed changes

subprojects/scala/build.gradle.kts Outdated Show resolved Hide resolved

ljacomet requested changes Feb 22, 2022

View reviewed changes

gradle/verification-keyring.keys Show resolved Hide resolved

gradle/verification-metadata.xml Show resolved Hide resolved

...ala/src/integTest/groovy/org/gradle/scala/compile/UpToDateScalaCompileIntegrationTest.groovy Show resolved Hide resolved

tgodzik force-pushed the update-zinc-1.6.1 branch from a922737 to 7647598 Compare February 22, 2022 14:26

Kotori316 mentioned this pull request Mar 15, 2022

bad option '-target:16' was ignored when compiling scala code with Scala3 library #20180

Closed

big-guy added the from:contributor PR by an external contributor label Apr 1, 2022

hegyibalint mentioned this pull request Apr 6, 2022

Upgrade JNA used for SBT Zinc #20361

Closed

ljacomet reviewed Apr 6, 2022

View reviewed changes

ljacomet added this to the 7.5 RC1 milestone Apr 6, 2022

ljacomet added in:scala-plugin @support Issues owned by GBT support team labels Apr 6, 2022

tgodzik force-pushed the update-zinc-1.6.1 branch 2 times, most recently from aac8b4b to ca4f815 Compare April 15, 2022 16:43

tgodzik force-pushed the update-zinc-1.6.1 branch from ca4f815 to b1d0193 Compare April 15, 2022 16:48

gradle deleted a comment from ljacomet Apr 19, 2022

tgodzik and others added 3 commits April 26, 2022 09:39

Update Zinc used by the Scala plugin to 1.6.1

597d813

Signed-off-by: Tomasz Godzik <tomek.godzik@gmail.com>

Update Zinc artifact to Scala 2.13

fced14f

Signed-off-by: Tomasz Godzik <tgodzik@virtuslab.com>

Fix Scala tests after Zinc updated to use Scala 2.13.x and update docs

e264d9f

Signed-off-by: Tomasz Godzik <tgodzik@virtuslab.com>

Fix IdeaIntegration test

6c8dc28

The issue is most likely related to sbt/zinc#799 Signed-off-by: Tomasz Godzik <tgodzik@virtuslab.com>

ljacomet force-pushed the update-zinc-1.6.1 branch from a4402a2 to 6c8dc28 Compare April 26, 2022 07:40

ljacomet changed the base branch from master to release April 26, 2022 07:40

gradle deleted a comment from ljacomet Apr 26, 2022

Remove left over references to older Zinc versions

584f78e

Issue gradle#15491

gradle deleted a comment from ljacomet Apr 26, 2022

ljacomet added 2 commits April 26, 2022 10:56

Drop unsupported versions from Zinc coverage

031d5c6

Issue gradle#15491

Minor fixes in upgrade notes

43ada16

Issue gradle#15491

gradle deleted a comment from ljacomet Apr 26, 2022

ljacomet approved these changes Apr 26, 2022

View reviewed changes

gradle deleted a comment from ljacomet Apr 26, 2022

bot-gradle merged commit d794efc into gradle:release Apr 26, 2022

mernst-github reviewed Oct 23, 2022

View reviewed changes

tgodzik deleted the update-zinc-1.6.1 branch October 24, 2022 08:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Zinc used by the Scala plugin to 1.6.1 #19864

Update Zinc used by the Scala plugin to 1.6.1 #19864

tgodzik commented Feb 9, 2022 •

edited

tgodzik commented Feb 9, 2022

dubinsky left a comment

ljacomet left a comment

dubinsky commented Mar 23, 2022

ljacomet left a comment

ljacomet commented Apr 6, 2022

tgodzik commented Apr 7, 2022

tgodzik commented Apr 15, 2022

tgodzik commented Apr 25, 2022

ljacomet commented Apr 25, 2022

ljacomet commented Apr 26, 2022

ljacomet commented Apr 26, 2022

bot-gradle commented Apr 26, 2022

tgodzik commented Apr 26, 2022

ljacomet commented Apr 26, 2022

ljacomet left a comment

ljacomet commented Apr 26, 2022

bot-gradle commented Apr 26, 2022

mernst-github Oct 23, 2022

mernst-github Oct 24, 2022

Update Zinc used by the Scala plugin to 1.6.1 #19864

Update Zinc used by the Scala plugin to 1.6.1 #19864

Conversation

tgodzik commented Feb 9, 2022 • edited

Context

Contributor Checklist

Gradle Core Team Checklist

tgodzik commented Feb 9, 2022

dubinsky left a comment

Choose a reason for hiding this comment

ljacomet left a comment

Choose a reason for hiding this comment

dubinsky commented Mar 23, 2022

ljacomet left a comment

Choose a reason for hiding this comment

ljacomet commented Apr 6, 2022

tgodzik commented Apr 7, 2022

tgodzik commented Apr 15, 2022

tgodzik commented Apr 25, 2022

ljacomet commented Apr 25, 2022

ljacomet commented Apr 26, 2022

ljacomet commented Apr 26, 2022

bot-gradle commented Apr 26, 2022

tgodzik commented Apr 26, 2022

ljacomet commented Apr 26, 2022

ljacomet left a comment

Choose a reason for hiding this comment

ljacomet commented Apr 26, 2022

bot-gradle commented Apr 26, 2022

mernst-github Oct 23, 2022

Choose a reason for hiding this comment

mernst-github Oct 24, 2022

Choose a reason for hiding this comment

tgodzik commented Feb 9, 2022 •

edited