kaiwangleo commented on code in PR #996:
URL: https://github.com/apache/flink-agents/pull/996#discussion_r3780802614
##########
tools/check-license.sh:
##########
@@ -20,41 +20,83 @@
# NOTE: This script is adapted from the Apache Spark project.
-acquire_rat_jar () {
+validate_rat_jar() {
+ local jar_cmd
+
+ if [ -n "${JAVA_HOME:-}" ] && [ -x "$JAVA_HOME/bin/jar" ]; then
+ jar_cmd="$JAVA_HOME/bin/jar"
+ elif command -v jar >/dev/null 2>&1; then
+ jar_cmd="$(command -v jar)"
+ elif command -v unzip >/dev/null 2>&1; then
+ unzip -tq "$JAR" >/dev/null 2>&1
+ return $?
+ else
+ printf "Cannot validate Apache RAT: install a JDK with 'jar' or install
'unzip'.\n" >&2
+ return 2
+ fi
-
URL="https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat/${RAT_VERSION}/apache-rat-${RAT_VERSION}.jar";
+ "$jar_cmd" tf "$JAR" >/dev/null 2>&1
+}
+acquire_rat_jar() {
+
URL="https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat/${RAT_VERSION}/apache-rat-${RAT_VERSION}.jar";
JAR="$rat_jar"
+ downloaded=false
- # Download rat launch jar if it hasn't been downloaded yet
if [ ! -f "$JAR" ]; then
- # Download
+ downloaded=true
printf "Attempting to fetch rat\n"
JAR_DL="${JAR}.part"
- if [ $(command -v curl) ]; then
- curl -L --silent "${URL}" > "$JAR_DL" && mv "$JAR_DL" "$JAR"
- elif [ $(command -v wget) ]; then
- wget --quiet ${URL} -O "$JAR_DL" && mv "$JAR_DL" "$JAR"
+ rm -f "$JAR_DL"
+ if command -v curl >/dev/null 2>&1; then
+ if ! curl --fail --silent --show-error --location --output "$JAR_DL"
"$URL"; then
+ rm -f "$JAR_DL"
+ printf "Failed to download Apache RAT from %s.\n" "$URL" >&2
+ return 1
+ fi
+ elif command -v wget >/dev/null 2>&1; then
+ if ! wget --no-verbose --output-document="$JAR_DL" "$URL"; then
+ rm -f "$JAR_DL"
+ printf "Failed to download Apache RAT from %s.\n" "$URL" >&2
+ return 1
+ fi
else
- printf "You do not have curl or wget installed, please install rat
manually.\n"
- exit -1
+ printf "Cannot download Apache RAT: install 'curl' or 'wget'.\n" >&2
+ return 1
+ fi
+ if ! mv "$JAR_DL" "$JAR"; then
+ rm -f "$JAR_DL"
+ printf "Failed to store the downloaded Apache RAT JAR at %s.\n" "$JAR"
>&2
+ return 1
fi
fi
- unzip -tq "$JAR" &> /dev/null
- if [ $? -ne 0 ]; then
- # We failed to download
- rm "$JAR"
- printf "Our attempt to download rat locally to ${JAR} failed. Please
install rat manually.\n"
- exit -1
+ validate_rat_jar
+ validation_status=$?
+ if [ "$validation_status" -eq 2 ]; then
+ if [ "$downloaded" = true ]; then
+ rm -f "$JAR"
+ printf "Cannot validate the downloaded Apache RAT JAR: install jar or
unzip.\n" >&2
+ return 1
Review Comment:
Added a regression test for a newly downloaded JAR when both jar and unzip
are unavailable. The test creates the partial download, asserts a non-zero
result, verifies that both the final JAR and .part file are removed, and checks
the diagnostic. Cached JARs retain the warning-and-proceed behavior.
##########
tools/check-license.sh:
##########
@@ -20,41 +20,83 @@
# NOTE: This script is adapted from the Apache Spark project.
-acquire_rat_jar () {
+validate_rat_jar() {
+ local jar_cmd
+
+ if [ -n "${JAVA_HOME:-}" ] && [ -x "$JAVA_HOME/bin/jar" ]; then
+ jar_cmd="$JAVA_HOME/bin/jar"
+ elif command -v jar >/dev/null 2>&1; then
Review Comment:
Added coverage for the JDK jar fallback with both successful and failing
validator shims. The tests now exercise the cached-JAR success path and the
invalid-JAR cleanup path when unzip is unavailable.
##########
tools/check-license.sh:
##########
@@ -20,41 +20,83 @@
# NOTE: This script is adapted from the Apache Spark project.
-acquire_rat_jar () {
+validate_rat_jar() {
+ local jar_cmd
+
+ if [ -n "${JAVA_HOME:-}" ] && [ -x "$JAVA_HOME/bin/jar" ]; then
+ jar_cmd="$JAVA_HOME/bin/jar"
+ elif command -v jar >/dev/null 2>&1; then
+ jar_cmd="$(command -v jar)"
+ elif command -v unzip >/dev/null 2>&1; then
+ unzip -tq "$JAR" >/dev/null 2>&1
+ return $?
+ else
+ printf "Cannot validate Apache RAT: install a JDK with 'jar' or install
'unzip'.\n" >&2
+ return 2
+ fi
-
URL="https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat/${RAT_VERSION}/apache-rat-${RAT_VERSION}.jar";
+ "$jar_cmd" tf "$JAR" >/dev/null 2>&1
Review Comment:
Good point. The validator now prefers unzip -tq, preserving the stronger CRC
validation used by the original implementation, and falls back to the JDK jar
tool only when unzip is unavailable. A test pins the preference when both tools
are present.
##########
tools/test/unit/check_license.bats:
##########
@@ -0,0 +1,95 @@
+#!/usr/bin/env bats
+
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+setup() {
+ load '../helpers/shim'
+ shim_setup
+ CHECK_LICENSE_SOURCE_ONLY=1
+ source "${BATS_TEST_DIRNAME}/../../check-license.sh"
+ unset CHECK_LICENSE_SOURCE_ONLY
+
+
+ RAT_VERSION="test"
+ rat_jar="$BATS_TEST_TMPDIR/apache-rat-test.jar"
+ JAR="$rat_jar"
+ JAVA_HOME="$BATS_TEST_TMPDIR/missing-java-home"
+}
+
+@test "reports missing validation tools without deleting an existing JAR" {
+ touch "$rat_jar"
+ shim_bin_missing jar
+ shim_bin_missing unzip
+
+ run acquire_rat_jar
+
+ [ "$status" -eq 0 ]
+ [[ "$output" == *"Warning: cannot validate cached Apache RAT JAR"* ]]
+ [ -f "$rat_jar" ]
+}
+
+@test "rejects and removes an invalid existing JAR" {
+ touch "$rat_jar"
+ shim_bin_missing jar
+ shim_bin unzip 1
+
+ run acquire_rat_jar
+
+ [ "$status" -ne 0 ]
+ [[ "$output" == *"is invalid"* ]]
+ [ ! -f "$rat_jar" ]
+}
+
+@test "does not download an existing valid JAR" {
+ touch "$rat_jar"
+ shim_bin_missing jar
+ shim_bin unzip
+ shim_bin curl
+
+ acquire_rat_jar
+
+ [ "$(shim_call_count curl)" = "0" ]
+ [ "$(shim_call_count unzip)" = "1" ]
+}
+
+@test "reports a download failure and removes the partial file" {
+ shim_bin_script curl 'prev=""; for arg in "$@"; do [[ "$prev" ==
"--output" ]] && : > "$arg"; prev="$arg"; done; exit 22'
+
+ run acquire_rat_jar
+
+ [ "$status" -ne 0 ]
+ [[ "$output" == *"Failed to download Apache RAT"* ]]
+ [ ! -e "${rat_jar}.part" ]
+ [ ! -e "$rat_jar" ]
+}
+
+@test "downloads with curl safety flags and validates the result" {
+ shim_bin_script curl 'while [[ "$1" != "--output" ]]; do shift; done;
touch "$2"'
Review Comment:
Reworked the curl stubs to scan the arguments once, support both --output
and -o, and exit with status 64 if no output argument is found. This prevents a
command-line regression from turning into a CI timeout.
##########
tools/check-license.sh:
##########
@@ -20,41 +20,83 @@
# NOTE: This script is adapted from the Apache Spark project.
-acquire_rat_jar () {
+validate_rat_jar() {
+ local jar_cmd
+
+ if [ -n "${JAVA_HOME:-}" ] && [ -x "$JAVA_HOME/bin/jar" ]; then
+ jar_cmd="$JAVA_HOME/bin/jar"
+ elif command -v jar >/dev/null 2>&1; then
+ jar_cmd="$(command -v jar)"
+ elif command -v unzip >/dev/null 2>&1; then
+ unzip -tq "$JAR" >/dev/null 2>&1
+ return $?
+ else
+ printf "Cannot validate Apache RAT: install a JDK with 'jar' or install
'unzip'.\n" >&2
+ return 2
+ fi
-
URL="https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat/${RAT_VERSION}/apache-rat-${RAT_VERSION}.jar";
+ "$jar_cmd" tf "$JAR" >/dev/null 2>&1
+}
+acquire_rat_jar() {
+
URL="https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat/${RAT_VERSION}/apache-rat-${RAT_VERSION}.jar";
JAR="$rat_jar"
+ downloaded=false
- # Download rat launch jar if it hasn't been downloaded yet
if [ ! -f "$JAR" ]; then
- # Download
+ downloaded=true
printf "Attempting to fetch rat\n"
JAR_DL="${JAR}.part"
- if [ $(command -v curl) ]; then
- curl -L --silent "${URL}" > "$JAR_DL" && mv "$JAR_DL" "$JAR"
- elif [ $(command -v wget) ]; then
- wget --quiet ${URL} -O "$JAR_DL" && mv "$JAR_DL" "$JAR"
+ rm -f "$JAR_DL"
+ if command -v curl >/dev/null 2>&1; then
+ if ! curl --fail --silent --show-error --location --output "$JAR_DL"
"$URL"; then
+ rm -f "$JAR_DL"
+ printf "Failed to download Apache RAT from %s.\n" "$URL" >&2
+ return 1
+ fi
+ elif command -v wget >/dev/null 2>&1; then
+ if ! wget --no-verbose --output-document="$JAR_DL" "$URL"; then
+ rm -f "$JAR_DL"
+ printf "Failed to download Apache RAT from %s.\n" "$URL" >&2
+ return 1
+ fi
else
- printf "You do not have curl or wget installed, please install rat
manually.\n"
- exit -1
+ printf "Cannot download Apache RAT: install 'curl' or 'wget'.\n" >&2
+ return 1
+ fi
+ if ! mv "$JAR_DL" "$JAR"; then
+ rm -f "$JAR_DL"
+ printf "Failed to store the downloaded Apache RAT JAR at %s.\n" "$JAR"
>&2
+ return 1
fi
fi
- unzip -tq "$JAR" &> /dev/null
- if [ $? -ne 0 ]; then
- # We failed to download
- rm "$JAR"
- printf "Our attempt to download rat locally to ${JAR} failed. Please
install rat manually.\n"
- exit -1
+ validate_rat_jar
+ validation_status=$?
Review Comment:
Done. downloaded and validation_status are now local to acquire_rat_jar.
--
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]
