Skip to content

feat!: Java 21 for build and runtime#7177

Merged
usmansaleem merged 15 commits intobesu-eth:mainfrom
usmansaleem:java_21_upgrade
Jun 6, 2024
Merged

feat!: Java 21 for build and runtime#7177
usmansaleem merged 15 commits intobesu-eth:mainfrom
usmansaleem:java_21_upgrade

Conversation

@usmansaleem
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
Contributor

macfarla commented Jun 5, 2024

needs a changelog entry

nvm it's already there!

private final AtomicBoolean closed = new AtomicBoolean(false);

/** Default constructor. */
public InMemoryTaskQueue() {}
Copy link
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
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

private final AtomicBoolean closed = new AtomicBoolean(false);

/** Default constructor. */
public InMemoryTaskQueue() {}
Copy link
Copy Markdown
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.

Comment thread CHANGELOG.md Outdated
## Next Release

### Breaking Changes
- Java 21 is required to build and run Besu.
Copy link
Copy Markdown
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
Copy Markdown
Contributor 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>
Comment thread gradle/verification-metadata.xml Outdated
<sha256 value="16e05e9f49621b87c53e69350140f3c46d42d966c67a933bdf4b063a2b1c8fc5" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="org.jacoco" name="org.jacoco.agent" version="0.8.8">
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done!

</artifact>
</component>
<component group="org.ow2.asm" name="asm-tree" version="9.2">
<artifact name="asm-tree-9.2.jar">
Copy link
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor 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 besu-eth:main Jun 6, 2024
@joaniekube joaniekube 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