-
Notifications
You must be signed in to change notification settings - Fork 537
HDDS-12462. Use exclude rules for defining shaded filesystem jar contents #8008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
0fde989
1eadebf
696ecb3
520031e
1e45f38
efe5471
892df45
90361b0
7111c6e
b5d92f9
5c46400
dc217c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,6 +102,15 @@ | |
<phase>package</phase> | ||
<configuration> | ||
<skip>${maven.shade.skip}</skip> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a user directly depend on Ozone and use Ozone's Filesystem without introducing Hadoop's package, can the current Ozone fat-jar provide all necessary dependencies? |
||
<filters> | ||
<filter> | ||
<artifact>*:*</artifact> | ||
<excludes> | ||
<!-- Exclude android (dependency from io.grpc:grpc-core) to reduce unnecessary dependencies and avoid potential conflicts --> | ||
<exclude>android/**</exclude> | ||
</excludes> | ||
</filter> | ||
</filters> | ||
<transformers> | ||
<transformer implementation="org.apache.maven.plugins.shade.resource.DontIncludeResourceTransformer"> | ||
<resources> | ||
|
@@ -120,38 +129,47 @@ | |
<relocation> | ||
<pattern>org</pattern> | ||
<shadedPattern>${shaded.prefix}.org</shadedPattern> | ||
<includes> | ||
<include>org.yaml.**.*</include> | ||
<include>org.sqlite.**.*</include> | ||
<include>org.tukaani.**.*</include> | ||
<include>org.bouncycastle.**.*</include> | ||
<include>org.rocksdb.**.*</include> | ||
<include>org.apache.commons.cli.**.*</include> | ||
<include>org.apache.commons.compress.**.*</include> | ||
<include>org.apache.commons.codec.**.*</include> | ||
<include>org.apache.commons.beanutils.**.*</include> | ||
<include>org.apache.commons.collections.**.*</include> | ||
<include>org.apache.commons.digester.**.*</include> | ||
<include>org.apache.commons.io.**.*</include> | ||
<include>org.apache.commons.logging.**.*</include> | ||
<include>org.apache.commons.validator.**.*</include> | ||
<include>org.apache.commons.lang3.**.*</include> | ||
<include>org.sqlite.**.*</include> | ||
<include>org.apache.thrift.**.*</include> | ||
</includes> | ||
<excludes> | ||
<exclude>org.apache.commons.logging.**</exclude> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These two are included to shade currently, but excluded in the new change. Is there any impact if not shaded? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
<exclude>org.apache.hadoop.**</exclude> | ||
<exclude>org.apache.log4j.**</exclude> | ||
<exclude>org.apache.ozone.**</exclude> | ||
<exclude>org.apache.ratis.**</exclude> | ||
<exclude>org.bouncycastle.**</exclude> | ||
<exclude>org.ietf.jgss.*</exclude> | ||
<exclude>org.junit.**</exclude> | ||
<exclude>org.omg.**</exclude> | ||
<exclude>org.slf4j.**</exclude> | ||
<exclude>org.w3c.dom.**</exclude> | ||
<exclude>org.xerial.snappy.**</exclude> | ||
<exclude>org.xml.sax.**</exclude> | ||
<exclude>**.pom.xml</exclude> | ||
</excludes> | ||
</relocation> | ||
<relocation> | ||
<pattern>com</pattern> | ||
<shadedPattern>${shaded.prefix}.com</shadedPattern> | ||
<includes> | ||
<include>com.google.common.**.*</include> | ||
<include>com.google.gson.**.*</include> | ||
<include>com.codahale.**.*</include> | ||
<include>com.fasterxml.**.*</include> | ||
<include>com.lmax.**.*</include> | ||
<include>com.github.joshelser.**.*</include> | ||
<include>com.twitter.**.*</include> | ||
</includes> | ||
<excludes> | ||
<exclude>com.google.protobuf.**</exclude> | ||
<exclude>com.sun.javadoc.**</exclude> | ||
<exclude>com.sun.jndi.**</exclude> | ||
<exclude>com.sun.management.**</exclude> | ||
<exclude>com.sun.security.**</exclude> | ||
<exclude>com.sun.tools.**</exclude> | ||
<exclude>**.pom.xml</exclude> | ||
</excludes> | ||
</relocation> | ||
<relocation> | ||
<pattern>google</pattern> | ||
<shadedPattern>${shaded.prefix}.google</shadedPattern> | ||
</relocation> | ||
<relocation> | ||
<pattern>net.jcip</pattern> | ||
<shadedPattern>${shaded.prefix}.net.jcip</shadedPattern> | ||
</relocation> | ||
<relocation> | ||
<pattern>javassist</pattern> | ||
<shadedPattern>${shaded.prefix}.javassist</shadedPattern> | ||
</relocation> | ||
<relocation> | ||
<pattern>kotlin</pattern> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adoroszlai @ChenSammi
I have an idea
https://github.com/apache/hadoop/blob/d552bb056c3daec8d540f166c41b463e8ef18645/hadoop-client-modules/hadoop-client-api/pom.xml#L131
For these classes that are not shaded in hadoop, should we choose to have Ozone not include these "hadoop client unshaded classes" because this may cause class conflicts? (exclude them in
artifactSet
andrelocation
)We should have two options for Ozone dependencies
In this way, the ozone release package will only contain shaded dependencies, and for these unshaded dependencies, Ozone will assume that Hadoop env provides these dependencies.
How do you think ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Hadoop is also not included in the fat jar, so it cannot work without Hadoop anyway.