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.

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

Jump to bottom

Shade the output jar, and relocate artifacts likely to cause collisions (notably auto-common) #118

Merged
merged 4 commits into from Apr 7, 2015
Merged

Shade the output jar, and relocate artifacts likely to cause collisions (notably auto-common) #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5462408
Ignore maven shade plugin's dependency reduced pom.xml
cgruber Feb 5, 2015
d4b429c
Shade the processor plugin, excluding from relocation any public APIs.
cgruber Feb 5, 2015
74d6b3d
relocate common.
cgruber Apr 1, 2015
3425e85
Fix xml style.
cgruber Apr 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Expand Up @@ -25,3 +25,5 @@ classes
obj

.DS_Store

dependency-reduced-pom.xml
52 changes: 35 additions & 17 deletions compiler/pom.xml
View file
Open in desktop
Expand Up @@ -129,23 +129,6 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
<configuration>
<descriptorRefs>
<descriptorRef>jar-with-dependencies</descriptorRef>
</descriptorRefs>
</configuration>
<executions>
<execution>
<phase>package</phase>
<goals>
<goal>single</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-invoker-plugin</artifactId>
Expand Down Expand Up @@ -173,6 +156,41 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>2.3</version>
<executions>
<execution>
<phase>package</phase>
<goals>
<goal>shade</goal>
</goals>
<configuration>
<minimizeJar>true</minimizeJar>
<artifactSet>
<excludes>
<!-- guava which has a consistent API and whose public types we vend in producers -->
<exclude>com.google.guava</exclude>
Copy link

Choose a reason for hiding this comment

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

Actually this might cause problems. For example, Android gradle plugin has a transient dependency on guava 15. And when some plugin has a dependency on guava 18, it apparently sparks problems.
I do not think such a problem will arise in processors classpath immediately, but such a situation might happen in future.

Copy link

Choose a reason for hiding this comment

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

Yep. We've had this happen multiple times when used on the Gradle
buildscript classpath.

On Thu, Apr 2, 2015, 7:22 AM Roman Mazur notifications@github.com wrote:

In compiler/pom.xml
#118 (comment):

  •  <plugin>

  •    <groupId>org.apache.maven.plugins</groupId>

  •    <artifactId>maven-shade-

plugin

  •    <version>2.3</version>

  •    <executions>

  •      <execution>

  •        <phase>package</phase>

  •        <goals>

  •          <goal>shade</goal>

  •        </goals>

  •        <configuration>

  •          <minimizeJar>true</minimizeJar>

  •          <artifactSet>

  •            <excludes>

  •              <!-- guava which has a consistent API and whose public types we vend in producers -->

  •              <exclude>com.google.guava</exclude>

Actually this might cause problems. For example, Android gradle plugin has
a transient dependency on guava 15. And when some plugin has a dependency
on guava 18, it apparently sparks problems.
I do not think such a problem will arise in processors classpath
immediately, but such a situation might happen in future.

—
Reply to this email directly or view it on GitHub
https://github.com/google/dagger/pull/118/files#r27648909.

Copy link

Choose a reason for hiding this comment

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

FYI, Immutables.org shades Guava but uses a trick (example use) to avoid the maven-shade-plugin from rewriting Guava class name literals used in generated code.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah - @tbroyer - we probably need to move away from literals anyway, and I have an internal commit I was toying with to do just that.

Given that this pull improves the situation of google processors working together, and improves upon no shading, I'm inclined to move forward with this and refine.

I don't think this pull with "cause" problems that aren't already present. Am I missing something? Right now there's no shading.

<!-- annotation processors dagger uses to be built, not to operate -->
<exclude>com.google.auto.service</exclude>
<exclude>com.google.auto.value</exclude>
<!-- projects should depend on api projects directly -->
<exclude>com.google.dagger:dagger</exclude>
<exclude>com.google.dagger:dagger-producers</exclude>
<exclude>javax.inject</exclude>
</excludes>
</artifactSet>
<relocations>
<relocation>
<pattern>com.google.auto.common</pattern>
<shadedPattern>dagger.shaded.auto.common</shadedPattern>
</relocation>
</relocations>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>