[700] Bundle license texts for all non-Apache-2.0 shaded dependencies - #857
Conversation
50ac2c9 to
efbffc9
Compare
|
@rangareddy I have ran the scripts in my local and it was breaking for few dependencies. Can you address the comments? I will take a look tomorrow again and merge for 0.4.0-incubating rc2. |
efbffc9 to
2f9ed96
Compare
|
@vinishjail97 all ten threads are answered inline and the branch is force-pushed ( Your two direct questions
Things the review surfaced that turned out to be bugs, not gaps
One I found while running your scripts, unrelated to any thread
Two things I deliberately did not decide
Also: I'd flag Not rebased onto |
ff95274 to
a611ada
Compare
|
@vinishjail97 the License Check failure on the previous push was the new gate working, and it turned up something worth knowing before rc2. Pushed The License Check has never actually run in CI. The script builds its module list with Every one of the last ten runs on What that was hiding Replacing To confirm this is not something I introduced: this PR touches no How I have handled it, and why not more Filed as #880 with the full evidence, and this PR adds an explicit one-module I did not fix the drift here because the two candidate fixes have materially different consequences for the published artifact:
That is a release/design call rather than a tooling fix, so it seemed wrong to decide it inside a license-text PR — and worse to leave the gate vacuous in order to avoid it. Happy to do either once you or Two things you may want to consider separately, given the gate has not been effective on any branch:
|
a611ada to
190a477
Compare
|
Replacing my earlier comment on scope, now that I have a concrete alternative to point at rather than just a suggestion. Suggestion: check the built jar instead of generating from the pomsI pushed one as #885 — Reading the artifact rather than the dependency tree removes the need for POM family resolution, the override tables, the parse fallbacks and the On duplicationMeasured across the 51 license files in this PR: 25 distinct texts, 9,050 lines of content, of which 2,681 are distinct and 6,369 are duplicates. The single largest item is the CDDL 1.1 text — 759 lines, committed 8 times: Two causes:
One process requestCould the description be trimmed? It has grown past what a reviewer can hold in one pass, and parts of it no longer match the head commit: open item 1 still says the generator refuses on Numbers above come from builds of both |
190a477 to
71436f4
Compare
|
Thanks — I reproduced all of your numbers exactly (51/25/9050/2681/6369, the CDDL 759×8, and 653 lines of code), and both description defects were real. Fixed in
Gates: rat pass, validator exit 0 (6 OK, no FAIL), generator idempotent. I also built and ran #885 rather than just reading it. Four things worth knowing before choosing between them:
None of that is fatal, and the approach genuinely finds things mine misses: on #857 alone it flags 7 stale texts ( And it caught a real bug in #880, which I'd rather you knew now: reconciling |
71436f4 to
4c20169
Compare
|
Took the duplication point from #885 and applied it —
3,746 duplicate lines removed. PR is The 2,623 that remain are the shipped per-module copies — the Verified: all 28 shipped texts regenerate byte-identical, so the jars are unchanged; the generator is idempotent; and deleting a curated text now fails the run naming the path to commit. Gates: rat pass, validator exit 0 (6 OK, no FAIL). A negative test caught one bug in the refactor — the failure path still referenced the removed directory and raised I did not swap the generator for #885's checker. Beyond the four defects in the earlier comment, it reads the coordinate listing from the jar it is validating, so it cannot see a dependency that is bundled but absent from that listing — I removed One thing that surfaced while testing that integration, worth its own issue: the jar's own |
There was a problem hiding this comment.
This is still being unnecessarily complicated...here's what we will do to close the issue from a functional perspective.
- Add validate_bundle_license_texts and run it as part of GH workflow mvn-license-check..this is similar to the validation ASF mentors run on the release bundled jars.
- Add/Edit/Remove license files to get the validations passing.
The scripts approach of reading pom.xml and trying to assess has pitfalls for bundled jars and that's lot of AI generated code in generated_shaded_license_metadata that's being added without tests. Adding a validation that works after building the jar and flagging the missing license texts is easier to maintain and understand.
The bundled (shaded) jars listed third-party dependencies in LICENSE-bundled but shipped no license text for many of them, even though the banner points to "licenses/ for text of these licenses". This adds the missing texts and makes the tooling keep them complete and accurate. generate_shaded_license_metadata.py: - Regenerates each module's META-INF/licenses/ directory: for every non-Apache-2.0 bundled dependency it writes LICENSE-<artifactId>, sourced from a curated override first, then the dependency's own jar, and fails loudly when a dependency has neither. - Resolves everything a run would write before writing any of it. A failure used to leave texts deleted and LICENSE-bundled/NOTICE-bundled rewritten for the modules already processed, so the only recovery was to discard the whole run; a failing run is now a no-op across all modules. - Parses the dependency tree correctly for classified artifacts. The version is the second-to-last field, not parts[3], which is the classifier on a six-field line. This aborted the run on org.apache.hudi:hudi-io:jar:shaded and, for the nine classified coordinates that an override happened to cover, silently recorded a classifier where LICENSE-bundled wants a version (io.netty:netty-transport-native-epoll:linux-aarch_64). A "tests" classifier is now excluded alongside test-jar. - Reads POMs that Maven accepts but ElementTree does not. hadoop-project-3.1.0 contains <Xlint:-unchecked/>, which expat rejects outright, and hadoop-yarn-client-2.7.1 binds the Maven namespace to a prefix rather than as the default namespace, leaving every element unqualified so the namespaced XPaths matched nothing. Both POMs are authentic; their SHA-1s match Maven Central. License lookups now match on local name and fall back to a text scan when a POM will not parse. - Drops 18 license-family overrides that only existed because of those three parsing bugs, including org.apache.hadoop, and drops the xml-apis override, which claimed Apache Software License 1.1 while both the POM and the jar's own license/LICENSE carry the Apache 2.0 text. - Checks that a dependency's license text is credible as the family resolved from its POM. Nothing previously tied the two together, so a jar shipping the wrong license file was attributed wrongly and silently. - Derives the bundled set from the runtime tree intersected with the shade <includes>, so the metadata describes what is inside the jar rather than what is on the classpath. - Refuses to run when a shade module has no dependency tree, rather than regenerating the rest and producing a diff that looks complete but is not, and skips xtable-utilities in step with the validator. validate_shaded_license_coverage.sh: - Fails when a non-Apache-2.0 dependency in LICENSE-bundled has no META-INF/licenses/LICENSE-<artifactId>, and also in the reverse direction, when a license text has no bundled dependency behind it. Checking one direction only is what let orphaned texts accumulate (apache#865). - Builds its module list with find and grep instead of ripgrep. ripgrep is not installed by mvn-license-check.yml and is not present on the GitHub ubuntu-latest runner, so this gate has never actually run in CI: the call sits in a process substitution where errexit cannot see it fail, the module list came back empty, and the script printed "No modules with maven-shade-plugin were found" and exited 0. Every License Check run on main reports success without validating anything. An empty module list is now a failure. - Downgrades the includes-vs-tree comparison to a warning for xtable-hive-metastore only, via an explicit KNOWN_INCLUDES_DRIFT list. That module's include list was written for a Hive 2.x dependency set: 25 entries name dependencies that are no longer resolved and 97 runtime dependencies are missing from it. Reconciling them changes what the shaded jar bundles, so it is a release decision rather than a tooling fix, tracked in apache#880. Running main's own unmodified validator with ripgrep available reproduces the same failure, so the drift is pre-existing; fixing the fail-open above is simply what made it visible. Every other check, for every module, stays blocking. - Parses LICENSE-bundled by calling the generator rather than reimplementing the parse in awk. The two implementations detected a family heading differently and could disagree about which dependencies need a text. javolution's family is recorded as MIT rather than the BSD its 5.5.1 POM declares. The project relicensed: that POM was published in 2010 and points at http://javolution.org/LICENSE.txt, which is long dead, there is no 5.5.1 tag from which the 2010 text could be recovered, and the LICENSE the project publishes today is MIT. license_overrides/LICENSE-javolution is that text verbatim, so the recorded family now matches the text actually shipped. Both are ASF Category A, so the artifact's licensing category is unchanged. This is what the new family-vs-text check flagged. junit is corrected from the Common Public License to the Eclipse Public License 1.0. An earlier revision of this change cited junit as the case where a jar ships someone else's license text and so a curated override must win. That is true of junit 4.11, which ships Hamcrest's BSD text, but 4.12 is what is bundled here and it ships its own correct EPL 1.0 text under the non-standard name LICENSE-junit.txt. The curated CPL override was therefore wrong and is removed; a last-resort jar-filename fallback picks the text up instead. Curated texts are no longer committed twice. release/scripts/license_overrides/ held a second copy of every hand-curated text purely to feed the generator, and 1405 of its 1520 lines were byte-identical to a shaded module's committed META-INF/licenses copy. The generator now reads those texts from the committed metadata, guided by a CURATED_TEXTS list naming the artifacts whose own jar carries no usable license; everything else still comes from the dependency jar, so an upgrade picks up an updated text. The directory is removed. Regenerating produces byte-identical output for all 28 shipped texts, and a missing curated text fails the run with the path to commit rather than silently shipping nothing. Coordinate lists in LICENSE-bundled/NOTICE-bundled are unchanged. Regenerating them requires first reconciling xtable-hive-metastore's shade <includes> with its runtime tree, which drifted over a Hive major version and is tracked in apache#880. Closes apache#700
Check the license texts against the artifact that ships rather than the poms. The bundled jar is what a release reviewer inspects, and it is the only thing that settles what is actually bundled: a shade <includes> list can name dependencies that no longer resolve, and dependency:tree does not describe what an uber-jar dependency contributes. release/scripts/validate_bundled_license_texts.py reads each *-bundled.jar, takes the coordinate listing from the jar's own META-INF/LICENSE, confirms each non-Apache-2.0 coordinate's classes are genuinely present, and requires META-INF/licenses/LICENSE-<artifactId>. It fails in both directions, so a text left behind for a dependency that is no longer bundled fails too (apache#865). It runs in mvn-license-check after a package of the shaded modules. The orphan check considers only the LICENSE-<artifactId> texts this project curates. A dependency may ship its own META-INF/licenses/ directory -- groovy-all carries antlr2-license.txt, asm-license.txt and three more, and its own META-INF/LICENSE refers to them for code compiled into it. Those are attribution the bundle must keep, not orphans to delete. Removes seven texts for dependencies that are not in the jar: activation, asm, asm-commons, asm-tree, commons-el, mail and oro. They were carried because the committed LICENSE-bundled still lists coordinates from an older dependency set. Drops the generator rework from this change. Resolving license families from poms has pitfalls the jar does not, and that work is better judged on its own with tests behind it.
…y artifact Two fixes for the License Check and Maven CI Build failures. spotless runs black over **/*.py and black is not installed on the runner, so any new script fails the build outright. generate_shaded_license_metadata.py is already excluded for this reason; the new script needs the same treatment. dependency_classes() keyed strictly on the version in the listing. Where that version has gone stale -- the listing says jersey 1.9 and 1.19 is what builds -- only the resolved version is ever downloaded, so nothing is found in ~/.m2, the dependency reads as absent from the jar, and its committed text looks like an orphan. That is why the check passed locally, against a repository that still had the older jars cached, and failed on a clean runner for jcodings, jersey-client, jersey-guice, jersey-json, joni and junit. Any version of the same groupId:artifactId answers the question being asked, which is whether this artifact's classes are in the bundle. Whether the listed version is correct is a separate problem, and one this check cannot settle.
ca7176d to
7839a1a
Compare
…fact Read the dependency tree for exact coordinates and confirm a dependency is bundled by checking that every class it declares is in the shaded jar. Nothing is sampled or thresholded: with the resolved version in hand, 207 of 213 dependencies match exactly and the other 6 differ only by module-info.class, which shade cannot merge. The tree must not be scope-filtered, since -Dscope=runtime drops protobuf-java, whose path runs through a provided node and whose classes are in the jar. A dependency's license comes from its own pom, walking the parent chain, and from the license file it ships when the chain declares none. That is how javax.transaction:jta and transaction-api are found to be CDDL 1.0 rather than the Apache-2.0 they were previously recorded as, so both now ship the text their own jars carry. Committed texts are also checked against the declared license, which catches a text for the wrong license or the wrong version of it. xml-apis and stax-api are removed: both declare Apache-2.0, which the banner at the top of LICENSE-bundled already covers. KNOWN_TEXT_MISMATCHES records the two texts that cannot be corrected from anything the artifacts carry. jamon-runtime declares MPL 1.1 and ships MPL 2.0 text; javolution 5.5.1 declares BSD at a dead URL and ships the project's later MIT text. Both dependencies are genuinely bundled, so the texts cannot be dropped either.
Comments only. The module docstring now lays out the call order, so the script can be read top down, and every function says in one line what it answers.
What
LICENSE-bundledsays "See licenses/ for text of these licenses", but many bundled non-Apache-2.0 dependencies shipped no text. This adds the missing texts and a check that keeps them correct.Closes #700. Supersedes #866 and #885.
Approach
The check reads the built
*-bundled.jarrather than the poms, because the artifact is the only thing that settles what is actually bundled — and it is what a release reviewer inspects.release/scripts/validate_bundled_license_texts.pytakes the exact coordinates Maven resolved fromdependency:tree, keeps the ones the shade plugin bundles, and confirms each is present by checking that every class it declares is in the jar. Nothing is sampled or thresholded: with the resolved version in hand, 207 of 213 dependencies match exactly and the other 6 differ only bymodule-info.class, which shade cannot merge.Each dependency's license comes from its own pom, walking the parent chain, and from the license file it ships when the chain declares none. Anything not Apache-2.0 must have
META-INF/licenses/LICENSE-<artifactId>, and the committed text is checked against the declared license so a text for the wrong license, or the wrong version of one, is caught. It fails in the other direction too, so a text with no bundled dependency behind it is reported (#865).It runs in
mvn-license-checkafter apackageof the shaded modules.What this corrects
javax.transaction:jtaandtransaction-apiare CDDL 1.0, not Apache-2.0. Both ship that text in their own jars; they were previously recorded as Apache-2.0 and shipped no text at all. Both now ship the text their jars carry.activation,asm,asm-commons,asm-tree,commons-el,mail,oro.xml-apisandstax-apiremoved: both declare Apache-2.0, which the banner at the top ofLICENSE-bundledalready covers.Result:
xtable-aws5 texts,xtable-hive-metastore22,xtable-hudi-support-extensions3.Known gaps
KNOWN_TEXT_MISMATCHESrecords two texts that cannot be corrected from anything the artifacts carry.jamon-runtimedeclares MPL 1.1 throughjamon-project-1.0.2and the committed text is MPL 2.0.javolution5.5.1 declares BSD at a dead URL and the committed text is the project's later MIT relicense. Both dependencies are genuinely bundled, so the texts cannot be dropped either. Thejamon-runtimemismatch predates this PR and is worth its own issue.Two things this deliberately does not do. It does not regenerate
LICENSE-bundled, which is stale in ways tracked separately — jersey is listed at 1.9 where 1.19 resolves, junit at 4.11 where 4.12 does. And it does not cover third-party code compiled inside a bundled dependency:groovyjarjarantlrandgroovyjarjarasmcome fromgroovy-alland attribute to it, so 308 classes are not separately listed. That is the listing-completeness half of #700 and needs a different check.Testing
Run on a clean build of all three shaded modules:
apache-rat:check— passspotless:check— passvalidate_shaded_license_coverage.sh— passvalidate_bundled_license_texts.py— exit 0,5 / 22 / 3dependencies, no missing, mismatched or orphaned textsEmptying
KNOWN_TEXT_MISMATCHESflips the run to exit 1 with both entries reported, so the allowlist is load-bearing rather than decorative.Note on authorship
The curated license texts are @rangareddy's work and are the part of this that has to happen whichever mechanism produces the metadata. The later commits replace the pom-based generator rework with the jar-based check described above.