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

Change names of files with the same path #86

Closed
wants to merge 4 commits into from
Closed

Change names of files with the same path #86

wants to merge 4 commits into from

Conversation

amorfis
Copy link

@amorfis amorfis commented Aug 20, 2014

If there are two files with the same path in two merged .jar files, e.g.

junit-3.8.1.jar: /LICENSE.TXT
hamcrest-1.3.jar: /LICENSE.TXT

don't overwrite one file with another, but include both in shadow jar with changed names:

/LICENSE-junit-3.8.1.TXT
/LICENSE-hamcrest-1.3.TXT

I have some problems with tests, but... when I rewinded back to commit 30a696d (onto which I rebased my changes) or even 43fe990 (the one that was HEAD when I forked this repository) and issued "./gradlew clean test" I still have the same problems.

It is quite random, and must be connected with classloaders:

Caused by: org.codehaus.groovy.runtime.typehandling.GroovyCastException: Cannot cast object 'org.apache.tools.zip.ZipOutputStream@583d38a9' with class 'org.apache.tools.zip.ZipOutputStream' to class 'org.apache.tools.zip.ZipOutputStream'
    at com.github.jengelman.gradle.plugins.shadow.tasks.ShadowCopyAction.execute(ShadowCopyAction.groovy:72)

@johnrengelman
Copy link
Collaborator

try killing your Gradle daemon: gradle --stop

@johnrengelman johnrengelman added this to the 2.0.0 milestone Aug 26, 2014
@amorfis
Copy link
Author

amorfis commented Sep 6, 2014

Tests fixed. Could you check this now, and maybe merge?

@johnrengelman
Copy link
Collaborator

Two things right away,

  1. The feature needs to be configurable. Some people might not want this behavior but as of now you're forcing it on them. Perhaps you could use the duplicateStrategy (https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/groovy/org/gradle/api/file/DuplicatesStrategy.java) property to drive this.
  2. I'd much prefer that this was implemented using the Transformer interface. That would take care of item 1. You'd probably need to modify the API a bit to to include the source archive information in the call to the transformer though.

@amorfis
Copy link
Author

amorfis commented Sep 8, 2014

Hmm... I thought transformers need explicit list of files to transform, but as far as I can see now, it's possible for transformer to handle every file it wants (i.e. each file is actually going to transformer). I'll change it to transformer then.

Should I create new fork? This one is already quite long.

@amorfis
Copy link
Author

amorfis commented Sep 28, 2014

Can you debug it somehow? I can't connect with remote degugging to running gradle test, nor can debug it in IDEA (ShadowCopyAction.groovy:70: Cannot cast object 'org.apache.tools.zip.ZipOutputStream@2f62c922' with class 'org.apache.tools.zip.ZipOutputStream' to class 'org.apache.tools.zip.ZipOutputStream' - must be from different classloaders.)

@amorfis amorfis closed this Nov 4, 2014
codefromthecrypt pushed a commit to openzipkin/zipkin that referenced this pull request Jan 4, 2016
The shadowJar plugin makes a single file out of all the dependencies of
our services. Occasionally, this leads to file conflicts.

Jackson's META-INF/LICENSE conflicts with the directory META-INF/license

While we can run the shadowJars, you cannot extract them with `jar -x`,
as it ends up with an IOException extracting META-INF/license.

This excludes Jackson's META-INF/LICENSE, until we have a better
solution, ideally with one of the proposals in the shadow jar project.

See GradleUp/shadow#86
See GradleUp/shadow#102
@johnrengelman johnrengelman removed this from the 5.0.0 milestone Jan 20, 2019
Fiouz added a commit to Fiouz/shadow that referenced this pull request Aug 30, 2021
Omitting the visibility modifier in Groovy [1] automatically creates a
private backing field and a public getter (+ setter if the field is not
final).
It is unlikely to be an intended design as it makes access to those
fields awkward from statically typed languages
`ShadowBasePlugin.getCONFIGURATION_NAME()`.

This is a backward incompatible change as this removes public getters.

Here is what the generated class looked liked before this
commit (7.0.0):

```
$ javap -p -c -constants ShadowBasePlugin.class
Compiled from "ShadowBasePlugin.groovy"
public class com.github.jengelman.gradle.plugins.shadow.ShadowBasePlugin implements org.gradle.api.Plugin<org.gradle.api.Project>, groovy.lang.GroovyObject {
  private static final java.lang.String EXTENSION_NAME = "shadow";

  private static final java.lang.String CONFIGURATION_NAME = "shadow";
[...]
  public static java.lang.String getEXTENSION_NAME();
    Code:
       0: getstatic     GradleUp#86                 // Field EXTENSION_NAME:Ljava/lang/String;
       3: areturn

  public static java.lang.String getCONFIGURATION_NAME();
    Code:
       0: getstatic     GradleUp#111                // Field CONFIGURATION_NAME:Ljava/lang/String;
       3: areturn
```

[1] https://groovy-lang.org/objectorientation.html#fields
Fiouz added a commit to Fiouz/shadow that referenced this pull request Aug 30, 2021
Omitting the visibility modifier creates a Groovy _property_ [1], which
is composed by:
* a _private_ backing field
* a public getter
* (+ setter if the property is not `final`)

It is unlikely to be an intended design as it makes access to those
fields awkward from statically typed languages
`ShadowBasePlugin.getCONFIGURATION_NAME()`.

This commit replaces such _properties_ with _public fields_ only.

This is a backward incompatible change as this removes public getters.

Here is what the generated class looked liked before this
commit (7.0.0):

```
$ javap -p -c -constants ShadowBasePlugin.class
Compiled from "ShadowBasePlugin.groovy"
public class com.github.jengelman.gradle.plugins.shadow.ShadowBasePlugin implements org.gradle.api.Plugin<org.gradle.api.Project>, groovy.lang.GroovyObject {
  private static final java.lang.String EXTENSION_NAME = "shadow";

  private static final java.lang.String CONFIGURATION_NAME = "shadow";
[...]
  public static java.lang.String getEXTENSION_NAME();
    Code:
       0: getstatic     GradleUp#86                 // Field EXTENSION_NAME:Ljava/lang/String;
       3: areturn

  public static java.lang.String getCONFIGURATION_NAME();
    Code:
       0: getstatic     GradleUp#111                // Field CONFIGURATION_NAME:Ljava/lang/String;
       3: areturn
```

[1] https://groovy-lang.org/objectorientation.html#properties
Fiouz added a commit to Fiouz/shadow that referenced this pull request Aug 30, 2021
Omitting the visibility modifier creates a Groovy _property_ [1], which
is composed by:
* a _private_ backing field
* a public getter
* (+ setter if the property is not `final`)

It is unlikely to be an intended design as it makes access to those
fields awkward from statically typed languages
`ShadowBasePlugin.getCONFIGURATION_NAME()`.

This commit replaces such _properties_ with _public fields_ only.

This is a backward incompatible change as this removes public getters.

Here is what the generated class looked liked before this
commit (7.0.0):

```
$ javap -p -c -constants ShadowBasePlugin.class
Compiled from "ShadowBasePlugin.groovy"
public class com.github.jengelman.gradle.plugins.shadow.ShadowBasePlugin implements org.gradle.api.Plugin<org.gradle.api.Project>, groovy.lang.GroovyObject {
  private static final java.lang.String EXTENSION_NAME = "shadow";

  private static final java.lang.String CONFIGURATION_NAME = "shadow";
[...]
  public static java.lang.String getEXTENSION_NAME();
    Code:
       0: getstatic     GradleUp#86                 // Field EXTENSION_NAME:Ljava/lang/String;
       3: areturn

  public static java.lang.String getCONFIGURATION_NAME();
    Code:
       0: getstatic     GradleUp#111                // Field CONFIGURATION_NAME:Ljava/lang/String;
       3: areturn
```

[1] https://groovy-lang.org/objectorientation.html#properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants