weiqingy commented on code in PR #996:
URL: https://github.com/apache/flink-agents/pull/996#discussion_r3792909678
##########
tools/check-license.sh:
##########
@@ -20,41 +20,84 @@
# NOTE: This script is adapted from the Apache Spark project.
-acquire_rat_jar () {
+validate_rat_jar() {
+ local jar_cmd
+
+ if command -v unzip >/dev/null 2>&1; then
+ unzip -tq "$JAR" >/dev/null 2>&1
+ return $?
Review Comment:
Right now `validate_rat_jar` passes unzip's exit code straight back to the
caller, and line 77 reads a 2 as "neither validator is installed". The catch is
that unzip also uses 2 to mean "this zip file is broken", so a genuinely
corrupt JAR ends up on the same path as a missing tool.
To check, I built a 200 KB zip and overwrote 64 bytes inside one entry while
leaving the directory intact, which is what a bad checksum looks like. `unzip
-tq` returned 2 and `jar tf` returned 0. Truncated, non-zip and empty files
gave unzip 9 and jar 1, so out of the cases I tried this is the one that
overlaps.
The effect at head, with a cached JAR and `shim_bin unzip 2`: the function
exits 0, prints `Warning: cannot validate cached Apache RAT JAR at ...; install
jar or unzip.`, and leaves the JAR in place, so it still gets run at line 116.
On `main` that same exit code hit `rm "$JAR"` and `exit -1` (lines 44-49), so
this is the one path that comes out more permissive than before the rewrite.
The message reads a little oddly too, since it asks for unzip at the moment
unzip is installed and is the thing reporting the problem.
One way to keep the two apart would be to fold any nonzero unzip result into
1, so 2 only ever means "no tool available". Something like this, if it helps:
```bash
unzip -tq "$JAR" >/dev/null 2>&1 || return 1
return 0
```
I tried that with a `shim_bin unzip 2` test added and the suite came out
10/10 green, and that same test goes red without it. Does keeping 2 for the
missing-tool case fit what you had in mind, or were you picturing the split
differently?
--
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]
