You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2023/10/10 04:43:20 UTC
[PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
cxzl25 opened a new pull request, #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967
### What changes were proposed in this pull request?
Flink/Spark jars packaged with SBT use the correct LICENSE and NOTICE.
### Why are the changes needed?
https://github.com/apache/incubator-celeborn/pull/1930#discussion_r1340410526
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378410862
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +691,8 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case PathList(ps@_*) if Assembly.isLicenseFile(ps.last) => licenseFileOnlyProjectMergeStrategy(ps.mkString(java.io.File.separator))
Review Comment:
sbtassembly.Assembly#isLicenseFile
```scala
/**
* Returns a flag if the given file is a license file
*
* @param fileName the given file to check
* @return flag representing if the file is a license file
*/
def isLicenseFile(fileName: String): Boolean = {
val LicenseFile = """(license|licence|notice|copying)([.]\w+)?$""".r
fileName.toLowerCase match {
case LicenseFile(_, ext) if ext != ".class" => true // DISLIKE
case _ => false
}
}
```
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
Review Comment:
```bash
./build/make-distribution.sh --sbt-enabled
```
```
META-INF/LICENSE from Project dependency: Project name = celeborn-client-spark-3-shaded_2.12-0.4.0-SNAPSHOT, target = META-INF/LICENSE
[Discard] META-INF/NOTICE.txt Not from Project dependency: Jar name = commons-lang3-3.12.0.jar, jar org = org.apache.commons, entry target = META-INF/NOTICE.txt
[Discard] META-INF/LICENSE.txt Not from Project dependency: Jar name = commons-lang3-3.12.0.jar, jar org = org.apache.commons, entry target = META-INF/LICENSE.txt
META-INF/NOTICE from Project dependency: Project name = celeborn-client-spark-3-shaded_2.12-0.4.0-SNAPSHOT, target = META-INF/NOTICE
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1356825138
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
the ideal goal here is to drop all files `**/LICENSE.*` and `**/NOTICE.*` that comes from the third party and only keep the one provided by ourselves. I don't find a elegant approach in Maven, hope we can make it happen in SBT
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1371718119
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
How about using contains? I tested it locally and verified that it can be excluded.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1382743696
##########
project/CelebornBuild.scala:
##########
@@ -958,11 +962,10 @@ object MRClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ // For netty-3.x.y.Final.jar
case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
- case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
- case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
- case m if m == "LICENSE.txt" => MergeStrategy.discard
- case m if m == "NOTICE.txt" => MergeStrategy.discard
+ case PathList(ps@_*) if ps.last == "LICENSE" || ps.last == "NOTICE" => MergeStrategy.preferProject
Review Comment:
```suggestion
case "LICENSE" | "NOTICE" => MergeStrategy.discard
case PathList(ps@_*) if ps.last == "LICENSE" || ps.last == "NOTICE" => MergeStrategy.preferProject
```
we should discard the NOTICE/LICENSE files from the `scala-library` for mr-shade client.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#issuecomment-1759605195
cc @pan3793 @cfmcgrady
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#issuecomment-1763793493
I wrote a script and found that SBT's LICENSE and NOTICE are consistent with Maven's.
```shell
#!/bin/bash
# Specify the paths for two directories
dir1="sbt"
dir2="maven"
# Use find command to list all files in the first directory
find "$dir1" -type f | while read file1; do
# Get the relative path
rel_path="${file1#$dir1}"
# Construct the corresponding file path in the second directory
file2="$dir2$rel_path"
# Check if a file with the same path exists in the second directory
if [ -e "$file2" ]; then
# Use md5sum command to compare contents of the two files
hash1=$(md5sum "$file1" | awk '{print $1}')
hash2=$(md5sum "$file2" | awk '{print $1}')
# Compare MD5 hash values
if [[ "$file1" != *.class && "$hash1" != "$hash2" ]]; then
echo "Files differ: $file1 and $file2"
fi
else
# Print file path if it doesn't exist in second directory
echo "File does not exist in second directory: $file1"
fi
done
# Check if there are files in the second directory not in the first
find "$dir2" -type f | while read file2; do
# Get the relative path
rel_path="${file2#$dir2}"
# Construct the corresponding file path in the first directory
file1="$dir1$rel_path"
# Print file path if it doesn't exist in first directory
if [ ! -e "$file1" ]; then
echo "File does not exist in first directory: $file2"
fi
done
```
```
Files differ: sbt/celeborn-client-spark-3-shaded_2.12-0.4.0-SNAPSHOT.jar and maven/celeborn-client-spark-3-shaded_2.12-0.4.0-SNAPSHOT.jar
File does not exist in second directory: sbt/org/apache/celeborn/package$.class
File does not exist in second directory: sbt/org/apache/celeborn/package.class
Files differ: sbt/META-INF/services/reactor.blockhound.integration.BlockHoundIntegration and maven/META-INF/services/reactor.blockhound.integration.BlockHoundIntegration
Files differ: sbt/META-INF/MANIFEST.MF and maven/META-INF/MANIFEST.MF
File does not exist in second directory: sbt/META-INF/NOTICE.txt
File does not exist in second directory: sbt/META-INF/LICENSE.txt
File does not exist in second directory: sbt/META-INF/INDEX.LIST
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-common_2.12/pom.xml
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-common_2.12/pom.properties
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client_2.12/pom.xml
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client_2.12/pom.properties
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client-spark-3-shaded_2.12/pom.xml
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client-spark-3-shaded_2.12/pom.properties
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client-spark-common_2.12/pom.xml
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client-spark-common_2.12/pom.properties
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client-spark-3_2.12/pom.xml
File does not exist in first directory: maven/META-INF/maven/org.apache.celeborn/celeborn-client-spark-3_2.12/pom.properties
File does not exist in first directory: maven/META-INF/DEPENDENCIES
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1381121977
##########
project/CelebornBuild.scala:
##########
@@ -870,6 +885,8 @@ trait FlinkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
Review Comment:
The MR module depends on the netty 3x version, so there is a `META-INF/license/` directory
Check that Spark and Flink do not depend on netty 3x. This configuration is now removed.
https://repo1.maven.org/maven2/io/netty/netty/3.10.6.Final/netty-3.10.6.Final.jar
<img width="672" alt="image" src="https://github.com/apache/incubator-celeborn/assets/3898450/8be6e197-7343-416a-8cf9-a7dfc1f70b8b">
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1356825138
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
the ideal goal here is to drop all files `**/LICENSE.*` and `**/NOTICE.*` that come from the third-party jars and only keep the one provided by ourselves. I don't find an elegant approach in Maven, hope we can make it happen in SBT
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1360263362
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
@cxzl25 can we implement it in sbt? the previous discussion aims to remove maven and only keep sbt finally.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady closed pull request #1967: [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars
URL: https://github.com/apache/incubator-celeborn/pull/1967
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378447979
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
+ CustomMergeStrategy("LicenseFileOnlyProjectMergeStrategy") { conflicts =>
+ Right(conflicts.map {
+ case dep if dep.isProjectDependency =>
+ println(s"$path from Project dependency: $dep")
+ Some(JarEntry(dep.target, dep.stream))
+ case dep =>
+ println(s"[Discard] $path Not from Project dependency: $dep")
Review Comment:
```
assembly - including file: celeborn-client-spark-3-shaded_2.12:META-INF/LICENSE
assembly - excluding file: commons-lang3-3.12.0.jar:META-INF/LICENSE.txt
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1379994078
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
Maybe not? If our project does not have the same file
https://github.com/sbt/sbt-assembly/blob/e4d3b362997a32da21d97c051c9d92ecc5cdb9c2/src/main/scala/sbtassembly/MergeStrategy.scala#L82-L88
sbtassembly.MergeStrategy#preferProject
```scala
/**
* Picks the dependency that is the first project/internal dependency, otherwise picks the first library/external dependency
*/
val preferProject: MergeStrategy = MergeStrategy("PreferProject") { conflicts =>
val entry = conflicts.find(_.isProjectDependency).getOrElse(conflicts.head)
Right(Vector(JarEntry(entry.target, entry.stream)))
}
```
### use CustomMergeStrategy
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378422371
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
+ CustomMergeStrategy("LicenseFileOnlyProjectMergeStrategy") { conflicts =>
+ Right(conflicts.map {
+ case dep if dep.isProjectDependency =>
+ println(s"$path from Project dependency: $dep")
+ Some(JarEntry(dep.target, dep.stream))
+ case dep =>
+ println(s"[Discard] $path Not from Project dependency: $dep")
Review Comment:
https://github.com/apache/incubator-toree/blob/master/build.sbt#L33C1
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378447979
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
+ CustomMergeStrategy("LicenseFileOnlyProjectMergeStrategy") { conflicts =>
+ Right(conflicts.map {
+ case dep if dep.isProjectDependency =>
+ println(s"$path from Project dependency: $dep")
+ Some(JarEntry(dep.target, dep.stream))
+ case dep =>
+ println(s"[Discard] $path Not from Project dependency: $dep")
Review Comment:
how about
```
assembly - including file: celeborn-client-spark-3-shaded_2.12:META-INF/LICENSE
assembly - excluding file: commons-lang3-3.12.0.jar:META-INF/LICENSE.txt
```
?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378473760
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
```bash
[info] assembly - excluding file: Jar name = commons-lang3-3.12.0.jar, jar org = org.apache.commons, entry target = META-INF/LICENSE.txt
[info] assembly - excluding file: Jar name = commons-lang3-3.12.0.jar, jar org = org.apache.commons, entry target = META-INF/NOTICE.txt
[info] assembly - including file: META-INF/NOTICE
[info] assembly - including file: META-INF/LICENSE
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1380205140
##########
project/CelebornBuild.scala:
##########
@@ -870,6 +885,8 @@ trait FlinkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
Review Comment:
why do we need to discard the files in folder `META-INF/license/`?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378447979
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
+ CustomMergeStrategy("LicenseFileOnlyProjectMergeStrategy") { conflicts =>
+ Right(conflicts.map {
+ case dep if dep.isProjectDependency =>
+ println(s"$path from Project dependency: $dep")
+ Some(JarEntry(dep.target, dep.stream))
+ case dep =>
+ println(s"[Discard] $path Not from Project dependency: $dep")
Review Comment:
```
assembly - including file: ${relative_path}
assembly - excluding file: commons-lang3-3.12.0.jar:META-INF/LICENSE.txt
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1381686519
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
```scala
case PathList(ps@_*) if ps.last == "LICENSE" || ps.last == "NOTICE" => MergeStrategy.preferProject
case PathList(ps@_*) if Assembly.isLicenseFile(ps.last) => MergeStrategy.discard
```
<img width="783" alt="image" src="https://github.com/apache/incubator-celeborn/assets/3898450/01d6f68c-d9ad-44c2-a16f-47a0f756f267">
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1382810037
##########
project/CelebornBuild.scala:
##########
@@ -690,6 +690,8 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE" || m == "META-INF/NOTICE" => MergeStrategy.preferProject
Review Comment:
```suggestion
case "META-INF/LICENSE" | "META-INF/NOTICE" => MergeStrategy.preferProject
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378420537
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
+ CustomMergeStrategy("LicenseFileOnlyProjectMergeStrategy") { conflicts =>
+ Right(conflicts.map {
+ case dep if dep.isProjectDependency =>
+ println(s"$path from Project dependency: $dep")
+ Some(JarEntry(dep.target, dep.stream))
+ case dep =>
+ println(s"[Discard] $path Not from Project dependency: $dep")
Review Comment:
I roughly remember the sbt has a built-in slf4j logger
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1379994078
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
Maybe not? If our project does not have the same file
https://github.com/sbt/sbt-assembly/blob/e4d3b362997a32da21d97c051c9d92ecc5cdb9c2/src/main/scala/sbtassembly/MergeStrategy.scala#L82-L88
sbtassembly.MergeStrategy#preferProject
```scala
/**
* Picks the dependency that is the first project/internal dependency, otherwise picks the first library/external dependency
*/
val preferProject: MergeStrategy = MergeStrategy("PreferProject") { conflicts =>
val entry = conflicts.find(_.isProjectDependency).getOrElse(conflicts.head)
Right(Vector(JarEntry(entry.target, entry.stream)))
}
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#issuecomment-1754368787
## [Codecov](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1967?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1967](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1967?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (9159ba9) into [main](https://app.codecov.io/gh/apache/incubator-celeborn/commit/56276e910f3489bc070846a19813e322b590fd17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (56276e9) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #1967 +/- ##
==========================================
+ Coverage 46.96% 46.97% +0.01%
==========================================
Files 164 164
Lines 10362 10362
Branches 957 957
==========================================
+ Hits 4865 4866 +1
- Misses 5179 5180 +1
+ Partials 318 316 -2
```
[see 2 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-celeborn/pull/1967/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#issuecomment-1793653752
```shell
$ ./build/sbt -Pspark-3.3 celeborn-client-spark-3-shaded/assembly
$ jar tf client-spark/spark-3-shaded/target/scala-2.12/celeborn-client-spark-3-shaded_2.12-0.4.0-SNAPSHOT.jar | grep -i 'license\|notice'
META-INF/LICENSE
META-INF/NOTICE
META-INF/licenses/
META-INF/licenses/LICENSE-protobuf.txt
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1382744014
##########
project/CelebornBuild.scala:
##########
@@ -958,11 +962,10 @@ object MRClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ // For netty-3.x.y.Final.jar
case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
- case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
- case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
- case m if m == "LICENSE.txt" => MergeStrategy.discard
- case m if m == "NOTICE.txt" => MergeStrategy.discard
+ case PathList(ps@_*) if ps.last == "LICENSE" || ps.last == "NOTICE" => MergeStrategy.preferProject
Review Comment:
```
$ ./build/sbt -Pmr celeborn-client-mr-shaded/assembly
$ jar tf client-mr/mr-shaded/target/scala-2.12/celeborn-client-mr-shaded_2.12-0.4.0-SNAPSHOT.jar | grep -i 'license\|notice'
LICENSE
META-INF/LICENSE
META-INF/NOTICE
META-INF/licenses/
META-INF/licenses/LICENSE-protobuf.txt
NOTICE
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1382768925
##########
project/CelebornBuild.scala:
##########
@@ -958,11 +962,10 @@ object MRClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ // For netty-3.x.y.Final.jar
case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
- case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
- case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
- case m if m == "LICENSE.txt" => MergeStrategy.discard
- case m if m == "NOTICE.txt" => MergeStrategy.discard
+ case PathList(ps@_*) if ps.last == "LICENSE" || ps.last == "NOTICE" => MergeStrategy.preferProject
Review Comment:
Thanks for finding this problem.
I have now changed to use `preferProject` only for `META-INF/LICENSE` and `META-INF/NOTICE`.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1360325085
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
> the ideal goal here is to drop all files `**/LICENSE.*` and `**/NOTICE.*` that come from the third-party jars and only keep the one provided by ourselves. I don't find an elegant approach in Maven, hope we can make it happen in SBT
we can refer to [pekko](https://github.com/apache/incubator-pekko/blob/5314dcf7ccd980587fa03b850deb0f375932cb56/project/SbtMultiJvm.scala#L167-L173), and achieve this goal with `MergeStrategy.concat`
```scala
assembly / assemblyMergeStrategy := {
case n if n.endsWith(".class") => MergeStrategy.first
case n if n.endsWith(".txt") => MergeStrategy.concat
case n if n.endsWith("NOTICE") => MergeStrategy.concat
case n if n.endsWith("LICENSE") => MergeStrategy.concat
case n => (assembly / assemblyMergeStrategy).value.apply(n)
},
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1356904187
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
Implemented a simple `ResourceTransformer` that can output `LICENSE.txt` and `NOTICE.txt` source jar.
```java
import org.apache.maven.plugins.shade.resource.ResourceTransformer;
public class PrintResourceTransformer implements ResourceTransformer {
@Override
public boolean canTransformResource(String s) {
boolean hasFlag = false;
if (s.contains("LICENSE.txt")) {
hasFlag = true;
} else if (s.contains("NOTICE.txt")) {
hasFlag = true;
}
if (hasFlag) {
URL resource = this.getClass().getResource("/" + s);
System.err.printf("PrintResourceTransformer %s,%s,%s%n", s, hasFlag, resource);
}
return hasFlag;
}
```
```java
PrintResourceTransformer META-INF/LICENSE.txt,true,jar:file:/Users/csy/.m2/repository/org/jdom/jdom2/2.0.6.1/jdom2-2.0.6.1.jar!/META-INF/LICENSE.txt
PrintResourceTransformer META-INF/NOTICE.txt,true,jar:file:/Users/csy/.m2/repository/commons-io/commons-io/2.6/commons-io-2.6.jar!/META-INF/NOTICE.txt
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1356980761
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
unfortunately, the built-in apache license/notice transformer does not work well during my previous test
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#issuecomment-1763906103
> I wrote a script and found that SBT's LICENSE and NOTICE are consistent with Maven's.
Thanks for the effort, actually, as mentioned above, I don't think the current implementation in Maven is perfect.
The goals of this part are:
1. filter out all LICENSE.* and NOTICE.* from the third-party jars, no matter from which folders.
2. only include the LICENSE and NOTICE we provided.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1382810718
##########
project/CelebornBuild.scala:
##########
@@ -881,6 +883,8 @@ trait FlinkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE" || m == "META-INF/NOTICE" => MergeStrategy.preferProject
Review Comment:
```suggestion
case "META-INF/LICENSE" | "META-INF/NOTICE" => MergeStrategy.preferProject
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1381171787
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
is it feasible to combine using `MergeStrategy.preferProject` and `MergeStrategy.discard`?
```
case "LICENSE" | "NOTICE" => MergeStrategy.preferProject
case m if Assembly.isLicenseFile(m) => MergeStrategy.discard
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1379994078
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
Maybe not? If our project does not have the same file
### MergeStrategy.preferProject
https://github.com/sbt/sbt-assembly/blob/e4d3b362997a32da21d97c051c9d92ecc5cdb9c2/src/main/scala/sbtassembly/MergeStrategy.scala#L82-L88
sbtassembly.MergeStrategy#preferProject
```scala
/**
* Picks the dependency that is the first project/internal dependency, otherwise picks the first library/external dependency
*/
val preferProject: MergeStrategy = MergeStrategy("PreferProject") { conflicts =>
val entry = conflicts.find(_.isProjectDependency).getOrElse(conflicts.head)
Right(Vector(JarEntry(entry.target, entry.stream)))
}
```
### use CustomMergeStrategy
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1379994078
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
Maybe not? If our project does not have the same file
https://github.com/sbt/sbt-assembly/blob/e4d3b362997a32da21d97c051c9d92ecc5cdb9c2/src/main/scala/sbtassembly/MergeStrategy.scala#L82-L88
sbtassembly.MergeStrategy#preferProject
```scala
/**
* Picks the dependency that is the first project/internal dependency, otherwise picks the first library/external dependency
*/
val preferProject: MergeStrategy = MergeStrategy("PreferProject") { conflicts =>
val entry = conflicts.find(_.isProjectDependency).getOrElse(conflicts.head)
Right(Vector(JarEntry(entry.target, entry.stream)))
}
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1378447979
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(path: String) =
+ CustomMergeStrategy("LicenseFileOnlyProjectMergeStrategy") { conflicts =>
+ Right(conflicts.map {
+ case dep if dep.isProjectDependency =>
+ println(s"$path from Project dependency: $dep")
+ Some(JarEntry(dep.target, dep.stream))
+ case dep =>
+ println(s"[Discard] $path Not from Project dependency: $dep")
Review Comment:
```
assembly - including project file: $path
assembly - excluding file: commons-lang3-3.12.0.jar:META-INF/LICENSE.txt
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1379991726
##########
project/CelebornBuild.scala:
##########
@@ -205,6 +205,18 @@ object CelebornCommonSettings {
Dependencies.junit % "test",
// https://www.scala-sbt.org/1.x/docs/Testing.html
Dependencies.junitInterface % "test")
+
+ def licenseFileOnlyProjectMergeStrategy(log: Logger) =
Review Comment:
I think the built-in strategy `MergeStrategy.preferProject` can achieve this goal
[merge strategy](https://github.com/sbt/sbt-assembly#merge-strategy)
> `MergeStrategy.preferProject` will choose the first project file over library files if present. Otherwise, it works like `MergeStrategy.first`
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1360263362
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
@cxzl25 can we implement it in sbt? the previous discussion aims to remove maven and only keep sbt eventually.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1356819310
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
is it possible to know which jar the file comes from?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1356897289
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
`maven-shade-plugin` provides some license transformers.
https://maven.apache.org/plugins/maven-shade-plugin/examples/resource-transformers.html
```xml
<transformer implementation="org.apache.maven.plugins.shade.resource.ApacheLicenseResourceTransformer"/>
<transformer implementation="org.apache.maven.plugins.shade.resource.ApacheNoticeResourceTransformer"/>
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1360100091
##########
project/CelebornBuild.scala:
##########
@@ -679,6 +679,11 @@ trait SparkClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
+ case m if m == "LICENSE.txt" => MergeStrategy.discard
+ case m if m == "NOTICE.txt" => MergeStrategy.discard
Review Comment:
> unfortunately, the built-in apache license/notice transformer does not work well during my previous test
`ApacheLicenseResourceTransformer` just ignores `META-INF/LICENSE`, `META-INF/LICENSE.txt`, `META-INF/LICENSE.md`.
https://github.com/apache/maven-shade-plugin/blob/master/src/main/java/org/apache/maven/plugins/shade/resource/ApacheLicenseResourceTransformer.java#L31
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#issuecomment-1794153510
thanks, merging to main(v0.4.0).
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1031] SBT correct the LICENSE and NOTICE for shaded client jars [incubator-celeborn]
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on code in PR #1967:
URL: https://github.com/apache/incubator-celeborn/pull/1967#discussion_r1382810973
##########
project/CelebornBuild.scala:
##########
@@ -958,11 +962,10 @@ object MRClientProjects {
(assembly / assemblyMergeStrategy) := {
case m if m.toLowerCase(Locale.ROOT).endsWith("manifest.mf") => MergeStrategy.discard
+ // For netty-3.x.y.Final.jar
case m if m.startsWith("META-INF/license/") => MergeStrategy.discard
- case m if m == "META-INF/LICENSE.txt" => MergeStrategy.discard
- case m if m == "META-INF/NOTICE.txt" => MergeStrategy.discard
- case m if m == "LICENSE.txt" => MergeStrategy.discard
- case m if m == "NOTICE.txt" => MergeStrategy.discard
+ case m if m == "META-INF/LICENSE" || m == "META-INF/NOTICE" => MergeStrategy.preferProject
Review Comment:
```suggestion
case "META-INF/LICENSE" | "META-INF/NOTICE" => MergeStrategy.preferProject
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org