[CELEBORN-1003] Correct the LICENSE and NOTICE for shaded client jars #1933

pan3793 · 2023-09-21T15:00:03Z

What changes were proposed in this pull request?

Correct the LICENSE and NOTICE for the following shaded client jars

celeborn-client-flink-1.14-shaded_2.12-<version>.jar
celeborn-client-flink-1.15-shaded_2.12-<version>.jar
celeborn-client-flink-1.17-shaded_2.12-<version>.jar
celeborn-client-mr-shaded_2.12-<version>.jar
celeborn-client-spark-2-shaded_2.11-<version>.jar
celeborn-client-spark-3-shaded_2.12-<version>.jar

Why are the changes needed?

The LICENSE and NOTICE shipped in a jar should match the content of the jar, for shaded jars, it should acknowledge all the third-party classes that are bundled.

See more discussion at https://lists.apache.org/thread/8v4wy5o132rpsjync6465zztgjlf6h5p

For how to determine which third-party jars are bundled, take celeborn-client-spark-3-shaded_2.12-<version>.jar as an example, the following command performs the packaging, and we can find them out by looking at logs like Including ... in the shaded jar

build/mvn clean package -DskipTests -pl :celeborn-client-spark-3-shaded_2.12 -am -Pspark-3.3

[INFO] --- maven-shade-plugin:3.4.0:shade (default) @ celeborn-client-spark-3-shaded_2.12 ---
[INFO] Including org.apache.celeborn:celeborn-client-spark-3_2.12:jar:0.4.0-SNAPSHOT in the shaded jar.
[INFO] Including org.apache.celeborn:celeborn-common_2.12:jar:0.4.0-SNAPSHOT in the shaded jar.
[INFO] Including org.apache.commons:commons-lang3:jar:3.12.0 in the shaded jar.
[INFO] Including io.netty:netty-all:jar:4.1.93.Final in the shaded jar.
[INFO] Including io.netty:netty-buffer:jar:4.1.93.Final in the shaded jar.
...
[INFO] Excluding org.apache.ratis:ratis-common:jar:2.5.1 from the shaded jar.
[INFO] Excluding org.apache.ratis:ratis-thirdparty-misc:jar:1.0.4 from the shaded jar.
[INFO] Excluding org.apache.ratis:ratis-proto:jar:2.5.1 from the shaded jar.
...

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manually.

pan3793 · 2023-09-21T15:04:40Z

client-flink/flink-1.14-shaded/pom.xml

+                <exclude>META-INF/LICENSE.txt</exclude>
+                <exclude>META-INF/NOTICE.txt</exclude>
+                <exclude>LICENSE.txt</exclude>
+                <exclude>NOTICE.txt</exclude>


@Apache9 the suggested ApacheLicenseResourceTransformer and ApacheNoticeResourceTransformer in [1] seem not to work as expected, I finally chose to filter those files out by rules.

[1] https://lists.apache.org/thread/8v4wy5o132rpsjync6465zztgjlf6h5p

If there are conflict LICENSE file then the ApacheLicenseResourceTransformer will ignore them, but I think ApacheNoticeResourceTransformer will aggregate all the NOTICE files into one. Anyway, you can do it manually but the problem is once you add new dependencies, you still need to manually modify the NOTICE file.

pan3793 · 2023-09-21T15:07:05Z

@pjfanning I would appreciate it if you could take a look at this license issue fix

waitinfuture · 2023-09-24T14:12:06Z

LGTM, thanks! @FMX could you also TAL?

Apache9 · 2023-09-25T00:52:04Z

client-flink/flink-1.14-shaded/pom.xml

+                <exclude>META-INF/LICENSE.txt</exclude>
+                <exclude>META-INF/NOTICE.txt</exclude>
+                <exclude>LICENSE.txt</exclude>
+                <exclude>NOTICE.txt</exclude>


If there are conflict LICENSE file then the ApacheLicenseResourceTransformer will ignore them, but I think ApacheNoticeResourceTransformer will aggregate all the NOTICE files into one. Anyway, you can do it manually but the problem is once you add new dependencies, you still need to manually modify the NOTICE file.

Apache9 · 2023-09-25T00:53:34Z

client-flink/flink-1.14-shaded/src/main/resources/META-INF/LICENSE

+This project bundles the following dependencies under the Apache Software License 2.0 (http://www.apache.org/licenses/LICENSE-2.0.txt):
+
+
+Apache Software Foundation License 2.0


Is this the correct name of this license?

Emm... seems not right, should be

- "Apache Software Foundation License 2.0" + "Apache License 2.0"

?

BTW, it was copied from Apache Spark...

Apache9 · 2023-09-25T00:55:09Z

client-flink/flink-1.14-shaded/src/main/resources/META-INF/NOTICE

+The Apache Software Foundation (https://www.apache.org/).
+
+=============================================================================
+= NOTICE file corresponding to section 4d of the Apache License Version 2.0 =


What about netty and guava? And I think we also need a section about protobuf?

there is no NOTICE.* inside netty and guava jars, how should we write in such cases?

client-spark/spark-3-shaded/src/main/resources/META-INF/LICENSE

pjfanning · 2023-09-25T10:25:11Z

@pjfanning I would appreciate it if you could take a look at this license issue fix

Definitely looks to be on the right track. When this is complete and merged, I can have a look at the jars.

codecov · 2023-09-28T11:07:02Z

Codecov Report

Merging #1933 (ccfbc67) into main (a9ed7f6) will decrease coverage by 0.17%.
Report is 2 commits behind head on main.
The diff coverage is n/a.

❗ Current head ccfbc67 differs from pull request most recent head 8343035. Consider uploading reports for the commit 8343035 to get more accurate results

@@            Coverage Diff             @@
##             main    #1933      +/-   ##
==========================================
- Coverage   46.54%   46.36%   -0.17%     
==========================================
  Files         164      164              
  Lines       10337    10337              
  Branches      953      953              
==========================================
- Hits         4810     4792      -18     
- Misses       5213     5227      +14     
- Partials      314      318       +4

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pan3793 · 2023-09-28T11:23:16Z

All comments are addressed, I'm going to merge it and backport to branch-0.3, then start the next RC of 0.3.1

Correct the `LICENSE` and `NOTICE` for the following shaded client jars - `celeborn-client-flink-1.14-shaded_2.12-<version>.jar` - `celeborn-client-flink-1.15-shaded_2.12-<version>.jar` - `celeborn-client-flink-1.17-shaded_2.12-<version>.jar` - `celeborn-client-mr-shaded_2.12-<version>.jar` - `celeborn-client-spark-2-shaded_2.11-<version>.jar` - `celeborn-client-spark-3-shaded_2.12-<version>.jar` The `LICENSE` and `NOTICE` shipped in a jar should match the content of the jar, for shaded jars, it should acknowledge all the third-party classes that are bundled. See more discussion at https://lists.apache.org/thread/8v4wy5o132rpsjync6465zztgjlf6h5p For how to determine which third-party jars are bundled, take `celeborn-client-spark-3-shaded_2.12-<version>.jar` as an example, the following command performs the packaging, and we can find them out by looking at logs like `Including ... in the shaded jar` ``` build/mvn clean package -DskipTests -pl :celeborn-client-spark-3-shaded_2.12 -am -Pspark-3.3 ``` ``` [INFO] --- maven-shade-plugin:3.4.0:shade (default) celeborn-client-spark-3-shaded_2.12 --- [INFO] Including org.apache.celeborn:celeborn-client-spark-3_2.12:jar:0.4.0-SNAPSHOT in the shaded jar. [INFO] Including org.apache.celeborn:celeborn-common_2.12:jar:0.4.0-SNAPSHOT in the shaded jar. [INFO] Including org.apache.commons:commons-lang3:jar:3.12.0 in the shaded jar. [INFO] Including io.netty:netty-all:jar:4.1.93.Final in the shaded jar. [INFO] Including io.netty:netty-buffer:jar:4.1.93.Final in the shaded jar. ... [INFO] Excluding org.apache.ratis:ratis-common:jar:2.5.1 from the shaded jar. [INFO] Excluding org.apache.ratis:ratis-thirdparty-misc:jar:1.0.4 from the shaded jar. [INFO] Excluding org.apache.ratis:ratis-proto:jar:2.5.1 from the shaded jar. ... ``` No. Manually. Closes #1933 from pan3793/CELEBORN-1003. Authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit 78553f1) Signed-off-by: Cheng Pan <[email protected]>

pan3793 force-pushed the CELEBORN-1003 branch from 6deac67 to aa5e40f Compare September 21, 2023 15:01

pan3793 commented Sep 21, 2023

View reviewed changes

pan3793 requested review from FMX and waitinfuture and removed request for FMX September 21, 2023 15:07

Apache9 reviewed Sep 25, 2023

View reviewed changes

FMX reviewed Sep 25, 2023

View reviewed changes

client-spark/spark-3-shaded/src/main/resources/META-INF/LICENSE Show resolved Hide resolved

client-spark/spark-3-shaded/src/main/resources/META-INF/LICENSE Outdated Show resolved Hide resolved

client-spark/spark-3-shaded/src/main/resources/META-INF/LICENSE Outdated Show resolved Hide resolved

pan3793 added 3 commits September 28, 2023 18:47

[CELEBORN-1003] Correct the LICENSE and NOTICE for shaded client jars

9a5d408

Correct name of Apache License 2.0

3c70a26

Exclude macOS libs from LICENSE

ccfbc67

pan3793 force-pushed the CELEBORN-1003 branch from aa5e40f to ccfbc67 Compare September 28, 2023 10:54

nit

8343035

pan3793 closed this in 78553f1 Sep 28, 2023

pan3793 mentioned this pull request Sep 28, 2023

[CELEBORN-1002] Add SBT MRClientProject #1930

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CELEBORN-1003] Correct the LICENSE and NOTICE for shaded client jars #1933

[CELEBORN-1003] Correct the LICENSE and NOTICE for shaded client jars #1933

pan3793 commented Sep 21, 2023

pan3793 Sep 21, 2023

Apache9 Sep 25, 2023

pan3793 commented Sep 21, 2023

waitinfuture commented Sep 24, 2023

Apache9 Sep 25, 2023

Apache9 Sep 25, 2023

pan3793 Sep 25, 2023

pan3793 Sep 25, 2023

Apache9 Sep 25, 2023

pan3793 Sep 25, 2023

pjfanning commented Sep 25, 2023

codecov bot commented Sep 28, 2023 •

edited

Loading

pan3793 commented Sep 28, 2023

		This project bundles the following dependencies under the Apache Software License 2.0 (http://www.apache.org/licenses/LICENSE-2.0.txt):


		Apache Software Foundation License 2.0

[CELEBORN-1003] Correct the LICENSE and NOTICE for shaded client jars #1933

[CELEBORN-1003] Correct the LICENSE and NOTICE for shaded client jars #1933

Conversation

pan3793 commented Sep 21, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pan3793 commented Sep 21, 2023

waitinfuture commented Sep 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjfanning commented Sep 25, 2023

codecov bot commented Sep 28, 2023 • edited Loading

Codecov Report

pan3793 commented Sep 28, 2023

codecov bot commented Sep 28, 2023 •

edited

Loading