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

Update to JDK 17 #7930

Closed
1 of 5 tasks
korthout opened this issue Oct 5, 2021 · 17 comments
Closed
1 of 5 tasks

Update to JDK 17 #7930

korthout opened this issue Oct 5, 2021 · 17 comments
Assignees
Labels
kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. version:1.3.0 Marks an issue as being completely or in parts released in 1.3.0

Comments

@korthout
Copy link
Member

korthout commented Oct 5, 2021

Description

In my tech talk Preparing for JDK 17 I've highlighted some of the advantages for Zeebe to update to JDK 17. It also discussed the necessary steps that need to be taken:

@korthout korthout added the kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. label Oct 5, 2021
@npepinpe
Copy link
Member

npepinpe commented Oct 5, 2021

This is one OKR we have for this quarter :)

@korthout
Copy link
Member Author

korthout commented Oct 5, 2021

For the problem with the fmt-maven-plugin, I want to propose to the switch to diffplug/spotless, which also has a maven plugin. Main reason for this proposal is that the fmt-maven-plugin seems unmaintained and there was some talk about transferring ownership, but it has not yet happened. It seems unlikely a solution for JDK 17 will be provided soon for the fmt-plugin. Secondary, spotless supports ratcheting which might improve execution speed and it could also allow us to provide formatting of our markdown files, as discussed in #7508

@npepinpe npepinpe added this to Planned in Zeebe Oct 7, 2021
@oleschoenburg
Copy link
Member

When upgrading maven-dependency-plugin to version 3.2.0 we run into this bug: https://issues.apache.org/jira/browse/MDEP-773

@korthout
Copy link
Member Author

korthout commented Oct 15, 2021

@oleschoenburg That seems like a similar issue as MDEP-753 (linked above). There's a workaround for that one, did you already try using it with a hard dependency to maven-dependency-analyzer 1.11.3? EDIT: There is also some new activity since today in that issue. I recommend subscribing to it to be notified of a potential new release.

@oleschoenburg
Copy link
Member

oleschoenburg commented Oct 15, 2021

@korthout Yes, I've tried it with the workaround from MDEP-753 but it's still running into the same issues:

[INFO] --- maven-dependency-plugin:3.2.0:analyze-only (analyze-dependencies) @ zeebe-util ---
[WARNING] Non-test scoped test only dependencies found:
[WARNING]    com.fasterxml.jackson.core:jackson-databind:jar:2.13.0:compile
[WARNING]    org.apache.logging.log4j:log4j-core:jar:2.14.1:compile
[WARNING]    org.agrona:agrona:jar:1.12.0:compile
[WARNING]    com.fasterxml.jackson.core:jackson-core:jar:2.13.0:compile
[WARNING]    org.apache.logging.log4j:log4j-api:jar:2.14.1:compile

EDIT: According to this comment there is no workaround for this.

@npepinpe npepinpe moved this from Planned to Ready in Zeebe Oct 18, 2021
@korthout
Copy link
Member Author

@oleschoenburg To make sure we don't forget. Recently, we've added some documentation that mentions the specific JDK version required by Zeebe. We should make sure to update this documentation when we do update. See https://docs.camunda.io/docs/components/zeebe/deployment-guide/interceptors/

@oleschoenburg
Copy link
Member

There seems to be another compilation issue in relation to org.immutables:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project zeebe-protocol-jackson: Compilation failure: Compilation failure: 
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/DeploymentRecordValueBuilder.java:[127,32] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/DeploymentRecordValueBuilder.java:[174,32] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/JobBatchRecordValueBuilder.java:[289,32] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/JobBatchRecordValueBuilder.java:[417,34] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/JobBatchRecordValueBuilder.java:[535,36] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/DeploymentRecordValueBuilder.java:[233,34] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/DeploymentRecordValueBuilder.java:[243,34] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/DeploymentRecordValueBuilder.java:[311,36] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>
[ERROR] /Users/ole/Source/zeebe/protocol-jackson/target/generated-sources/annotations/io/camunda/zeebe/protocol/jackson/record/DeploymentRecordValueBuilder.java:[316,36] incompatible types: java.lang.String cannot be converted to java.lang.Class<?>

The offending generated code looks like this:

@Generated(from = "AbstractDeploymentRecordValue", generator = "Immutables")
public final class DeploymentRecordValueBuilder {

  /**
   * Sets or replaces all elements for {@link AbstractDeploymentRecordValue#getResources() resources} list.
   * @param elements An iterable of resources elements
   * @return {@code this} builder for use in a chained invocation
   */
  @JsonProperty("resources")
  @JsonDeserialize(contentAs = "<error>")
  public final DeploymentRecordValueBuilder resources(Iterable<? extends DeploymentResource> elements) {
    this.resources.clear();
    return addAllResources(elements);
  }
}

Upgrading immutables from 2.8.9-ea-1 to 2.9.0-rc1 did not resolve this issue.

@npepinpe
Copy link
Member

It's likely this module will not be updated to Java 17 for the time being, partially because it may end up being used by users/customers in the near future. So we can leave it to Java 11 for now.

@npepinpe
Copy link
Member

One small note: once we've broken down into multiple issues the topic, we can close this issue imo. If you want to keep it as a kind of epic issue, then we can put it back in the backlog.

@oleschoenburg
Copy link
Member

I've created an issue with a minimal reproducer for the immutables problem: immutables/immutables#1339

@npepinpe
Copy link
Member

It looks like 2.9.0-rc1 was released a bit over a month ago (Sep. 25), and 20 days ago (so on Sep. 29) a PR was merged for Java 17 support, so maybe it's fixed on master?

@oleschoenburg
Copy link
Member

No, unfortunately this doesn't fix the issue.

@npepinpe
Copy link
Member

Regarding the format plugin, I got it to work in another project with the following:

      <plugin>
        <groupId>com.coveo</groupId>
        <artifactId>fmt-maven-plugin</artifactId>
        <version>${fmt-maven-plugin.version}</version>
        <dependencies>
          <dependency>
            <groupId>com.google.googlejavaformat</groupId>
            <artifactId>google-java-format</artifactId>
            <version>1.11.0</version>
          </dependency>
        </dependencies>
        <executions>
          <execution>
            <id>format</id>
            <goals>
              <goal>format</goal>
            </goals>
            <phase>process-sources</phase>
          </execution>
        </executions>
      </plugin>

And checking into the project the following .mvn/jvm.config:

--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

Hopefully the next releases simplify this 😄

@oleschoenburg
Copy link
Member

@npepinpe The fmt-maven-plugin works fine (for the Zeebe project) without the explicit dependency on google-java-format, it only requires the exports.

@npepinpe npepinpe moved this from Ready to In progress in Zeebe Oct 20, 2021
@oleschoenburg
Copy link
Member

We have a workaround for the immutables issue: immutables/immutables#1339 (comment)

@npepinpe npepinpe moved this from In progress to Planned in Zeebe Oct 21, 2021
@npepinpe
Copy link
Member

Putting this back in planned as I see you will work on single issues.

ghost pushed a commit that referenced this issue Nov 4, 2021
8117: Replace fmt-plugin with Spotless r=korthout a=korthout

## Description

<!-- Please explain the changes you made here. -->

Replaces the `com.coveo:fmt-maven-plugin` plugin with [spotless](https://github.com/diffplug/spotless/tree/main/plugin-maven) and cleans up our build profiles regarding auto-formatting and formatting checks.

The fmt-plugin seems to no longer be maintained, or at least it is no longer maintained as well as other plugins. See #7930 (comment). 

Switching to spotless means switching to an actively maintained project. Using it allows us to update the google-format to v1.12. Spotless works on our jdk8 build. Spotless supports [ratcheting](https://github.com/diffplug/spotless/tree/main/plugin-maven#how-can-i-enforce-formatting-gradually-aka-ratchet) to speed up our builds (~10s per build on our local measurements, should be at least 4 hours per year for our team 🎉 ) and might allow us to replace the different plugins we use related to formatting (sortpom, license, etc) with 1 plugin. We could even add markdown support in the future with some additional effort from our side.

This PR switches the fmt-plugin for the spotless plugin, updates the google-format to v1.12.0, enables ratcheting for faster local builds and disables the auto formatting on our CI pipeline.

It makes sense to review this PR on a per-commit basis.

## Related issues

<!-- Which issues are closed by this PR or are related -->

This is the result of slacktime for @oleschoenburg and @korthout.

relates to #7930 
relates to #7508 



Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
@npepinpe npepinpe added this to the Migrate to Java 17 milestone Nov 7, 2021
@oleschoenburg
Copy link
Member

I'm closing this for now. The only remaining step is to upgrade modules to language level 17 which is tracked here: #8030

Zeebe automation moved this from Planned to Done Nov 11, 2021
@korthout korthout added the version:1.3.0 Marks an issue as being completely or in parts released in 1.3.0 label Jan 4, 2022
@KerstinHebel KerstinHebel removed this from Done in Zeebe Mar 23, 2022
saig0 added a commit to camunda-community-hub/eze that referenced this issue May 11, 2022
* make the fmt-maven-plugin work with Java 17
* follow instructions of camunda/camunda#7930 (comment)
saig0 added a commit to camunda-community-hub/eze that referenced this issue May 12, 2022
* make the fmt-maven-plugin work with Java 17
* follow instructions of camunda/camunda#7930 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. version:1.3.0 Marks an issue as being completely or in parts released in 1.3.0
Projects
None yet
Development

No branches or pull requests

3 participants