adoroszlai commented on code in PR #4596:
URL: https://github.com/apache/ozone/pull/4596#discussion_r1172769622
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -26,6 +26,21 @@ else
MAVEN_OPTIONS="${MAVEN_OPTIONS} -Djacoco.skip"
fi
+PROJECT_VERSION=$(mvn help:evaluate -Dexpression=project.version -q
-DforceStdout)
+VERSION_NUMBER=$(echo "${PROJECT_VERSION}"| cut -f1 -d'-')
+EXPECTED_ROCKS_NATIVE_VERSION=${VERSION_NUMBER}".${NATIVE_ROCKS_SHA}"${PROJECT_VERSION:${#VERSION_NUMBER}}
+echo "Checking Maven repo contains hdds-rocks-native of version
${EXPECTED_ROCKS_NATIVE_VERSION}"
+mvn dependency:get
-Dartifact=org.apache.ozone:hdds-rocks-native:${EXPECTED_ROCKS_NATIVE_VERSION}
+
+EXPECTED_ROCKS_NATIVE_VERSION_EXISTS=$?
+if [[ "${EXPECTED_ROCKS_NATIVE_VERSION_EXISTS}" == "0" ]]; then
+ echo "Build using hdds-rocks-native version: $(mvn help:evaluate
-Dexpression=hdds.rocks.native.version -q -DforceStdout
-Dhdds.rocks.native.version=${EXPECTED_ROCKS_NATIVE_VERSION})"
+ MAVEN_OPTIONS="${MAVEN_OPTIONS}
-Dhdds.rocks.native.version=${EXPECTED_ROCKS_NATIVE_VERSION}"
+else
+ echo "Build using hdds-rocks-native version: $(mvn help:evaluate
-Dexpression=hdds.rocks.native.version -q -DforceStdout)"
+ MAVEN_OPTIONS="${MAVEN_OPTIONS} -Drocks_tools_native"
Review Comment:
The message:
```
Build using hdds-rocks-native version: 1.4.0-SNAPSHOT
```
is confusing. Instead of the generic version number, it should clearly
state that it is also building the rocks native module.
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -26,6 +26,21 @@ else
MAVEN_OPTIONS="${MAVEN_OPTIONS} -Djacoco.skip"
fi
+PROJECT_VERSION=$(mvn help:evaluate -Dexpression=project.version -q
-DforceStdout)
+VERSION_NUMBER=$(echo "${PROJECT_VERSION}"| cut -f1 -d'-')
+EXPECTED_ROCKS_NATIVE_VERSION=${VERSION_NUMBER}".${NATIVE_ROCKS_SHA}"${PROJECT_VERSION:${#VERSION_NUMBER}}
+echo "Checking Maven repo contains hdds-rocks-native of version
${EXPECTED_ROCKS_NATIVE_VERSION}"
+mvn dependency:get
-Dartifact=org.apache.ozone:hdds-rocks-native:${EXPECTED_ROCKS_NATIVE_VERSION}
+
+EXPECTED_ROCKS_NATIVE_VERSION_EXISTS=$?
+if [[ "${EXPECTED_ROCKS_NATIVE_VERSION_EXISTS}" == "0" ]]; then
+ echo "Build using hdds-rocks-native version: $(mvn help:evaluate
-Dexpression=hdds.rocks.native.version -q -DforceStdout
-Dhdds.rocks.native.version=${EXPECTED_ROCKS_NATIVE_VERSION})"
Review Comment:
The `mvn` command here is unnecessary. It outputs the value of an
expression, but the value is provided in the same command. It should simply
print the `${EXPECTED_ROCKS_NATIVE_VERSION}` variable.
```suggestion
echo "Build using hdds-rocks-native version:
${EXPECTED_ROCKS_NATIVE_VERSION}"
```
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -26,6 +26,21 @@ else
MAVEN_OPTIONS="${MAVEN_OPTIONS} -Djacoco.skip"
fi
+PROJECT_VERSION=$(mvn help:evaluate -Dexpression=project.version -q
-DforceStdout)
+VERSION_NUMBER=$(echo "${PROJECT_VERSION}"| cut -f1 -d'-')
+EXPECTED_ROCKS_NATIVE_VERSION=${VERSION_NUMBER}".${NATIVE_ROCKS_SHA}"${PROJECT_VERSION:${#VERSION_NUMBER}}
Review Comment:
This will be a strange version string when run locally:
```
1.4.0.-SNAPSHOT
```
because `${NATIVE_ROCKS_SHA}` is only defined in Github Actions.
It would be better to skip the change in Github Actions
(`selective_ci_checks.sh` and `ci.yml`), and let `build.sh` be self-contained.
In other words, move SHA calculation here. Better yet, extract this logic to a
helper script, which can be tested in isolation (without a full build).
Also, please make this optional, using an environment variable as condition.
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -26,6 +26,21 @@ else
MAVEN_OPTIONS="${MAVEN_OPTIONS} -Djacoco.skip"
fi
+PROJECT_VERSION=$(mvn help:evaluate -Dexpression=project.version -q
-DforceStdout)
+VERSION_NUMBER=$(echo "${PROJECT_VERSION}"| cut -f1 -d'-')
+EXPECTED_ROCKS_NATIVE_VERSION=${VERSION_NUMBER}".${NATIVE_ROCKS_SHA}"${PROJECT_VERSION:${#VERSION_NUMBER}}
+echo "Checking Maven repo contains hdds-rocks-native of version
${EXPECTED_ROCKS_NATIVE_VERSION}"
+mvn dependency:get
-Dartifact=org.apache.ozone:hdds-rocks-native:${EXPECTED_ROCKS_NATIVE_VERSION}
Review Comment:
No need to execute for all subprojects:
```suggestion
mvn --non-recursive dependency:get
-Dartifact=org.apache.ozone:hdds-rocks-native:${EXPECTED_ROCKS_NATIVE_VERSION}
```
--
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]
