ci: enrich check_binary_deps failure report with license + target file#5057
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5057 +/- ##
============================================
+ Coverage 43.06% 43.07% +0.01%
- Complexity 2206 2211 +5
============================================
Files 1045 1045
Lines 40220 40218 -2
Branches 4244 4243 -1
============================================
+ Hits 17319 17325 +6
+ Misses 21830 21827 -3
+ Partials 1071 1066 -5
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/request-review @aicam |
aicam
left a comment
There was a problem hiding this comment.
LGTM. Big quality-of-life improvement for the failure report — now tells contributors exactly which per-module LICENSE-binary to edit and what license the dep declares, so they can verify ASF Category A/B compatibility without leaving the CI log.
Verified locally
Unit tests:
$ python -m unittest test_check_binary_deps -v
Ran 34 tests in 0.005s
OK
All 34 tests pass, including the 4 new tests covering license parsing and report rendering (CollectNpmReturnsLicenses, CollectPythonReturnsLicenses, ReportRendersLicenseAndTargetFile).
Smoke test against a synthetic npm input:
$ python bin/licensing/check_binary_deps.py npm /tmp/test_npm.json
NEW npm packages not claimed by LICENSE-binary:
+ fake-new-pkg@1.0.0 (license: MIT) → add to frontend/LICENSE-binary
ACTION REQUIRED
1. Verify each dep's license is ASF Category A or B.
2. Add a bullet for each dep above to frontend/LICENSE-binary
under the matching license section (see existing
'npm-compatible token' bullets for format).
...
Exactly the actionable hint that was missing before. Cross-checked DEFAULT_TARGET_FILE against the CI invocations in .github/workflows/build.yml (lines 125, 218, 533, 608, 708) — paths line up with each module's actual LICENSE-binary location. The collect_npm / collect_python signature change from set → (set, dict) is consistently propagated to the single call site each.
Backward-compatible: explicit --license-binary <path> invocations (jar CI steps) still work — the target file shown becomes the explicit path.
What changes were proposed in this PR?
Enrich the failure output of
bin/licensing/check_binary_deps.pywith two pieces of context that previously required manual lookup for every offending package: the dep's declared license string (read from3rdpartylicenses.jsonfor npm/agent-npm orpip-licenses.csvfor python) and the per-moduleLICENSE-binaryfile to edit. Each bullet now renders as+ name@version (license: X) → add to <file>for new packages,→ remove from <file>for stale, and→ update in <file>for drift, with theACTION REQUIREDblock naming the same file. Transitive deps are covered the same way because the input files already list everything bundled. No CI workflow changes; The script's exit semantics and aggregation behavior are unchanged.Any related issues, documentation, or discussions?
Closes: #5056
How was this PR tested?
Added five unit tests in
bin/licensing/test_check_binary_deps.pycovering license rendering, target-file rendering, the jar no-license case, stale and drift hints, and the no-target-file fallback. Full suite of 34 tests passes locally viapython3 -m unittest discover -s bin/licensing -p "test_*.py". Also smoke-tested by running the script against a synthetic3rdpartylicenses.jsonand confirming the enriched bullets render correctly for new, stale, and drifted entries.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.7 in compliance with ASF