rangareddy commented on code in PR #857:
URL: https://github.com/apache/incubator-xtable/pull/857#discussion_r3718153106
##########
release/scripts/validate_shaded_license_coverage.sh:
##########
@@ -254,6 +254,38 @@ for module in "${SHADE_MODULES[@]}"; do
printf ' - %s\n' "${unknown[@]}"
overall_status=1
fi
+
+ # Every dependency bundled under a non-Apache-2.0 license must ship its own
+ # license text under META-INF/licenses/LICENSE-<artifactId>. The Apache
banner
+ # at the top of LICENSE-bundled only covers the Apache License 2.0 family.
+ licenses_dir="${module}/src/main/resources/META-INF/licenses"
+ missing_license_texts=()
+ while IFS= read -r artifact_id; do
+ [[ -z "${artifact_id}" ]] && continue
+ if [[ ! -f "${licenses_dir}/LICENSE-${artifact_id}" ]]; then
+ missing_license_texts+=("${artifact_id}")
+ fi
+ done < <(
+ awk '
Review Comment:
Yes, and done: the generator now has a `--non-apache-artifact-ids
<LICENSE-bundled>` mode and the validator calls it, so the awk reimplementation
is deleted. `bundled_non_apache_coords()` is the single implementation, which
is also why I kept it rather than deleting it as the other comment suggested.
You were right to be uneasy about the two parsers agreeing only by luck —
the counts matched on the current files but the heading detection genuinely
differed, and that class of drift can no longer happen.
##########
release/scripts/generate_shaded_license_metadata.py:
##########
@@ -447,6 +475,86 @@ def notice_text_for(group: str, artifact: str, version:
str) -> str | None:
return None
+@lru_cache(maxsize=None)
+def license_text_from_jar(group: str, artifact: str, version: str) -> str |
None:
Review Comment:
Worth it, yes — implemented as `FAMILY_TEXT_MARKERS` plus
`text_matches_family()`, checked in `resolve_license_texts()`. Families absent
from the table are not checked, so entries can be added incrementally rather
than needing an exhaustive list up front. Matching collapses runs of
whitespace, since license texts are hand-wrapped and a marker phrase otherwise
fails on formatting alone.
It immediately found four discrepancies, three of them real:
1. **`org.slf4j:slf4j-api:2.0.9`** — a false positive from my own first
marker: slf4j's MIT text reads `free of charge` with two spaces. Fixed by the
whitespace-insensitive match, not by weakening the check.
2. **`xml-apis:xml-apis:1.3.04`** — the override claimed ASL 1.1 while the
POM and the jar both say Apache 2.0. Override removed.
3. **`junit:junit:4.12`** — this one matters. The curated override shipped
the **Common Public License**, but junit 4.12 is **EPL 1.0**: its POM says so
and its jar carries the correct EPL text, just under the non-standard name
`LICENSE-junit.txt`. So the wrong text was being shipped in
`xtable-hive-metastore`.
Your observation about junit 4.11 shipping Hamcrest's BSD text is
correct, but 4.12 is what is bundled here and it does ship its own license. The
CPL override is deleted, a last-resort fallback picks up `LICEN[SC]E*` at the
jar root or in `META-INF/`, and the committed `LICENSE-junit` is corrected to
EPL 1.0. I have also rewritten the commit message, which cited junit/CPL as the
motivating example for overrides — that claim was wrong for the version we
bundle and should not outlive the PR.
4. **`javolution:javolution:5.5.1`** — genuinely unresolved, and I would
rather not guess. Its POM declares `BSD License` (url
`http://javolution.org/LICENSE.txt`, long dead), the curated text in the repo
is the **MIT** license with a **2017** copyright for a 2010 artifact, and the
jar carries no license file at all. The text predates this PR — it came in with
#691 and #857 only moved it into `license_overrides/`.
Writing a BSD text with a copyright line I cannot verify would be
fabricating a legal document, and accepting the MIT text means overriding the
artifact's own declared license on no evidence. So the generator currently
refuses on it, which is exactly the behaviour you asked this check to produce.
Options: confirm the MIT text's provenance and pin the family with a commented
`ARTIFACT_OVERRIDES` entry; or treat the POM as authoritative and source
javolution 5.5.1's real BSD text from an archived copy; or take it to
legal-discuss. Your call — happy to do any of them.
Your framing of `license_overrides/` as "an allowlist of the misleading jars
we happen to know about" was the useful part here: two of the four above were
exactly that.
--
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]
