apache / incubator-xtable

Apache XTable (incubating) is a cross-table converter for lakehouse table formats that facilitates interoperability across data processing systems and query engines.
https://xtable.apache.org/
Apache License 2.0
919 stars 147 forks source link

[551] Create separate LICENSE and NOTICE files for xtable-hudi-support-extensions #572

Closed vinishjail97 closed 3 weeks ago

vinishjail97 commented 3 weeks ago

Important Read

What is the purpose of the pull request

The bundled jar in xtable-hudi-support-extensions needs a different version of LICENSE and NOTICE file as it bundles MIT and BSD-3 license dependencies. Excluded hudi-common's transitive dependencies (hbase, hadoop etc.) from xtable-core by making it as provided to avoid the following non ASF compliant dependencies in the class path. Refer xtable-hudi-support-extensions tab in the sheet.

  1. org.openjdk.jol:jol-core:jar:0.16
  2. javax.activation:javax.activation-api:jar:1.2.0
  3. javax.ws.rs:javax.ws.rs-api:jar:2.1.1
  4. org.glassfish.web:javax.servlet.jsp:jar:2.3.2
  5. org.glassfish:javax.el:jar:3.0.0
  6. javax.xml.bind:jaxb-api:jar:2.2.11

Brief change log

(for example:)

Verify this pull request

(Please pick either of the following options)

This pull request is a trivial rework / code cleanup without any test coverage.

Verified changes locally by building jar.

mvn clean install -DskipTests
# Verified the contents of LICENSE and NOTICE.
jar xf ./xtable-hudi-support/xtable-hudi-support-extensions/target/xtable-hudi-support-extensions_2.12-0.2.0-SNAPSHOT-bundled.jar META-INF/ 
# Verified there's no presence of non-ASF compliant dependency in the class path. 
jar tf xtable-hudi-support/xtable-hudi-support-extensions/target/xtable-hudi-support-extensions_2.12-0.2.0-SNAPSHOT-bundled.jar | sort > jarOutput.txt 
vinishjail97 commented 3 weeks ago

Overall the changes look reasonable. I added a few suggestions for improvements. Apart from that I didn't verify the content of the jar file to see if the LICENSE info is complete. I will try to do that once all the other comments are addressed.

Thanks for the feedback @zabetak, addressed comments. You can refer the google sheet as well for checking licenses for transitive dependencies in the bundled jar in this section, but yeah you can do your independent check as well for the presence of non ASF compliant classes. Bundled Depdencies through maven-shade-plugin (After making hudi-common as provided)

vinishjail97 commented 3 weeks ago

Appreciate the patience on this @zabetak, have addressed the issues you pointed out.

jar tf xtable-hudi-support/xtable-hudi-support-extensions/target/xtable-hudi-support-extensions_2.12-0.2.0-SNAPSHOT-bundled.jar | grep NOTICE
META-INF/NOTICE
vinishjail97 commented 3 weeks ago

@zabetak Updated, my bad didn't notice LICENSE file when re-ordering the transformers for maven shade.

jar tf xtable-hudi-support/xtable-hudi-support-extensions/target/xtable-hudi-support-extensions_2.12-0.2.0-SNAPSHOT-bundled.jar | grep NOTICE
META-INF/NOTICE

jar tf xtable-hudi-support/xtable-hudi-support-extensions/target/xtable-hudi-support-extensions_2.12-0.2.0-SNAPSHOT-bundled.jar | grep LICENSE
META-INF/licenses/LICENSE-antlr4-runtime
META-INF/licenses/LICENSE-checker-qual
META-INF/licenses/LICENSE-javax-annotation-api
META-INF/licenses/LICENSE-paranamer
META-INF/licenses/LICENSE-slf4j-api
META-INF/LICENSE
vinishjail97 commented 3 weeks ago

LGTM! Many thanks for your efforts Vinish and apologies for the multiple review rounds.

Appreciate your patience and reviews as well to get LICENSE and NOTICE related work in good shape.

vinishjail97 commented 3 weeks ago

Squash into a single commit.