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

feat!: Java 21 for build and runtime #7177

Merged
merged 15 commits into from
Jun 6, 2024

Conversation

usmansaleem
Copy link
Member

@usmansaleem usmansaleem commented Jun 5, 2024

PR description

Enforce Java 21 for Besu build and CI. The docker image was already on Java 21. Also fixed javadoc lint related warnings.

Fixed Issue(s)

See #6722

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem added the doc-change-required Indicates an issue or PR that requires doc to be updated label Jun 5, 2024
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem changed the title Java 21 upgrade feat!: Java 21 for build and runtime Jun 5, 2024
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

needs a changelog entry

@macfarla
Copy link
Contributor

macfarla commented Jun 5, 2024

needs a changelog entry

nvm it's already there!

@@ -32,6 +32,9 @@ public class InMemoryTaskQueue<T> implements TaskCollection<T> {
private final Set<InMemoryTask<T>> unfinishedOutstandingTasks = new HashSet<>();
private final AtomicBoolean closed = new AtomicBoolean(false);

/** Default constructor. */
public InMemoryTaskQueue() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a need for a default constructor here ? it should be implicit or there're changes related to generics in Java 21 that makes them mandatory ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ahamlat Hi Ameziane, We have javadoc lint enabled in our gradle build and fail our build on lint warnings. One of the "lint" warning that has been enabled since Java 19 is to provide javadoc on default constructors.

https://bugs.openjdk.org/browse/JDK-8249634

To avoid adding such empty constructors, we have to disable gradle build failure on javadoc lint warnings.

Copy link
Contributor

Choose a reason for hiding this comment

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

My opinion would to disable gradle build failure on javadoc lint warnings, and not add "dead code", but this is not blocking.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had a look, looks like we have -Werror in java compile task, this seems to apply to javadoc task as well.

Copy link
Contributor

@ahamlat ahamlat left a comment

Choose a reason for hiding this comment

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

few comments but nothing really important

@@ -32,6 +32,9 @@ public class InMemoryTaskQueue<T> implements TaskCollection<T> {
private final Set<InMemoryTask<T>> unfinishedOutstandingTasks = new HashSet<>();
private final AtomicBoolean closed = new AtomicBoolean(false);

/** Default constructor. */
public InMemoryTaskQueue() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

My opinion would to disable gradle build failure on javadoc lint warnings, and not add "dead code", but this is not blocking.

CHANGELOG.md Outdated
@@ -3,6 +3,7 @@
## Next Release

### Breaking Changes
- Java 21 is required to build and run Besu.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say : Java 21 is required to build Besu, and at least Java 21 to run it.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍🏼

Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
<artifact name="org.jacoco.agent-0.8.11.pom">
<sha256 value="16e05e9f49621b87c53e69350140f3c46d42d966c67a933bdf4b063a2b1c8fc5" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="org.jacoco" name="org.jacoco.agent" version="0.8.8">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the old jacoco entry be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

<artifact name="asm-commons-9.6.pom">
<sha256 value="a98ae4895334baf8ff86bd66516210dbd9a03f1a6e15e47dda82afcf6b53d77c" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="org.ow2.asm" name="asm-tree" version="9.2">
<artifact name="asm-tree-9.2.jar">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the old asm-tree entry be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

apparently they are still referenced ....

Copy link
Contributor

Choose a reason for hiding this comment

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

so both versions are being used?

Copy link
Member Author

Choose a reason for hiding this comment

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

looks like it, I removed it and gradle started complaining about missing metadata.

 Remove 0.8.8 entries

Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
…asm entries"

This reverts commit 1505585.

Signed-off-by: Usman Saleem <usman@usmans.info>
…n CI

Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem merged commit 2d59f4d into hyperledger:main Jun 6, 2024
40 checks passed
@joaniefromtheblock joaniefromtheblock removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Jun 19, 2024
gtebrean pushed a commit to gtebrean/besu that referenced this pull request Jun 26, 2024
* build: Update jacoco version to 0.8.11
* build: Enforce Java 21 and above check for build
* CI: Use Java 21 in Github CI workflows
* CI: Use Java 21 in circleci workflows
* build: Update gradle verification metadata for jacoco 0.8.11
* refactor: Fix javadoc related warnings which are applicable to Java 21
* fix(test): BackwardSyncAlgSpec slightly increase timeout to pass it in CI

---------

Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: George Tebrean <george@web3labs.com>
gtebrean pushed a commit to gtebrean/besu that referenced this pull request Jun 26, 2024
* build: Update jacoco version to 0.8.11
* build: Enforce Java 21 and above check for build
* CI: Use Java 21 in Github CI workflows
* CI: Use Java 21 in circleci workflows
* build: Update gradle verification metadata for jacoco 0.8.11
* refactor: Fix javadoc related warnings which are applicable to Java 21
* fix(test): BackwardSyncAlgSpec slightly increase timeout to pass it in CI

---------

Signed-off-by: Usman Saleem <usman@usmans.info>
@usmansaleem usmansaleem deleted the java_21_upgrade branch June 28, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants