vinishjail97 commented on code in PR #857:
URL: https://github.com/apache/incubator-xtable/pull/857#discussion_r3717009198
##########
release/scripts/generate_shaded_license_metadata.py:
##########
@@ -471,7 +579,15 @@ def render_notice(module: pathlib.Path) -> str:
def main() -> None:
+ missing_texts: dict[pathlib.Path, list[tuple[str, str, str]]] = {}
for module in shade_modules():
+ if not (module / "target" / "dependency-tree-runtime.txt").exists():
Review Comment:
This skip is what makes an existing bug in `tree_coords()` reachable, and as
a result the generator cannot currently complete a run across all shade modules.
Once `xtable-utilities` has a dependency tree (which the AGENTS.md command
produces, and which this condition then no longer skips), the whole run aborts:
```
ValueError: Unmapped license for org.apache.hudi:hudi-io:shaded:
('__MISSING__',)
```
The root cause is at line 335, `coords.add((parts[0], parts[1], parts[3]))`,
which treats the fourth field as the version. A *classified* artifact has an
extra field, so for this line in
`xtable-utilities/target/dependency-tree-runtime.txt`:
```
org.apache.hudi:hudi-io:jar:shaded:1.2.0:compile
```
the version parses as the literal string `shaded`, the POM lookup misses,
and `normalize_family()` raises.
Two suggestions. Parse the version as `parts[-2]`, or branch on
`len(parts)`, so classified artifacts are handled. And consider giving the
generator the same explicit skip list the validator has
(`SKIPPED_SHADE_MODULES`, line 66) instead of relying on a tree file being
absent, since right now the validator skips `xtable-utilities` and the
generator does not, so CI never exercises the one module that breaks the
generator.
##########
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:
`normalize_family()` determines the license family from the dependency's
POM, and this function independently extracts some text from the dependency's
jar, but nothing verifies that the two agree.
The junit case in the `license_text_for()` docstring is real, I confirmed
that `junit-4.11.jar` ships a `LICENSE.txt` whose content is Hamcrest's BSD
text. But it is only handled because someone noticed, which makes
`license_overrides/` an allowlist of the misleading jars we happen to know
about, and a jar that lies without anyone noticing will ship the wrong license
text silently.
Would it be worth asserting that a marker string for the assigned family
appears in the extracted text, for instance "Mozilla Public License Version
2.0" for MPL 2.0 or "Common Public License" for CPL 1.0, and failing otherwise?
That converts the unknown cases into a build failure rather than a wrong
attribution.
##########
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:
+ path = jar_path(group, artifact, version)
+ if not path.exists():
+ return None
+ with zipfile.ZipFile(path) as jar_file:
+ names = set(jar_file.namelist())
+ for candidate in JAR_LICENSE_CANDIDATES:
+ if candidate not in names:
+ continue
+ text = jar_file.read(candidate).decode("utf-8", errors="replace")
+ lines = [line.rstrip() for line in text.replace("\r\n",
"\n").replace("\r", "\n").split("\n")]
+ normalized = "\n".join(lines).strip()
+ if normalized:
+ return normalized + "\n"
+ return None
+
+
+def license_text_for(group: str, artifact: str, version: str) -> str | None:
+ """Return the license text for a bundled dependency.
+
+ A curated override keyed by artifactId is consulted first: it is human
+ verified and is required for dependencies whose jar carries no license, or
a
+ misleading one (e.g. junit bundles Hamcrest's BSD text rather than its own
+ Common Public License). Otherwise the dependency's own jar is
authoritative.
+ Returns None when neither source has a text, so the caller can fail loudly
+ rather than ship an unattributed dependency.
+ """
+ override = OVERRIDES_DIR / f"LICENSE-{artifact}"
+ if override.exists():
+ content = override.read_text()
+ return content if content.endswith("\n") else content + "\n"
+
+ return license_text_from_jar(group, artifact, version)
+
+
+def bundled_non_apache_coords(license_path: pathlib.Path) -> list[tuple[str,
str, str]]:
+ """Parse the non-Apache dependency coordinates out of a LICENSE-bundled
file.
+
+ License texts are keyed off the committed LICENSE-bundled (the canonical
list
+ of what the shaded jar bundles) rather than a freshly resolved dependency
+ tree, so the texts stay reproducible regardless of the local ~/.m2 state.
+ """
+ coords: list[tuple[str, str, str]] = []
+ family = None
+ lines = license_path.read_text().splitlines()
+ for index, line in enumerate(lines):
+ stripped = line.strip()
+ if not stripped:
+ family = None
+ continue
+ if index + 1 < len(lines) and re.fullmatch(r"-{3,}", lines[index +
1].strip()):
+ family = stripped
+ continue
+ if re.fullmatch(r"-{3,}", stripped):
+ continue
+ if family and family != APACHE_FAMILY and stripped.count(":") == 2:
+ group, artifact, version = stripped.split(":")
+ coords.append((group, artifact, version))
+ return coords
+
+
+def sync_license_texts(module: pathlib.Path, coords: list[tuple[str, str,
str]]) -> list[tuple[str, str, str]]:
+ """Regenerate META-INF/licenses/ for a module; return coords with no
text."""
+ licenses_dir = module / "src" / "main" / "resources" / "META-INF" /
"licenses"
+ licenses_dir.mkdir(parents=True, exist_ok=True)
+
+ for stale in licenses_dir.glob("LICENSE-*"):
Review Comment:
The wipe happens before any text has been resolved, and `main()` only raises
at the very end after looping every module, so a failed run leaves the tree
half-regenerated rather than unchanged.
When I hit the `xtable-utilities` abort described above,
`xtable-hive-metastore` had already lost five texts: `LICENSE-activation`,
`LICENSE-commons-el`, `LICENSE-mail`, `LICENSE-oro` and
`LICENSE-protobuf-java`, with `LICENSE-bundled` and `NOTICE-bundled` rewritten.
`protobuf-java` is one of the dependencies #700 calls out by name, so having it
silently deleted by a failed run is unfortunate, especially since the resulting
tree still looks plausible.
Could this resolve all texts for the module first and only unlink and write
once they are all in hand? Then a failure leaves the tree untouched and the
script is safe to re-run.
##########
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:
This awk reimplements `bundled_non_apache_coords()` from the generator, and
the two detect a family heading differently: the Python version looks ahead for
the `---` underline, this one tracks `prevline`.
They do agree on the current files, I checked (5, 29 and 3 non-Apache-2.0
artifactIds for aws, hive-metastore and hudi-support-extensions, against 5, 29
and 3 texts on disk). But the generator and the validator silently disagreeing
about which dependencies need a text is exactly the failure mode this tooling
exists to prevent, and two parsers make that possible.
Would it be reasonable to expose the parse from the Python module and call
it from here, so there is a single implementation?
--
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]
