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

Upgrade JDKs used by GitHub Actions builds #1329

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

Stephan202
Copy link
Member

@Stephan202 Stephan202 commented Sep 14, 2024

Suggested commit message:

Upgrade JDKs used by GitHub Actions builds (#1329)

Summary of changes:
- Use JDK 17.0.13 instead of 17.0.10.
- Use JDK 21.0.5 instead of 21.0.2.
- Use JDK 23.0.1 instead of 22.0.2.
- Have GitHub issue template reference more recent version numbers.

See:
- https://adoptium.net/temurin/release-notes/?version=jdk-17.0.11+9
- https://adoptium.net/temurin/release-notes/?version=jdk-17.0.12+7
- https://adoptium.net/temurin/release-notes/?version=jdk-17.0.13+11
- https://adoptium.net/temurin/release-notes/?version=jdk-21.0.3+9
- https://adoptium.net/temurin/release-notes/?version=jdk-21.0.4+7
- https://adoptium.net/temurin/release-notes/?version=jdk-21.0.5+11
- https://adoptium.net/temurin/release-notes/?version=jdk-23+37
- https://adoptium.net/temurin/release-notes/?version=jdk-23.0.1+11

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202
Copy link
Member Author

I would have expected an Error Prone incompatibility, but in fact it's an OpenRewrite incompatibility 😅. @timtebeek FYI:

Caused by: java.lang.NoSuchMethodError: 'com.sun.tools.javac.tree.DCTree$DCDocComment com.sun.tools.javac.tree.DocCommentTable.getCommentTree(com.sun.tools.javac.tree.JCTree)'
	at org.openrewrite.java.isolated.ReloadableJava11ParserVisitor.convert(ReloadableJava11ParserVisitor.java:1590)

JDK 23 will be released later this month, so perhaps worth looking into. (My schedule is packed the coming weeks, so unfortunately I won't have time to dive in myself.)

@timtebeek
Copy link
Contributor

JDK 23 will be released later this month, so perhaps worth looking into. (My schedule is packed the coming weeks, so unfortunately I won't have time to dive in myself.)

Understandable; the usage is here indeed (and in the Java 8/11/17 parser visitors): https://github.com/openrewrite/rewrite/blob/a98d83d698dda9971c93a0767b7f85d5866f306d/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21ParserVisitor.java#L1666

Adding some links for context:

Can't immediately find any public mention of the changes to DocCommentTable, so it seems we'd need to install 23-ea to explore further and see what's available. Annoying that SDKman doesn't yet list a 23-ea release. 🤔

/cc @knutwannheden

As an aside you might want to change these lines to use rewrite-java-17, or a version matching the runtime Java version, as you're right now getting an error from our Java 11 parser, but a fix is likely to land in a newer parser version instead.

<dependency>
<groupId>org.openrewrite</groupId>
<artifactId>rewrite-java-11</artifactId>
<scope>test</scope>
</dependency>

@Stephan202
Copy link
Member Author

As an aside you might want to change these lines to use rewrite-java-17, or a version matching the runtime Java version, as you're right now getting an error from our Java 11 parser, but a fix is likely to land in a newer parser version instead.

Ack! I filed #1330.

@Stephan202 Stephan202 force-pushed the sschroevers/prepare-for-jdk-23-release branch from 4d862e0 to b67e7fc Compare September 16, 2024 06:39
Copy link

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@timtebeek
Copy link
Contributor

timtebeek commented Sep 23, 2024

We'll do another release this Wednesday; should help to see if this worked. :)

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 changed the title Build against early access JDK 23 release Upgrade JDKs used by GitHub Actions builds Oct 27, 2024
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 added the chore A task not related to code (build, formatting, process, ...) label Oct 27, 2024
@Stephan202
Copy link
Member Author

Updated the PR; build should pass once openrewrite/rewrite#4618 is released.

@timtebeek
Copy link
Contributor

Updated the PR; build should pass once openrewrite/rewrite#4618 is released.

I've just released https://github.com/openrewrite/rewrite/releases/tag/v8.38.1 ;
That's on top of yesterday's https://github.com/openrewrite/rewrite-templating/releases/tag/v1.16.3
Do let me know how those work out for you!

@Stephan202
Copy link
Member Author

Nice! I expect Renovate to open the upgrade PR tonight. 🌚

@Stephan202 Stephan202 added this to the 0.19.0 milestone Oct 28, 2024
@Stephan202 Stephan202 force-pushed the sschroevers/prepare-for-jdk-23-release branch from ceae66c to c000ea5 Compare October 29, 2024 06:54
@Stephan202
Copy link
Member Author

Nice! I expect Renovate to open the upgrade PR tonight. 🌚

Nope, cause we pull it in via rewrite-recipe-bom, for which no release happened yet. For now I added an upgrade commit to this branch; to be dropped again later. Build should now pass.... 👀

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 marked this pull request as ready for review October 29, 2024 07:13
Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📈 📈 📈

- Error Prone Support version (e.g. `0.15.0`).
- Java version (i.e. `java --version`, e.g. `17.0.13`).
- Error Prone version (e.g. `2.35.1`).
- Error Prone Support version (e.g. `0.18.0`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's bump to 0.19.0 already? As we are gonna release that soon :).

@rickie
Copy link
Member

rickie commented Oct 29, 2024

Nice to see the fixes @timtebeek 🚀 !

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the sschroevers/prepare-for-jdk-23-release branch from d7f5770 to d56d7f5 Compare October 29, 2024 09:15
@rickie rickie force-pushed the sschroevers/prepare-for-jdk-23-release branch from d56d7f5 to 8b945d4 Compare October 29, 2024 09:16
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

1 similar comment
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

@rickie rickie merged commit 507d759 into master Oct 29, 2024
16 checks passed
@rickie rickie deleted the sschroevers/prepare-for-jdk-23-release branch October 29, 2024 09:26
@timtebeek
Copy link
Contributor

Thanks all! Great to see the fix land here; we release OpenRewrite every two weeks, with the next one the day before JFall to remove the temporary override added here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A task not related to code (build, formatting, process, ...)
Development

Successfully merging this pull request may close these issues.

4 participants