Skip to content

[SPARK-41450][BUILD] Fix shading in core module #38999

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.

Sign up for GitHub

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

Closed
pan3793 wants to merge 3 commits into from
Closed

[SPARK-41450][BUILD] Fix shading in core module #38999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32366e8
[SPARK-41244] Fix shading in core module
pan3793 Dec 9, 2022
661d269
nit
pan3793 Dec 9, 2022
07bee24
nit
pan3793 Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,10 +629,34 @@
<shadeTestJar>true</shadeTestJar>
<artifactSet>
<includes>
<include>org.spark-project.spark:unused</include>
Copy link
Contributor

@LuciferYang LuciferYang Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you compared the compiled logs of Maven shade? If it is not use append mode, we need make sure there is no omission.

Copy link
Member Author

@pan3793 pan3793 Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the result is expected.

This PR

[INFO] --- maven-shade-plugin:3.2.4:shade (default) @ spark-core_2.12 ---
[INFO] Including org.eclipse.jetty:jetty-plus:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-security:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-util:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-server:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-io:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-http:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-continuation:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlet:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-proxy:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-client:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlets:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including com.google.protobuf:protobuf-java:jar:3.21.9 in the shaded jar.
[INFO] Including org.spark-project.spark:unused:jar:1.0.0 in the shaded jar.

Before #38779

[INFO] --- maven-shade-plugin:3.2.4:shade (default) @ spark-core_2.12 ---
[INFO] Including org.eclipse.jetty:jetty-plus:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-security:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-util:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-server:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-io:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-http:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-continuation:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlet:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-proxy:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-client:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlets:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.spark-project.spark:unused:jar:1.0.0 in the shaded jar.

<include>org.eclipse.jetty:jetty-io</include>
<include>org.eclipse.jetty:jetty-http</include>
<include>org.eclipse.jetty:jetty-proxy</include>
<include>org.eclipse.jetty:jetty-client</include>
<include>org.eclipse.jetty:jetty-continuation</include>
<include>org.eclipse.jetty:jetty-servlet</include>
<include>org.eclipse.jetty:jetty-servlets</include>
<include>org.eclipse.jetty:jetty-plus</include>
<include>org.eclipse.jetty:jetty-security</include>
<include>org.eclipse.jetty:jetty-util</include>
<include>org.eclipse.jetty:jetty-server</include>
<include>com.google.guava:guava</include>
<include>com.google.protobuf:*</include>
</includes>
</artifactSet>
<relocations>
<relocation>
<pattern>org.eclipse.jetty</pattern>
<shadedPattern>${spark.shade.packageName}.jetty</shadedPattern>
<includes>
<include>org.eclipse.jetty.**</include>
</includes>
</relocation>
<relocation>
<pattern>com.google.common</pattern>
<shadedPattern>${spark.shade.packageName}.guava</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.protobuf</pattern>
<shadedPattern>${spark.shade.packageName}.spark-core.protobuf</shadedPattern>
Expand Down
11 changes: 0 additions & 11 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3229,17 +3229,6 @@
<artifactSet>
<includes>
<include>org.spark-project.spark:unused</include>
<include>org.eclipse.jetty:jetty-io</include>
Copy link
Contributor

@LuciferYang LuciferYang Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we move this part to core/pom.xml, also need remove the relocation of jetty from parent pom.xml?

Copy link
Member Author

@pan3793 pan3793 Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly, jetty classes should be included into core only, but relocation is required for all modules, otherwise, other modules like sql reference the vanilla jetty classes but core ships relocated jetty classes.

Copy link
Contributor

@LuciferYang LuciferYang Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it

<include>org.eclipse.jetty:jetty-http</include>
<include>org.eclipse.jetty:jetty-proxy</include>
<include>org.eclipse.jetty:jetty-client</include>
<include>org.eclipse.jetty:jetty-continuation</include>
<include>org.eclipse.jetty:jetty-servlet</include>
<include>org.eclipse.jetty:jetty-servlets</include>
<include>org.eclipse.jetty:jetty-plus</include>
<include>org.eclipse.jetty:jetty-security</include>
<include>org.eclipse.jetty:jetty-util</include>
<include>org.eclipse.jetty:jetty-server</include>
<include>com.google.guava:guava</include>
<include>org.jpmml:*</include>
</includes>
Expand Down