-
Notifications
You must be signed in to change notification settings - Fork 861
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
Conversation
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>
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>
There was a problem hiding this 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
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() {} |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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() {} |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
gradle/verification-metadata.xml
Outdated
<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"> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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"> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ....
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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>
* 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>
* 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>
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?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests