[hotfix][tools] Report RAT validation dependencies correctly

[kaiwangleo]

Linked issue: N/A (hotfix)

Purpose of change

Make tools/check-license.sh report the actual cause when Apache RAT cannot be downloaded or validated. The script now:

• uses the JDK jar tool when available and falls back to unzip;
• reports missing download and validation tools explicitly;
• preserves HTTP errors from curl with --fail --show-error --location;
• removes partial downloads after failures;
• validates cached RAT JARs before using them;
• distinguishes an invalid JAR from a missing validation dependency.

Tests

• Added tools/test/unit/check_license.bats covering missing validation tools, invalid cached JARs, valid cached JARs, download failures, partial-file cleanup, curl flags, and successful validation.
• bash -n tools/check-license.sh
• bash tools/test/.bats-cache/bats-core/bin/bats tools/test/unit/check_license.bats (5 tests passed)
• git diff --check
• bash tools/check-license.sh did not complete within 60 seconds on the Windows-mounted workspace; no failure result was produced.

API

No public API changes.

Documentation

• doc-needed
• doc-not-needed
• doc-included

[weiqingy]

Thanks for taking this on. A few questions inline.

[weiqingy]

With the [[ -f "$rat_jar" ]] || short-circuit gone from the call site at line 101, validation runs on every invocation, so status 2 now aborts even when a good cached JAR is already on disk. I ran both versions in one environment (cached JAR present, neither jar nor unzip on PATH, JAVA_HOME unset): main prints RAT checks passed. and exits 0, this branch exits 1. It takes both tools missing, so a JRE-only image without unzip is the realistic case. Worth knowing too that ci.yml:33-35 runs this script before Set up JDK, so the new requirement rests on whatever the runner image preinstalls.

Since the PR is about reporting a missing dependency, would warning and proceeding be closer to the intent when the JAR is already on disk, keeping the hard failure for one we just downloaded?

[kaiwangleo]

Thanks for reproducing this edge case. I agree that a cached RAT JAR should remain usable when validation tools are unavailable, especially because the CI license step runs before the workflow installs a JDK. The script now distinguishes cached from newly downloaded JARs: an unavailable validator emits a warning and proceeds for a cached JAR, while a newly downloaded JAR fails closed and is removed because it has not been validated. Invalid JARs still fail in both cases. This is covered by the updated cached-JAR test in commit 585770e.

[weiqingy]

nit: --show-error only takes effect together with --silent. man curl describes it as "When used with -s, --silent, it makes curl show an error message if it fails." The failure text comes through either way, so nothing is lost. As written, though, the flag is inert and curl writes its progress meter to stderr on every download, tty or not.

What do you think about putting --silent back next to it, so the pair does what it says?

[kaiwangleo]

Good catch. --show-error is now paired with --silent, so curl suppresses the progress meter while still surfacing useful error text together with --fail. The focused download tests pass in commit 585770e.

[weiqingy]

wget -q turns off wget's output entirely, error text included, so this branch still surfaces only the generic Failed to download Apache RAT from %s. line. That is the diagnostic the curl branch is being changed to improve. --no-verbose is the middle setting: it drops the progress output while, per the wget manual, "error messages and basic information still get printed".

Is the quiet output deliberate on this branch, or would --no-verbose fit the goal better?

[kaiwangleo]

Agreed. I changed wget from --quiet to --no-verbose, which keeps normal progress output suppressed while preserving error and basic diagnostic messages. The download-failure path remains covered by the focused tests in commit 585770e.

[weiqingy]

tools/test/helpers/shim.bash already provides all five of these helpers plus a shim_setup, and shim_bin / shim_bin_script here are byte-identical to shim.bash:37-45 and 50-58. tools/test/unit/shim_self_test.bats:5-10 shows the load pattern for a unit test. I tried the swap locally: load '../helpers/shim' plus a shim_setup call in setup(), with the five local copies deleted, keeps all 5 tests green.

Would reusing the shared helper work for you here?

[kaiwangleo]

Yes, reusing the shared helper is cleaner. I removed the duplicated shim implementations and now load ../helpers/shim and call shim_setup from the test setup. The five focused tests remain green in commit 585770e.

[weiqingy]

shim_bin curl 22 exits without ever creating ${rat_jar}.part, so the [ ! -e "${rat_jar}.part" ] assertion at line 126 holds no matter what the script does. I confirmed by deleting rm -f "$JAR_DL" from the curl-failure branch: all 5 tests still pass. Could the stub create the file first, so the assertion has something to catch? Something like this, if it helps:

shim_bin_script curl 'prev=""; for arg in "$@"; do [[ "$prev" == "--output" ]] && : > "$arg"; prev="$arg"; done; exit 22'

[kaiwangleo]

Thanks for identifying the vacuous assertion. The curl failure stub now creates the requested .part output file before exiting 22, so the [ ! -e .part\ ] assertion verifies the cleanup performed by the script rather than passing trivially. The focused test suite passes in commit 585770e.