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

Refactor Besu custom error prone dependency #6692

Merged
merged 41 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
06bc097
Use external custom error prone dependency
usmansaleem Mar 7, 2024
c59b33e
spotless fix
usmansaleem Mar 7, 2024
3ad6fb4
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 7, 2024
c3be7b4
custom errorprone check fail test
usmansaleem Mar 7, 2024
613ac16
testing errorprone checks
usmansaleem Mar 7, 2024
c8d2e0f
Using latest version of google errorprone
usmansaleem Mar 9, 2024
e33b131
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 11, 2024
9e43c2a
errorprone reported fix
usmansaleem Mar 12, 2024
f65fd38
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 12, 2024
6283277
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 13, 2024
28847c2
Add suppression for ComparisonOutOfRange
usmansaleem Mar 18, 2024
06e8a21
Resolve errorprone locally
usmansaleem Mar 18, 2024
1f18502
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 18, 2024
652a840
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 25, 2024
d8ac80e
build: Update besu errorprone library version
usmansaleem Mar 25, 2024
8536cc0
chore: Revert changes to StorageRangeDataRequest
usmansaleem Mar 25, 2024
abc5e43
chore: Remove mavanLocal used in local testing
usmansaleem Mar 25, 2024
9c9bee7
chore: Cleanup StringBuilder
usmansaleem Mar 25, 2024
01d72f3
errorprone: Fix reported issues
usmansaleem Mar 25, 2024
345e499
chore: Fix supress warning typo
usmansaleem Mar 25, 2024
b13262d
fix: Fix invalid assertion
usmansaleem Mar 25, 2024
456b6bd
errorprone: Fix reported issues
usmansaleem Mar 25, 2024
71a2fa7
chore: spotless apply
usmansaleem Mar 25, 2024
becbbb0
fix: Remove custom error prone source
usmansaleem Mar 25, 2024
988310c
chore: Remove comment from build.gradle
usmansaleem Mar 25, 2024
a2c1ab2
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 25, 2024
1a28596
chore: Update gradle verification metadata
usmansaleem Mar 25, 2024
597a920
chore: Update gradle verification metadata
usmansaleem Mar 25, 2024
3afd788
chore: Update gradle verification metadata
usmansaleem Mar 25, 2024
e96c52c
chore: Update gradle verification metadata
usmansaleem Mar 25, 2024
72ee8cb
chore: Update fixing gradle verification metadata
usmansaleem Mar 25, 2024
603c21d
build: Update build.gradle, removing temporary errorprone check
usmansaleem Mar 25, 2024
095c112
build: verification metadata for spotless
usmansaleem Mar 26, 2024
cb8f07a
build: verification metadata for spotless
usmansaleem Mar 26, 2024
6ad2b70
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 26, 2024
6b77b18
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 26, 2024
13b9607
review: Simplify checkHostInAllowlist logic
usmansaleem Mar 26, 2024
fa9053e
review: Change TreeMap to NavigatableMap
usmansaleem Mar 26, 2024
ecacd19
review: Change TreeMap to NavigatableMap
usmansaleem Mar 26, 2024
4287430
Merge remote-tracking branch 'upstream/main' into werror_issue
usmansaleem Mar 26, 2024
0dd03d4
review: Change TreeMap to NavigableMap
usmansaleem Mar 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
Expand Down Expand Up @@ -431,7 +432,9 @@ public NodeRequests nodeRequests() {
getGenesisConfig()
.map(
gc ->
gc.toLowerCase().contains("ibft") ? ConsensusType.IBFT2 : ConsensusType.QBFT)
gc.toLowerCase(Locale.ROOT).contains("ibft")
? ConsensusType.IBFT2
: ConsensusType.QBFT)
.orElse(ConsensusType.IBFT2);

nodeRequests =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import org.web3j.crypto.Credentials;
Expand Down Expand Up @@ -83,7 +84,7 @@ && parameterTypesAreEqual(i.getParameterTypes(), parameterObjects))

@SuppressWarnings("rawtypes")
private boolean parameterTypesAreEqual(
final Class<?>[] expectedTypes, final ArrayList<Object> actualObjects) {
final Class<?>[] expectedTypes, final List<Object> actualObjects) {
if (expectedTypes.length != actualObjects.size()) {
return false;
}
Expand Down
7 changes: 4 additions & 3 deletions besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -800,7 +801,7 @@ public Runner build() {
metricsSystem,
supportedCapabilities,
jsonRpcConfiguration.getRpcApis().stream()
.filter(apiGroup -> !apiGroup.toLowerCase().startsWith("engine"))
.filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
.collect(Collectors.toList()),
filterManager,
accountLocalConfigPermissioningController,
Expand Down Expand Up @@ -938,7 +939,7 @@ public Runner build() {
metricsSystem,
supportedCapabilities,
webSocketConfiguration.getRpcApis().stream()
.filter(apiGroup -> !apiGroup.toLowerCase().startsWith("engine"))
.filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
.collect(Collectors.toList()),
filterManager,
accountLocalConfigPermissioningController,
Expand Down Expand Up @@ -1021,7 +1022,7 @@ public Runner build() {
metricsSystem,
supportedCapabilities,
jsonRpcIpcConfiguration.getEnabledApis().stream()
.filter(apiGroup -> !apiGroup.toLowerCase().startsWith("engine"))
.filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
.collect(Collectors.toList()),
filterManager,
accountLocalConfigPermissioningController,
Expand Down
5 changes: 1 addition & 4 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -2623,10 +2623,7 @@ private void instantiateSignatureAlgorithmFactory() {
SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get()));
} catch (final IllegalArgumentException e) {
throw new CommandLine.InitializationException(
new StringBuilder()
.append("Invalid genesis file configuration for ecCurve. ")
.append(e.getMessage())
.toString());
"Invalid genesis file configuration for ecCurve. " + e.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.cli.config;

import java.math.BigInteger;
import java.util.Locale;
import java.util.Optional;

import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -91,7 +92,7 @@ public boolean canSnapSync() {
* @return the string
*/
public String normalize() {
return StringUtils.capitalize(name().toLowerCase());
return StringUtils.capitalize(name().toLowerCase(Locale.ROOT));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
*/
package org.hyperledger.besu.cli.config;

import java.util.Locale;

import org.apache.commons.lang3.StringUtils;

/** Enum for profile names. Each profile corresponds to a configuration file. */
Expand Down Expand Up @@ -51,6 +53,6 @@ public String getConfigFile() {

@Override
public String toString() {
return StringUtils.capitalize(name().replaceAll("_", " ").toLowerCase());
return StringUtils.capitalize(name().replaceAll("_", " ").toLowerCase(Locale.ROOT));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.EnumSet;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -54,7 +55,7 @@ public <T extends Enum<T> & MetricCategory> void addCategories(final Class<T> ca
* @param metricCategory the metric category
*/
public void addRegistryCategory(final MetricCategory metricCategory) {
metricCategories.put(metricCategory.getName().toUpperCase(), metricCategory);
metricCategories.put(metricCategory.getName().toUpperCase(Locale.ROOT), metricCategory);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;

import java.util.List;
import java.util.Locale;

import org.apache.commons.lang3.StringUtils;
import picocli.CommandLine;
Expand Down Expand Up @@ -193,6 +194,6 @@ public List<String> getCLIOptions() {
* @return the normalized string
*/
public String normalizeDataStorageFormat() {
return StringUtils.capitalize(dataStorageFormat.toString().toLowerCase());
return StringUtils.capitalize(dataStorageFormat.toString().toLowerCase(Locale.ROOT));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.hyperledger.besu.cli.options.stable;

import java.util.Locale;
import java.util.Set;

import picocli.CommandLine;
Expand Down Expand Up @@ -52,8 +53,8 @@ public void setLogLevel(final String logLevel) {
if ("FATAL".equalsIgnoreCase(logLevel)) {
System.out.println("FATAL level is deprecated");
this.logLevel = "ERROR";
} else if (ACCEPTED_VALUES.contains(logLevel.toUpperCase())) {
this.logLevel = logLevel.toUpperCase();
} else if (ACCEPTED_VALUES.contains(logLevel.toUpperCase(Locale.ROOT))) {
this.logLevel = logLevel.toUpperCase(Locale.ROOT);
} else {
throw new CommandLine.ParameterException(
spec.commandLine(), "Unknown logging value: " + logLevel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,9 @@ private void importPublicKey(final JsonNode publicKeyJson) {

if (!SIGNATURE_ALGORITHM.get().isValidPublicKey(publicKey)) {
throw new IllegalArgumentException(
new StringBuilder()
.append(publicKeyText)
.append(" is not a valid public key for elliptic curve ")
.append(SIGNATURE_ALGORITHM.get().getCurveName())
.toString());
publicKeyText
+ " is not a valid public key for elliptic curve "
+ SIGNATURE_ALGORITHM.get().getCurveName());
}

writeKeypair(publicKey, null);
Expand Down Expand Up @@ -297,10 +295,7 @@ private void processEcCurve() {
SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get()));
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
new StringBuilder()
.append("Invalid parameter for ecCurve in genesis config: ")
.append(e.getMessage())
.toString());
"Invalid parameter for ecCurve in genesis config: " + e.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ protected Synchronizer createSynchronizer(
return sync;
}

@SuppressWarnings("UnusedVariable")
private void initTransitionWatcher(
final ProtocolContext protocolContext, final TransitionCoordinator composedCoordinator) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import java.util.Collection;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -58,7 +59,10 @@ public <T> void registerRPCEndpoint(
namespaces.stream()
.anyMatch(
namespace ->
entry.getKey().toUpperCase().startsWith(namespace.toUpperCase())))
entry
.getKey()
.toUpperCase(Locale.ROOT)
.startsWith(namespace.toUpperCase(Locale.ROOT))))
.map(entry -> new PluginJsonRpcMethod(entry.getKey(), entry.getValue()))
.collect(Collectors.toMap(PluginJsonRpcMethod::getName, e -> e));
}
Expand All @@ -71,6 +75,7 @@ public <T> void registerRPCEndpoint(
*/
public boolean hasNamespace(final String namespace) {
return rpcMethods.keySet().stream()
.anyMatch(key -> key.toUpperCase().startsWith(namespace.toUpperCase()));
.anyMatch(
key -> key.toUpperCase(Locale.ROOT).startsWith(namespace.toUpperCase(Locale.ROOT)));
}
}
38 changes: 15 additions & 23 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ plugins {
id 'com.jfrog.artifactory' version '5.1.11'
id 'io.spring.dependency-management' version '1.1.4'
id 'me.champeau.jmh' version '0.7.2' apply false
id 'net.ltgt.errorprone' version '3.0.1'
id 'net.ltgt.errorprone' version '3.1.0'
id 'maven-publish'
id 'org.sonarqube' version '4.4.1.3373'
}
Expand Down Expand Up @@ -129,12 +129,12 @@ allprojects {
}

task sourcesJar(type: Jar, dependsOn: classes) {
classifier = 'sources'
archiveClassifier = 'sources'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this need to change?

Copy link
Member Author

Choose a reason for hiding this comment

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

These were warnings in build.gradle file that will be an issue in future release of gradle. Although they should not be technically part of this PR, I fixed them originally while researching this problem and were using debug flags to the build.

from sourceSets.main.allSource
}

task javadocJar(type: Jar, dependsOn: javadoc) {
classifier = 'javadoc'
archiveClassifier = 'javadoc'
from javadoc.outputDirectory
}

Expand All @@ -157,6 +157,7 @@ allprojects {
url 'https://splunk.jfrog.io/splunk/ext-releases-local'
content { includeGroupByRegex('com\\.splunk\\..*') }
}

mavenCentral()

// ethereum execution spec tests fixtures. Exclusively for ethereum submodule to run ref tests
Expand All @@ -178,6 +179,8 @@ allprojects {
dependencies {
components.all(BouncyCastleCapability)
errorprone 'com.google.errorprone:error_prone_core'
// https://github.com/hyperledger/besu-errorprone-checks/
errorprone "org.hyperledger.besu:besu-errorprone-checks"
}

configurations.all {
Expand Down Expand Up @@ -213,7 +216,7 @@ allprojects {
format 'sol', { target '**/*.sol' }
}

tasks.withType(JavaCompile) {
tasks.withType(JavaCompile).configureEach {
options.compilerArgs += [
'-Xlint:unchecked',
'-Xlint:cast',
Expand All @@ -226,8 +229,8 @@ allprojects {
]

options.errorprone {
excludedPaths = '.*/(generated/*.*|.*ReferenceTest_.*|build/.*/annotation-output/.*)'

excludedPaths = '.*/generated/*.*'
disableWarningsInGeneratedCode = true
// Our equals need to be symmetric, this checker doesn't respect that.
check('EqualsGetClass', CheckSeverity.OFF)
// We like to use futures with no return values.
Expand Down Expand Up @@ -289,7 +292,7 @@ allprojects {
*
*/
test {
jvmArgs = [
jvmArgs += [
'-Xmx4g',
'-XX:-UseGCOverheadLimit',
// Mockito and jackson-databind do some strange reflection during tests.
Expand Down Expand Up @@ -394,7 +397,7 @@ subprojects {

task testSupportJar(type: Jar) {
archiveBaseName = "${project.name}-support-test"
classifier = 'test-support'
archiveClassifier = 'test-support'
from sourceSets.testSupport.output
}
}
Expand Down Expand Up @@ -994,7 +997,7 @@ task checkSpdxHeader(type: CheckSpdxHeader) {

jacocoTestReport {
reports {
xml.enabled true
xml.required = true
}
}

Expand All @@ -1005,25 +1008,12 @@ task jacocoRootReport(type: org.gradle.testing.jacoco.tasks.JacocoReport) {
executionData.from fileTree(dir: '.', includes: ['**/jacoco/*.exec'])
reports {
xml.required = true
xml.enabled = true
csv.required = true
html.destination file("build/reports/jacocoHtml")
}
onlyIf = { true }
}

configurations { annotationProcessor }

// Prevent errorprone-checks being dependent upon errorprone-checks!
// However, ensure all subprojects comply with the custom rules.
configure(subprojects.findAll { it.name != 'errorprone-checks' }) {
dependencies { annotationProcessor project(":errorprone-checks") }

tasks.withType(JavaCompile) {
options.annotationProcessorPath = configurations.annotationProcessor
}
}

// http://label-schema.org/rc1/
// using the RFC3339 format "2016-04-12T23:20:50.52Z"
def buildTime() {
Expand Down Expand Up @@ -1104,9 +1094,11 @@ tasks.register("verifyDistributions") {
}

dependencies {
errorprone 'com.google.errorprone:error_prone_core'
// https://github.com/hyperledger/besu-errorprone-checks/
errorprone 'org.hyperledger.besu:besu-errorprone-checks'
implementation project(':besu')
implementation project(':ethereum:evmtool')
errorprone 'com.google.errorprone:error_prone_core'
}

@CompileStatic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.math.BigInteger;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.OptionalInt;

Expand Down Expand Up @@ -89,7 +90,7 @@ public Optional<BigInteger> getBlockRewardWei() {
return Optional.empty();
}
final String weiStr = configFileContent.get();
if (weiStr.toLowerCase().startsWith("0x")) {
if (weiStr.toLowerCase(Locale.ROOT).startsWith("0x")) {
return Optional.of(new BigInteger(1, Bytes.fromHexStringLenient(weiStr).toArrayUnsafe()));
}
return Optional.of(new BigInteger(weiStr));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.hyperledger.besu.datatypes.Address;

import java.math.BigInteger;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;

Expand Down Expand Up @@ -117,7 +118,7 @@ public BigInteger getBlockRewardWei() {
return BigInteger.ZERO;
}
final String weiStr = configFileContent.get();
if (weiStr.toLowerCase().startsWith("0x")) {
if (weiStr.toLowerCase(Locale.ROOT).startsWith("0x")) {
return new BigInteger(1, Bytes.fromHexStringLenient(weiStr).toArrayUnsafe());
}
return new BigInteger(weiStr);
Expand Down
Loading
Loading