Skip to content
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

Use shading to build the packages #587

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 15 additions & 1 deletion benchmark/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>2.2</version>
<version>3.1.0</version>
<executions>
<execution>
<phase>package</phase>
Expand All @@ -109,6 +109,20 @@
</goals>
<configuration>
<finalName>${uberjar.name}</finalName>
<relocations>
<relocation>
<pattern>io.netty</pattern>
<shadedPattern>com.turo.pushy.external.io.netty</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.code.gson</pattern>
<shadedPattern>com.turo.pushy.external.com.google.code.gson</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons</pattern>
<shadedPattern>com.turo.pushy.external.org.apache.commons</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
<mainClass>org.openjdk.jmh.Main</mainClass>
Expand Down
1 change: 1 addition & 0 deletions dropwizard-metrics-listener/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
<groupId>${project.groupId}</groupId>
<artifactId>pushy</artifactId>
<version>${project.version}</version>
<classifier>shaded</classifier>
Copy link
Owner

Choose a reason for hiding this comment

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

This feels a little odd to me; I don't think we want to make the Dropwizard metrics listener explicitly dependent upon the shaded version of Pushy. The Maven docs aren't quite clear on this point; if we leave out a classifier here, will any classification of com.turo.pushy at the same version fulfill the requirement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. You're right, that's a bit strange.

What could be done, for example, is also providing a shaded dropwizard-metrics-listener that had the shaded pushy dependency. However I don't even know if that's possible with maven.

What also could be done, which I think would be easier, is marking the pushy dependency in dropwizard-metrics-listener as optional, which would force users of the artifact, to also add pushy as a dependency explicitly (and then they'd be able to use either the shaded or the non-shaded one).

Copy link

@justinpermar justinpermar Mar 19, 2018

Choose a reason for hiding this comment

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

@jchambers From https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html: NOTE: In two of these dependency references, we had to specify the <type/> element. This is because the minimal set of information for matching a dependency reference against a dependencyManagement section is actually {groupId, artifactId, type, classifier}. In many cases, these dependencies will refer to jar artifacts with no classifier. This allows us to shorthand the identity set to {groupId, artifactId}, since the default for the type field is jar, and the default classifier is null.

</dependency>
<dependency>
<groupId>io.dropwizard.metrics</groupId>
Expand Down
31 changes: 31 additions & 0 deletions pushy/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,37 @@
<build>
<plugins>
<!-- Download the alpn-boot.jar in advance to add it to the boot classpath. -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>3.1.0</version>
<executions>
<execution>
<phase>package</phase>
<goals>
<goal>shade</goal>
</goals>
<configuration>
<relocations>
<relocation>
<pattern>io.netty</pattern>
<shadedPattern>com.turo.pushy.external.io.netty</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.code.gson</pattern>
<shadedPattern>com.turo.pushy.external.com.google.code.gson</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons</pattern>
<shadedPattern>com.turo.pushy.external.org.apache.commons</shadedPattern>
</relocation>
</relocations>
<shadedArtifactAttached>true</shadedArtifactAttached>
<shadedClassifierName>shaded</shadedClassifierName>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
Expand Down