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

Shadowed JARs should not be added to JAR manifest Class-Path header #324

Open
sbrannen opened this issue Sep 6, 2017 · 14 comments
Open

Comments

@sbrannen
Copy link

sbrannen commented Sep 6, 2017

Original JUnit Issue

junit-team/junit5#1044

Code in Question

https://github.com/johnrengelman/shadow/blob/5e89b3d650f964e62a1b617223ee25d9001fa3d6/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowJavaPlugin.groovy#L52-L54

Expected Behavior

Shadowed JARs should not be added to manifest Class-Path header.

Actual Behavior

Shadowed JARs are added to manifest Class-Path header.

Gradle Build Script(s)

https://github.com/junit-team/junit5/blob/master/junit-jupiter-params/junit-jupiter-params.gradle

Content of MANIFEST.MF

Manifest-Version: 1.0
Implementation-Title: junit-jupiter-params
Automatic-Module-Name: org.junit.jupiter.params
Build-Date: 2017-08-23
Implementation-Version: 5.0.0-RC3
Built-By: JUnit Team
Specification-Vendor: junit.org
Specification-Title: junit-jupiter-params
Class-Path: univocity-parsers-2.5.1.jar
Implementation-Vendor: junit.org
Build-Revision: e0af182b1440da44af16a7181cd3b03d1a3fe82d
Build-Time: 20:40:28.393+0200
Created-By: 1.8.0_131 (Oracle Corporation 25.131-b11)
Specification-Version: 5.0.0
@tandr
Copy link

tandr commented Sep 14, 2017

We are hitting the same issue. We relocate protobuf and grpc under subfolder (sub-namespace), but they are still appearing in manifest, causing all sorts of problems or with Maven build, or tests, or runtime. Short of repackaging the resulting jar file I run out of ideas...

@johnrengelman johnrengelman added this to the 3.0.0 milestone Nov 4, 2017
@mernst
Copy link

mernst commented Feb 18, 2018

This bug in the shadowJar task (probably introduced while fixing #65) was blocking me. Here is a workaround that others may find useful.

// Work around https://github.com/johnrengelman/shadow/issues/324
shadowJar.doLast {
  exec {
    workingDir 'build/libs'
    commandLine 'jar', 'xf', shadowJar.archiveName, 'META-INF/MANIFEST.MF'
  }
  exec {
    workingDir 'build/libs'
    commandLine 'sed', '-i', '/^Class-Path:.*/d', 'META-INF/MANIFEST.MF'
  }
  exec {
    workingDir 'build/libs'
    commandLine 'zip', '-q', '-u', shadowJar.archiveName, 'META-INF/MANIFEST.MF'
    ignoreExitValue true  // Exit value 12 means "nothing to do"
  }
  delete {
    delete 'build/libs/META-INF'
  }
}

mernst added a commit to plume-lib/options that referenced this issue Feb 18, 2018
@johnrengelman
Copy link
Collaborator

@sbrannen I don't understand where the issue here is. In the linked gradle file, you explicitly declare the univocity-parsers-2.5.1 dependency to be in configurations.shadow. This configuration is reserved for dependencies that are not merged into the JAR, so they are included in the manifest classpath as they are required for runtime.
Additionally, by declaring it on that configuration, it should not be merged, so if you see teh resulting files in the merged output, then something in your compile or runtime configurations is transitively pulling that depedendency in.

@johnrengelman
Copy link
Collaborator

johnrengelman commented Feb 18, 2018

So far for all the examples people have posted, this is behaving as intended. It seems like there is just a misunderstanding of when to use configurations.shadow

Specifically, that configuration is not intended to be a catch-all. It is very specifically for usage when you have a dependency that you don't know want merged into the shadow JAR, but is a required dependency to be shipped with the application.

@mernst
Copy link

mernst commented Feb 18, 2018

Thanks for following up.

The documentation says, "Think of configurations.shadow as unmerged, runtime dependencies." In my case, one of the dependencies is the JDK's tools.jar file.

I don't want the huge tools.jar file to appear in my options-all.jar file. (Any Java developer has a copy, but at an unpredictable location.) Therefore, I declared it as a shadow dependency:

  shadow files(org.gradle.internal.jvm.Jvm.current().toolsJar)

Now, if I run gradle shadowJar, the resulting options-all.jar file's META-INF/MANIFEST.MF contains a line

Class-Path: lib/tools.jar

Clients using the options-all.jar file suffer a warning

warning: [path] bad path element "lib/tools.jar": no such file or directory

If the client has set -Werror, then this warning halts the build.
In other words, the options-all.jar file is unusable because it breaks the build of its clients.

If I apply the workaround in my comment above, then clients are not broken.

If the current behavior is intended, then how can I create a shadow jar file that is usable by clients but does not include the whole tools.jar file?

Thanks!

@johnrengelman
Copy link
Collaborator

johnrengelman commented Feb 18, 2018

@mernst Why do you need to declare a dependency to the JVM's tools.jar? That dependency ships with the JVM and is available to any running program. You should just omit it.

You never have to declare libraries provided by the JVM as runtime dependencies.

@mernst
Copy link

mernst commented Feb 19, 2018

That dependency [tools.jar] ships with the JVM and is available to any running program.

Unfortunately, this is incorrect. It ships with the JDK, but it is not part of the JRE nor the JVM and it is not on the default classpath.

You should just omit it.

If I omit it, the build fails with 12 errors like this:

src/main/java/org/plumelib/options/OptionsDoclet.java:13: error: package com.sun.javadoc does not exist
import com.sun.javadoc.ClassDoc;
                      ^

Please try this, and you will see that the failure occurs when running either gradle build or gradle shadowJar.

@johnrengelman
Copy link
Collaborator

My mistake, if you aren't shipping it as part of the jar, how do you intend a user to have it then?
If you aren't shadowing it and the dependency isn't part of the POM to be resolved when someone depends on your app, just omit the library and tell users that they have to include it on the classpath when using your library.

@mernst
Copy link

mernst commented Feb 19, 2018

What does "just omit the library" mean? I expected, from the documentation, that this is what the shadow dependency would do. Can you give a concrete example of the code that should go in the build.gradle file? Recall from above that omitting the dependency in the build.gradle file does not work.

@johnrengelman
Copy link
Collaborator

using configurations.shadow forces the specified dependeny to be available at the specified relative location to the jar as encoded in the manifest file. How else would a use get tools.jar onto their classpath?

@johnrengelman
Copy link
Collaborator

If you're having issues with the behavior of configurations.shadow, then I suggest, simply creating your own configuration and adding it to gradle build in a way that does what you want.

configurations {
  tools
}

dependencies {
  tools files(org.gradle.internal.jvm.Jvm.current().toolsJar)
}

configurations.compileClasspath.extendsFrom configurations.tools

This should do it for you (maybe not exact).

@mernst
Copy link

mernst commented Feb 19, 2018

using configurations.shadow forces the specified dependeny to be available at the specified relative location to the jar as encoded in the manifest file. How else would a use get tools.jar onto their classpath?

I showed this in a comment above:

org.gradle.internal.jvm.Jvm.current().toolsJar

There is no fixed location, and even if there was, there would be no fixed relative path.

@mernst
Copy link

mernst commented Feb 19, 2018

The suggestion of adding
a configurations block, a dependencies block, and an extendsFrom directive works. Thanks! And it's shorter and more portable than the hack I was forced to devise in the absence of other information.

I suggest that it would be useful to update the manual with some of the useful information in this discussion: how to handle this use case (which has come up for several users), the assumption about shadowJar only works for dependendencies at a fixed relative location, and so forth.

Even better would be to provide an option to shadow that handles this case without additional configuration. That's what this issue is requesting.

mernst added a commit to plume-lib/options that referenced this issue Feb 19, 2018
@marcphilipp
Copy link
Contributor

@johnrengelman From the JUnit Team's point of view, this issue can be closed as "works-as-designed". Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants