rangareddy opened a new issue, #19511:
URL: https://github.com/apache/hudi/issues/19511
`hudi-presto-bundle` was the first of three bundles that depend on
`hudi-hadoop-mr-bundle`, a shaded fat jar,
while shading the same library classes themselves. #19469 / #19490 handled
that one. Two remain:
- `packaging/hudi-hive-sync-bundle/pom.xml:167`
- `packaging/hudi-gcp-bundle/pom.xml:157`
Both list `org.apache.hudi:hudi-hadoop-mr` and
`org.apache.hudi:hudi-hadoop-common` in their own
`artifactSet`, so they shade those classes directly and have no need for the
fat jar as a dependency.
### Why this is not simply the presto change again
In the presto bundle, `hudi-hadoop-mr` was reached *only* through
`hudi-hadoop-mr-bundle`'s published
dependency-reduced POM, which is what made #19433's regression possible:
removing the fat jar emptied the
includes and shade did not complain. These two bundles are in a better
position — both reach the library
modules through `hudi-hive-sync`, a library:
```
$ mvn -o dependency:tree -Dincludes=org.apache.hudi -pl
packaging/hudi-hive-sync-bundle,packaging/hudi-gcp-bundle
hudi-hive-sync-bundle
+- hudi-common:compile
+- hudi-hadoop-mr-bundle:compile <-- the fat jar
\- hudi-hive-sync:compile
+- hudi-hadoop-common:compile <-- reached via the library, not
the bundle
+- hudi-hadoop-mr:compile
\- hudi-sync-common:compile
hudi-gcp-bundle
+- hudi-common:compile
+- hudi-hadoop-mr-bundle:compile <-- the fat jar
+- hudi-sync-common:compile
| \- hudi-hadoop-common:compile
+- hudi-hive-sync:compile
| \- hudi-hadoop-mr:compile
\- hudi-gcp:compile
```
So dropping the fat-jar dependency should not empty either `artifactSet` —
but "should not" is what the
presto bundle taught us to measure rather than assume.
### What needs measuring before either change
Reviewer feedback on #19490 established the recipe, and it is not optional
here:
1. **Both resolution paths, because they disagree.** A reactor build
resolves `hudi-hadoop-mr-bundle` from the
reactor and never reads its reduced POM; a repository build does. The
presto bundle carried 651
`org/codehaus/jackson/**` entries on one path and 0 on the other, at the
same commit. Releases go through
the reactor path (`scripts/release/deploy_staging_jars.sh` builds the
full reactor with no `-pl`), so that
is the one that defines the contract.
```
# repository path
mvn install -DskipTests -Dscala-2.12 -Dspark3.5 -Dflink1.20 -pl
packaging/<bundle>
# reactor path
mvn install -DskipTests -Dscala-2.12 -Dspark3.5 -Dflink1.20 -pl
packaging/hudi-hadoop-mr-bundle,packaging/<bundle>
```
2. **The jar entry listing, diffed and not just counted**, before and after,
on both paths.
3. **The published `dependency-reduced-pom.xml`**, parsed to
`groupId:artifactId:version:scope` and diffed.
With `promoteTransitiveDependencies` on, removing a dependency changes
what consumers resolve; on the
presto bundle this moved the parquet transitives from 1.10.1 back to the
bundle's own version and dropped
`it.unimi.dsi:fastutil` and `org.apache.parquet:parquet-format`. Both
bundles publish a POM that consumers
read, so that half of the contract needs the same treatment.
### Also worth checking while in here
Neither bundle has an `org.codehaus.jackson:*` include, so neither carries
the Jackson 1.x problem that
#19490 cleaned up. But `hudi-hive-sync-bundle` does include
`org.apache.parquet:parquet-*` without declaring
the artifacts, so it is worth confirming which dependency currently supplies
them, and whether that is the
fat jar.
### Context
- #19469 — the presto bundle, question 3 of which asked whether "bundles
depend on library modules only"
should be a general rule. Answered yes for one bundle; this issue is the
remaining two.
- #19433 — the regression that showed shade silently produces a smaller jar
when an `artifactSet` include
matches nothing.
- #19490 — the presto bundle change, whose review thread contains the
measurement discipline above.
- #19491 — `validate_presto_bundle.sh`, the CI gate that now asserts the
presto bundle's required classes.
Equivalent coverage for these two bundles does not exist and would be the
right thing to add alongside
any change here.
--
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]
