rangareddy commented on PR #857: URL: https://github.com/apache/incubator-xtable/pull/857#issuecomment-5187891990
@vinishjail97 all ten threads are answered inline and the branch is force-pushed (`efbffc9` → `2f9ed96`). The PR description is rewritten, since most of it no longer matched the diff. Summary of the round: **Your two direct questions** - *Is the hadoop override required?* **No — you were right, it is deleted.** It took closing two POM-parsing gaps to make that true, and neither was a `~/.m2` miss: `hadoop-project-3.1.0.pom` contains `<Xlint:-unchecked/>`, which expat rejects outright, and `hadoop-yarn-client-2.7.1.pom` binds the Maven namespace to a *prefix* so every element is unqualified and the namespaced XPaths matched nothing. Both POMs are authentic — their SHA-1s match Central. Auditing the rest of the table on top of that removed **18** overrides in total. - *Will `errors="strict"` break this today?* **It would have, on `javax.activation:activation:1.1` and `javax.mail:mail:1.4.1`** (both latin-1 `META-INF/LICENSE.txt`), out of 363 embedded entries. After the classified-artifact fix neither is bundled any more, so the current run has zero decode failures. `notice_text_for` had none, so the pre-existing `errors="replace"` there was harmless — it is strict now too. **Things the review surfaced that turned out to be bugs, not gaps** - The `parts[3]` bug was not only aborting the run. Nine classified coordinates were being written into `LICENSE-bundled` with the **classifier in place of the version**, and six `ARTIFACT_OVERRIDES` existed solely to stop the resulting raise. `hbase-common:jar:tests` was also being counted as a bundled runtime dependency. - Your family/text assertion caught a real mis-attribution: **`junit` was shipping the Common Public License, but 4.12 is EPL 1.0.** Your note about 4.11 bundling Hamcrest's BSD text is correct, but 4.12 ships its own correct EPL text under `LICENSE-junit.txt`. So the override was wrong; it is removed and the committed text corrected. That also means the original commit message's justification for override-first was wrong for the version we bundle, so I rewrote it rather than let it merge. **One I found while running your scripts, unrelated to any thread** `validate_shaded_license_coverage.sh` was a **fail-open**. It builds the module list with `rg`, which `mvn-license-check.yml` never installs; on a machine without ripgrep the list came back empty and the script printed "No modules with maven-shade-plugin were found" and **exited 0**. The whole release license gate passed while checking nothing — which is also why your local run and mine could differ. Now `find`/`grep`, and an empty list fails. **Two things I deliberately did not decide** 1. **`javolution:javolution:5.5.1`** — POM says `BSD License`, the curated text is **MIT** with a **2017** copyright for a 2010 artifact, and the jar has no license file. It predates this PR (came in with #691). Writing a BSD text with a copyright line I cannot verify would be fabricating a legal document, so the generator refuses on it. Needs your call — details in the inline thread. 2. **`xtable-hive-metastore`'s shade `<includes>` has drifted** and the validator's pre-existing includes-vs-tree check fails on this branch *and on main* (this PR touches no `pom.xml`): 25 includes name dependencies no longer in the tree, 97 runtime dependencies are missing from the list. It reads like a list written for Hive 2.x and never updated for 3.1.3. **This is why I left the coordinate lists unchanged** — regenerating now would flip a currently-passing check to failing, because the committed metadata still carries those 25 dead entries. Happy to file it as its own issue and do the regeneration as a follow-up. Also: I'd flag `javax.servlet` and `javax.servlet.jsp`, whose overrides claim Apache-2.0 while their POMs say CDDL + GPLv2 w/ CPE. That downgrades Category B to A and suppresses license texts those dependencies would otherwise ship. I left them alone since it is a legal call, but it deserves a second opinion. Not rebased onto `main` yet (8 commits, none touching these files) — say the word and I'll rebase and re-run before you look. -- 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]
