adoroszlai commented on code in PR #11320:
URL: https://github.com/apache/ozone/pull/11320#discussion_r4101964448
##########
hadoop-hdds/rocks-native/pom.xml:
##########
@@ -219,6 +219,8 @@
<url>https://github.com/facebook/rocksdb/archive/refs/tags/v${rocksdb.source.version}.tar.gz</url>
<outputFileName>rocksdb-v${rocksdb.source.version}.tar.gz</outputFileName>
<outputDirectory>${project.build.directory}/rocksdb</outputDirectory>
+ <readTimeOut>120000</readTimeOut>
+ <skipCache>false</skipCache>
Review Comment:
`skipCache=false` by default.
https://github.com/download-maven-plugin/download-maven-plugin/blob/e8c1b68c4bd5338fab1a77b9ca56430de8b498c6/src/main/java/io/github/download/maven/plugin/internal/WGetMojo.java#L248-L249
##########
hadoop-ozone/dev-support/checks/repro.sh:
##########
@@ -23,4 +23,10 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null
2>&1 && pwd )"
CHECK=repro
ERROR_PATTERN='ERROR.*mismatch'
-source "${DIR}"/_build.sh verify artifact:compare "$@"
+# CI repro downloads build-job jars into ~/.m2 (OZONE_REPO_CACHED); compare
against that reference.
+# Local runs without a cached repo need install first to populate the
reference.
+if [[ "${OZONE_REPO_CACHED:-false}" == "true" ]]; then
+ source "${DIR}"/_build.sh verify artifact:compare "$@"
+else
+ source "${DIR}"/_build.sh install artifact:compare "$@"
+fi
Review Comment:
This change is unnecessary. `artifact:compare` does not re-build the
project, only compares. So installing first is useless, comparison can never
fail afterwards.
For using `repro.sh` locally, you have to run `build.sh` first.
##########
pom.xml:
##########
@@ -1905,6 +1905,14 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
<version>${maven-source-plugin.version}</version>
+ <configuration>
+ <archive>
+ <manifest>
+
<addDefaultImplementationEntries>false</addDefaultImplementationEntries>
+
<addDefaultSpecificationEntries>false</addDefaultSpecificationEntries>
+ </manifest>
+ </archive>
+ </configuration>
Review Comment:
I don't think this is a problem, see
[doc](https://maven.apache.org/shared/maven-archiver/examples/manifest.html):
> Starting with version 2.1, Maven Archiver no longer creates the
Implementation and Specification details in the manifest by default.
##########
hadoop-ozone/dev-support/checks/junit.sh:
##########
@@ -54,10 +54,33 @@ if [[ -f
hadoop-ozone/dist/src/shell/ozone/ozone-functions.sh ]]; then
ozone_java_setup
fi
-mvn ${MAVEN_OPTIONS} clean
-
-if [[ ${ITERATIONS} -gt 1 ]] && [[ ${OZONE_REPO_CACHED} == "false" ]]; then
- mvn ${MAVEN_OPTIONS} -DskipTests install
+MAVEN_LIFECYCLE="verify"
+PL_ARGS=()
+if [[ "${OZONE_REPO_CACHED}" == "true" ]]; then
+ MAVEN_LIFECYCLE="test"
+ if [[ "${CHECK}" == "integration" ]]; then
+ # test-* profiles configure Surefire on the root POM; matching tests can
live in any module
+ # (e.g. test-om in ozone-manager and ozone-integration-test). Use
full-reactor mvn test.
+ # hadoop-native-lib is bound on ozone-main (inherited=false).
+ if [[ "$*" == *"-Phadoop-native-lib"* ]]; then
+ # Bootstrap must not inherit --fail-never from the main test invocation.
+ NATIVE_LIB_MVN_OPTIONS="${MAVEN_OPTIONS//--fail-never/}"
+ NATIVE_LIB_MVN_OPTIONS="${NATIVE_LIB_MVN_OPTIONS} --fail-fast"
+ if ! mvn ${NATIVE_LIB_MVN_OPTIONS} -pl :ozone-main -Phadoop-native-lib
generate-resources; then
+ echo "Failed to populate target/native-lib (hadoop-native-lib
profile)" >&2
+ exit 1
+ fi
+ if [[ ! -d target/native-lib ]] || ! ls target/native-lib/* >/dev/null
2>&1; then
+ echo "target/native-lib is missing after hadoop-native-lib
generate-resources" >&2
+ exit 1
+ fi
+ fi
+ fi
Review Comment:
Running `generate-resources` separately is unnecessary. It is already
executed as part of the [default
lifecycle](https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html#lifecycle-reference)
when running `install` (or `verify`, or `test`, or any similar phase).
##########
hadoop-ozone/dev-support/checks/junit.sh:
##########
@@ -77,7 +100,9 @@ for i in $(seq 1 ${ITERATIONS}); do
mkdir -p "${REPORT_DIR}"
fi
- mvn ${MAVEN_OPTIONS}
-Dmaven-surefire-plugin.argLineAccessArgs="${OZONE_MODULE_ACCESS_ARGS}" "$@"
verify \
+ # shellcheck disable=SC2068
+ mvn ${MAVEN_OPTIONS}
-Dmaven-surefire-plugin.argLineAccessArgs="${OZONE_MODULE_ACCESS_ARGS}" \
+ ${PL_ARGS[@]+"${PL_ARGS[@]}"} "$@" ${MAVEN_LIFECYCLE} \
Review Comment:
`PL_ARGS` is never populated, please don't add it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
