scala plugin joint compilation succeed without producing bytecode files

[apflieger]

Expected Behavior

Having a scala project and using joint compilation. After deleting a class file (bytecode) in build/, when re-running the compilation, the task succeed but the class file is not generated.

Steps to Reproduce

I wrote a sample project : https://github.com/apflieger/gradle-incremental-build

10:29:27 ~/src/gradle-incremental-build refs/heads/master $ ./gradlew classes

BUILD SUCCESSFUL in 2s
1 actionable task: 1 executed
10:29:32 ~/src/gradle-incremental-build refs/heads/master $ ls build/classes/scala/main/base/Main.class
build/classes/scala/main/base/Main.class
10:29:43 ~/src/gradle-incremental-build refs/heads/master $ rm build/classes/scala/main/base/Main.class
10:29:48 ~/src/gradle-incremental-build refs/heads/master $ ./gradlew classes

BUILD SUCCESSFUL in 2s
1 actionable task: 1 executed
10:29:57 ~/src/gradle-incremental-build refs/heads/master $ ls build/classes/scala/main/base/Main.class
ls: build/classes/scala/main/base/Main.class: No such file or directory

Even with multiple run of ./gradlew classes, the class isn't outputted. The task is considered up-to-date.

Environment

./gradlew -version

Gradle 6.6-rc-3

Build time: 2020-07-24 14:04:09 UTC
Revision: 13b5dcc

Kotlin: 1.3.72
Groovy: 2.5.12
Ant: Apache Ant(TM) version 1.10.8 compiled on May 10 2020
JVM: 14.0.1 (Oracle Corporation 14.0.1+7)
OS: Mac OS X 10.15.6 x86_64

java -version
openjdk version "14.0.1" 2020-04-14
OpenJDK Runtime Environment (build 14.0.1+7)
OpenJDK 64-Bit Server VM (build 14.0.1+7, mixed mode, sharing)

[ljacomet]

@SheliakLyr Do you know if the incremental compiler in Zinc has mechanism to detect that the output was messed up? And do a full recompile in that case?

[SheliakLyr]

I think that Zinc supports this fully: it will compile incrementally only missing files. That's how my sample sbt project works, and the code responsible for this is in zinc. See doc for sbt.internal.inc.IncrementalCommon.detectInitialChanges:

   * The logic of this method takes care of the following tasks:
   *
   * 1. Detecting the sources that changed between the past and present compiler iteration.
   * 2. Detecting the removed products based on the stamps from the previous and current products. (!!)
   * 3. Detects the class names changed in a library (classpath entry such as jars or analysis).
   * 4. Computes the API changes in dependent and external projects.

I guess products mentioned in 2 is the same thing as outputs.

That method is used in sbt.internal.inc.Incremental.compile. For my sbt project, removed products are correctly detected. For sample project provided by @apflieger, removed products are not detected.

The difference between sbt & gradle is that sbt provides an implementation of the ExternalHooks.Lookup trait that feeds zinc the information about removed products (classes). Not sure how exactly this works and why zinc cannot do this on its own.

The solution: provide an implementation of the ExternalHooks.Lookup trait and inject it into IncOptions.

[SheliakLyr]

My last comment is no longer relevant. Newer sbt/zinc work differently. I suspect that upgrade to zinc 1.4.4 will solve this and #15961 (possible duplicate).

Upgrade to newer zinc is blocked by #15491, but I made necessary changes locally and with zinc 1.4.4 missing classes are correctly restored. Further tests are needed to verify this works correctly.

[ijuma]

@SheliakLyr Are you planning to submit the local changes you have made as a PR?

[SheliakLyr]

No, I have only tested it to check what causes the issue. My changes (very rough) are here: https://github.com/SheliakLyr/gradle/tree/newZinc_1.4.4.

If no one from gradle is going to tackle those 3 related issues, I could work on a proper PR - just let me know. I have seen some initial work by @big-guy in #15961, which made me think this topic is already being worked upon.

[ijuma]

Let's see what @big-guy says, but I assume they would appreciate contributions.

[ljacomet]

@SheliakLyr A contribution to fix this and the related issues would be very welcome. The Gradle team currently has very limited capacity for this unfortunately.

[SheliakLyr]

Ok, I would like to inquire about one topic: compatibility with zinc. Is it possible to drop support for zinc <= 1.5.0 in gradle 7.x? Or should I build some reflection-based layer of compatibility between gradle and zinc in order to keep the support for older versions?

Arguments for using only zinc 1.5.0:

• Simpler, no reflection is needed.
• Having ability to change zinc version per build/module is cool but in practice may cause unexpected problems and is harder to maintain.
• Newer versions of zinc work better (this issue is an example of that)

Arguments for keeping support for older zincs:

• Users can set older version if 1.5.0 introduces any regressions.
• There may be some unusual cases when specific version of zinc is needed. Maybe Scala 2.13 with Zinc 1.4.3 fails with NoSuchMethodError #15491 requires 1.4.3?

BTW, I asked about 1.5.0 because zinc 1.4.x seems kind of broken. zinc_2.12:1.4.4 has indirect dependency on scala 2.13 :(

IMHO: I would drop support for older zincs and limit the ability to change zinc version (only bugfixes, ex. 1.5.0 -> 1.5.1).

[ijuma]

I think it makes the most sense to go with the latest zinc version and support it well versus supporting multiple versions poorly.

[ljacomet]

Thanks for the answer and the proposal. I now wish we had that conversation two weeks ago, selecting Zinc 1.5 for Gradle 7.0 would have been an easy choice.
Let me check internally on the compatibility question and get back to you.

[ljacomet]

@SheliakLyr Sorry for taking so long to get back to you.
If you are still interested, a contribution to fix this issue would be most welcome. Ideally we would like to not drop support for older Zinc version but as commented by Ismael having compilation produce the correct output is the first goal.

[SheliakLyr]

I guess this is my turn to apologize for the delay :)

I think this issue was solved without the need to upgrade zinc to newer version by @rieske in 4990228. I still have my branch (with zinc upgrade), I will look at other issues and decide what to do with it.

[ljacomet]

No need to apologize, especially when you are a community contributor 😉

Given your feedback, I am closing this issue and marking it fixed in Gradle 7.2.

And contributions on other issues, Scala related or not, will be appreciated.