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.

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

Java/Scala build with no explicit toolchain: build fails with Gradle 8.0.1 / Scala 2.13 #23962

Closed
dejan2609 opened this issue Feb 18, 2023 · 11 comments · Fixed by #24059
Closed
Assignees
Labels
Milestone

Comments

@dejan2609
Copy link

dejan2609 commented Feb 18, 2023

Expected Behavior

Build should work with Gradle 8.0.1 and Scala 2.13

Current Behavior

Context

We are trying to upgrade Apache Kafka Gradle build (from 7 to 8) and we are facing issues with Gradle 8.0.1/Scala 2.13:

Compatibility matrix (for a PR mentioned above):

Gradle version 8.0.1:

Scala 2.12.15 Scala 2.13.10
JDK 8 https://scans.gradle.com/s/55xpvswy3mp5a
JDK 11 https://scans.gradle.com/s/6txyrxfjt7ive
JDK 17 https://scans.gradle.com/s/fbbdzt2zqls6e

Gradle version 8.0: (just move PR head to a previous commit like this: git checkout HEAD~1)

Scala 2.12.15 Scala 2.13.10
JDK 8
JDK 11
JDK 17

Gradle patch 8.0.1 changed some things related to Scala compile options, see this changes/issues:

Gradle documentation for Scala plugin 8.0 vs. 8.0.1:

When running the Scala compile task, Gradle will always add a parameter to configure the Java target for the Scala compiler that is derived from the Gradle configuration:

  • When using toolchains, the -release option, or target for older Scala versions, is selected, with a version matching the Java language level of the toolchain configured.
  • When not using toolchains, Gradle will always pass a target flag — with exact value dependent on the Scala version — to compile to Java 8 bytecode.

FYI @ljacomet Given all facts mentioned above chances are that this is a regression, but I will still wait for a triage and review by your team.

Notes:

Steps to Reproduce

  • checkout Apache Kafka PR mentioned above: git clone https://github.com/dejan2609/kafka.git
  • cd kafka
  • git checkout gradle-8
  • decide on Scala version (2.12 or 2.13) that you want to build against like this:
    • ./gradlew -PscalaVersion=2.12 clean jar
    • ./gradlew -PscalaVersion=2.13 clean jar
  • change JDK version and repeat last step

Environment

  • Java versions on Windows:
openjdk version "1.8.0_362"
OpenJDK Runtime Environment (Temurin)(build 1.8.0_362-b09)
OpenJDK 64-Bit Server VM (Temurin)(build 25.362-b09, mixed mode)
openjdk version "11.0.18" 2023-01-17
OpenJDK Runtime Environment Temurin-11.0.18+10 (build 11.0.18+10)
OpenJDK 64-Bit Server VM Temurin-11.0.18+10 (build 11.0.18+10, mixed mode)
openjdk version "17.0.6" 2023-01-17
OpenJDK Runtime Environment Temurin-17.0.6+10 (build 17.0.6+10)
OpenJDK 64-Bit Server VM Temurin-17.0.6+10 (build 17.0.6+10, mixed mode, sharing)
  • Java versions on Linux:
dejan@dejan-HP-ProBook-450-G7:~$ apt list --installed | grep openjdk | grep -v jre | grep -v headless

WARNING: apt does not have a stable CLI interface. Use with caution in scripts.

openjdk-11-jdk/jammy-updates,jammy-security,now 11.0.17+8-1ubuntu2~22.04 amd64 [installed]
openjdk-17-jdk/jammy-updates,jammy-security,now 17.0.5+8-2ubuntu1~22.04 amd64 [installed]
openjdk-8-jdk/jammy-updates,jammy-security,now 8u352-ga-1~22.04 amd64 [installed]
dejan@dejan-HP-ProBook-450-G7:~$
  • OS:
    • Windows:
        C:\>systeminfo | findstr /B /C:"OS Name" /C:"OS Version"
        OS Name:                   Microsoft Windows 11 Pro
        OS Version:                10.0.22621 N/A Build 22621
        
        C:\>
  • Linux:
          dejan@dejan-HP-ProBook-450-G7:~$ lsb_release -a 
          No LSB modules are available.
          Distributor ID:	Ubuntu
          Description:	Ubuntu 22.04.2 LTS
          Release:	22.04
          Codename:	jammy
          dejan@dejan-HP-ProBook-450-G7:~$
@dejan2609 dejan2609 added a:regression This used to work to-triage labels Feb 18, 2023
@dejan2609 dejan2609 changed the title Java/Scala build with no explicit toolchain: build works fine with Gradle 8.0 and fails with 8.0.1 Java/Scala build with no explicit toolchain: build fails with Gradle 8.0.1 / Scala 2.13 Feb 19, 2023
@dejan2609
Copy link
Author

dejan2609 commented Feb 19, 2023

A note for a Gradle team: I changed issue description above (too) many times, but now it is finalized and ready for a review

I had to make few more changes (somehow I managed to lose build scan URLs, so I had to create them again)

Feel free to review now 🎉

@ljacomet
Copy link
Member

ljacomet commented Feb 20, 2023

Thanks for the report.

We decided that Gradle 8 would introduce potential breaking changes with regards to Scala compile options. However the way it was done in Gradle 8.0 was too much of a breakage.

In your case, I believe that Gradle 8.0.1 strikes the right balance but it means your build needs a change.

Gradle 8.0.1 fails because you have -Werror which makes the Scala compiler warning an error AFAICT. In order to work around that, you need to follow the recommendation from the new documentation section you linked:

Setting any of these flags explicitly, or using flags containing java-output-version, on ScalaCompile.scalaCompileOptions.additionalParameters disables that logic in favor of the explicit flag.

@augi
Copy link

augi commented Feb 20, 2023

Hello, the build also doesn't work for us, Gradle 8.0.1, Scala 2.13.10. Still getting [Error] : -target is deprecated: Use -release instead to compile against the correct platform API..

The documentation mentions that if one of the parameters is spefied then the logic is disabled, but this doesn't seem to be working, because we tried the following and still getting the same message (so -target is still being set automatically).

tasks.withType(ScalaCompile).configureEach { it.scalaCompileOptions.additionalParameters.addAll(['-release', '8']) }

We tried also --release that is also allowed.

@ljacomet
Copy link
Member

So it looks like the logic of Gradle for detecting the options has gaps.

We use the following:

private static final List<String> TARGET_DEFINING_PARAMETERS = Arrays.asList(
// Scala 2
"-target:", "--target:",
// Scala 2 and 3
"-release:", "--release:",
// Scala 3
"-Xtarget:", "-java-output-version:", "-Xunchecked-java-output-version:"
);

But that accounts only for -release:11 not -release 11.

Can you confirm this works if you move from space separated to : separated in your configuration?

@dejan2609
Copy link
Author

dejan2609 commented Feb 21, 2023

Few things to unpack:

  1. I just pushed another commit to Apache Kafka PR mentioned above (that temporarily comments out Java compiler argument Werror and Scala compiler argument -release (build contains no -target Scala arguments).

I assume that according to https://docs.gradle.org/8.0.1/userguide/scala_plugin.html#sec:scala_target paragraph

When not using toolchains, Gradle will always pass a target flag — with exact value dependent on the Scala version — to compile to Java 8 bytecode

and given a fact that no toolchains are used Gradle should just add appropriate target parameter.

However, results are still the same (environment: JDK 8 / Gradle 8.0.1):

  • ./gradlew -PscalaVersion=2.12 jar ✅ (Scala version is resolved to 2.12.15)
  • ./gradlew -PscalaVersion=2.13 jar ❌ (Scala version is resolved to 2.13.10)
> Task :streams:streams-scala:compileScala FAILED
[Error] : -target is deprecated: Use -release instead to compile against the correct platform API.
one error found
  1. even with -target:8 explicitly added into Scala compile options build fails with Scala 2.13.10 (same as above)

@ljacomet I opted to keep this comment as short as possible, let me know in case you need some more details, build scans, etc.


Note: Apache Kafka for JDK 8 builds avoids to use-release flag (and its alias java-output-version):

@ljacomet
Copy link
Member

Hi @dejan2609,

I was mistaken on blaming -Werror which is set for JavaCompile and not ScalaCompile. For the later, you set -Xfatal-warnings here. This is the reason why -target is deprecated: ... appears as Error and not Warn.

But I see that we might be in a bad position here if:

  • -release cannot be used if the JVM is not at least Java 9
  • -target 8 is tripping a deprecation warning for Scala 2.13.9+

However looking at the code you linked for Scala 2.13.10, -release 8 is OK for a Java 8 VM. And I confirmed that locally with a test.
But I also see that Scala 2.12 does not behave the same, if you pass -release 8, it prints the following:
-release is only supported on Java 9 and higher but compiles the code fine. However, using -target:jvm1.8 is not an issue there, meaning it is not deprecated.

I am sorry this is such a mess. I don't think that changing this further in patch releases make sense however. We might need a longer time to come up with a proper integration that respects all of these exceptions while not breaking users more. And given how changes to these flags appear in patch versions of Scala, I am not even sure we can come up with a scheme that is future proof.

@ijuma
Copy link
Contributor

ijuma commented Feb 21, 2023

@dejan2609 We can change the Kafka build to always set the release flag with Scala 2.13 and conditionally set it with Scala 2.12 & Java >= 9. And source//target should only be set if release is not set. Hopefully that works with Gradle 8.0.1.

@dejan2609
Copy link
Author

@ijuma Makes sense. I went the other way and just pushed (like, minutes ago 😃) different draft solution (suppresion for one specific Scala compiler warning).
Lets continue discussion on Apache Kafka PR.

@ljacomet I understand you point: you have to support quite a big matrix of JDK versions / Scala versions / toolchan and no-toolchain situations and on top of that comes plethora of compiler options; really hectic situation, to put it mildly.

Kudos for your help 👍 and feel free to navigate this situation in whatever way you see fit.

@ljacomet
Copy link
Member

Thanks for your understanding and happy to hear you have a solution.

Will keep this issue open for now to track any improvement we can do on the Gradle side for this.

@augi
Copy link

augi commented Feb 22, 2023

So it looks like the logic of Gradle for detecting the options has gaps.

We use the following:

private static final List<String> TARGET_DEFINING_PARAMETERS = Arrays.asList(
// Scala 2
"-target:", "--target:",
// Scala 2 and 3
"-release:", "--release:",
// Scala 3
"-Xtarget:", "-java-output-version:", "-Xunchecked-java-output-version:"
);

But that accounts only for -release:11 not -release 11.

Can you confirm this works if you move from space separated to : separated in your configuration?

Hi, I guess the problem is that target and release have a different syntax, -target:8 vs. -release 8. See the docs: https://docs.scala-lang.org/overviews/compiler-options/index.html

dejan2609 added a commit to dejan2609/kafka that referenced this issue Feb 23, 2023
Gradle 8 related links:
* https://github.com/gradle/gradle/releases/tag/v8.0.0
* https://github.com/gradle/gradle/releases/tag/v8.0.1
* https://docs.gradle.org/8.0.1/userguide/upgrading_version_7.html#changes_8.0

notes:
* Javac and Scalac options are reshuffled (workaround for Gradle 8.0.1 bug: gradle/gradle#23962 (comment))
* spotless plugin task reference is removed (newer plugin versions require Java 11, so we can't use them until Kafka 4.0); plugin configuration is kept
* jacoco version is bumped: 0.8.7 -->> 0.8.8 https://docs.gradle.org/8.0.1/userguide/jacoco_plugin.html#sec:configuring_the_jacoco_plugin
bot-gradle added a commit that referenced this issue Feb 28, 2023
…ady configured in parameters

Fixes #23962

When deciding whether to supply Java target from toolchain during Scala compilation configuration, we now detect more cases.

More specifically, new cases include specifying a parameter and its value as separate values:
```groovy
tasks.withType(ScalaCompile).configureEach {
    it.scalaCompileOptions.additionalParameters.addAll(['-release', '8'])
}
```

An additional target flag variant for Scala 3 (`--Xtarget`) is also detected.

Support for these flags in different minor releases of Scala varies, but we need to detect as many variants as possible to avoid overriding the target or release. The sources for Scala flags:
- [Scala 2.12.17](https://github.com/scala/scala/blob/v2.13.10/src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala#L54-L75)
- [Scala 2.13.10](https://github.com/scala/scala/blob/v2.12.17/src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala#L59-L82)
- [Scala 3.2.2](https://github.com/lampepfl/dotty/blob/3.2.2/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala#L112)

Co-authored-by: Alex Semin <asemin@gradle.com>
@ljacomet ljacomet added this to the 8.0.2 (unconfirmed) milestone Feb 28, 2023
@ljacomet ljacomet assigned alllex and unassigned ljacomet Feb 28, 2023
@ljacomet
Copy link
Member

ljacomet commented Mar 1, 2023

Improvements will be released in 8.0.2 which makes Gradle detect better when a flag was passed by hand, short circuiting the logic of Gradle itself.

fabianhjr added a commit to LibreCybernetics/flix that referenced this issue Jun 8, 2023
Release flag had to be added manually, additional context
gradle/gradle#23962
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants