bharos opened a new pull request, #13256:
URL: https://github.com/apache/gravitino/pull/13256
### What changes were proposed in this pull request?
`dev/release/check-license.sh` validated only the tokens on a line that
begin with `./`. Any reference written without that prefix was never checked,
which is how a broken path in `LICENSE` has been able to survive in the tree.
This PR replaces the prefix test with a regular expression that extracts every
path-like token, and validates a token when its first path segment is an
existing directory in the project root.
That single condition is what keeps the check quiet: URLs, Java package
names and paths internal to a bundled jar such as `META-INF/NOTICE` all fail
it, because none of them name a file that is expected to exist in the source
tree.
Three other changes come with it:
- `NOTICE` is now checked as well. Previously only `LICENSE` was read.
- A document that cannot be read is now a failure. Previously `cat` wrote an
error to stderr and the script still reported success and exited `0`.
- Only unresolved references are printed, each with its line number, the
referencing line, and any candidate locations found elsewhere in the tree, so
the correct path is visible in the failure itself.
The `LICENSE.bin` and `NOTICE.bin` variants are deliberately left out of
scope. Their paths describe the layout of the binary package assembled by
`build.gradle.kts`, not the source tree, so resolving them here would report
failures that are not real.
### Why are the changes needed?
The check does not currently catch the thing it exists to catch.
On `main` today, `LICENSE:204` says:
```
The Web UI also bundles various third-party components also under
different licenses, please see web/LICENSE for these.
```
`web/LICENSE` does not exist. The file is at `web/web/LICENSE`. Because the
reference is not written as `./web/LICENSE`, the existing script skips it:
```
$ bash dev/release/check-license.sh . # before this PR
exit=0
output lines: 340
[NOT FOUND] count: 0
```
It exits `0` and prints 340 lines of `[OK]` noise, in which a genuine
failure would be easy to miss anyway. After this PR the same tree produces 9
lines and a non-zero exit:
```
[NOT FOUND] LICENSE:204 --> web/LICENSE
different licenses, please see web/LICENSE for these.
candidates:
./clients/client-python/LICENSE
./LICENSE
./web/web/LICENSE
./mcp-server/LICENSE
./web-v2/web/LICENSE
Some files referenced by the LICENSE or NOTICE files are missing.
exit=1
```
The correct path appears in the candidate list.
The silent-pass bug is the more serious of the two, since it makes the check
unreliable rather than merely incomplete. Pointed at a directory with no
`LICENSE`, the current script reports success:
```
cat: /tmp/.../LICENSE: No such file or directory
All files listed in the LICENSE file are present.
exit=0
```
Fix: #13255
### Does this PR introduce _any_ user-facing change?
No. This is a release-tooling script. Its output format changes, but no CI
job or Gradle task invokes it.
### How was this patch tested?
Manually, against four scenarios, plus `shellcheck`.
1. **Unfixed tree** (current `main`, the reference to `web/LICENSE` still
broken) — exits `1` and reports the reference with its line number and
candidates, as shown above. This is the defect the previous version passed over.
2. **Fixed `LICENSE` and `NOTICE`** (taken from the corrected files in
#13254) — exits `0` with `All files referenced by the LICENSE and NOTICE files
are present.` Together with (1) this shows the check fails on exactly the
broken reference and passes once it is corrected, rather than being insensitive
in both directions.
3. **Directory containing neither document** — reports `LICENSE not found`
and `NOTICE not found` and exits `1`, where the previous version exited `0`.
4. **No argument given**, invoked from another directory — resolves the
project root correctly and reports the same finding as (1).
A document with no trailing newline was also checked, confirming its last
line is still read.
`LICENSE` and `NOTICE` were restored after the swap in (2) and verified
identical by SHA-256.
`shellcheck` is clean on the new version. The previous version reported
three `SC2046` word-splitting warnings on the unquoted `dirname`/`readlink`
nesting used to derive the project root; those are quoted now and the warnings
are gone.
--
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]
