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

Unable to publish only unshadowed artifacts #651

Open
ileasile opened this issue Mar 23, 2021 · 12 comments · May be fixed by #661
Open

Unable to publish only unshadowed artifacts #651

ileasile opened this issue Mar 23, 2021 · 12 comments · May be fixed by #661

Comments

@ileasile
Copy link
Contributor

I've used 5.2.0 before and then decided to migrate to 6.1.0
What I found is that project.components.java is updated with shadowJar configurations here:
https://github.com/johnrengelman/shadow/blame/b3677a26c706aec9aa7b035b9d36a091523aff97/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowJavaPlugin.groovy#L56

So, I can't use maven-publish to publish only raw, unshadowed JARs: publication JAR conflict happens.
Please, make adding shadowJar things to java software component optional: I'd enable this option in project config.
Thank you!

ileasile added a commit to Kotlin/kotlin-jupyter that referenced this issue Mar 23, 2021
@m8nmueller
Copy link
Contributor

m8nmueller commented Apr 8, 2021

The line you quoted is about dependency handling of the shadowRuntimeElements source set. It has nothing to do with publications.

I suggest you remove this line in your build script which bypasses the normal measurements for distinguishing the shadow jar artifact from plain old jar task's one (archiveClassifier).

Edit: I'm sorry, it slowly came to my mind that those shadowRuntimeElements are the reason why the shadowJar task is executed.
It is possible but undoubtedly not a good option to include that in the build script:

getComponents().withType(AdhocComponentWithVariants::class.java).forEach { c ->
  c.withVariantsFromConfiguration(project.configurations.shadowRuntimeElements.get()) {
    skip()
  }
}

@chali
Copy link

chali commented May 5, 2021

I was actually thinking about the same or very similar problem. In our ecosystem we see those 3 modes how shadow jar is used:

  1. Users want to replace normal jar with shaded jar but there is no name difference from consumer perspective it is "just" a jar
  2. Users want to have both normal and shaded and differentiate between them with a classifier
  3. Users want to have both normal and shaded jar but they want to use a different artifact name

Unfortunately when switching to Gradle Metadata I found it is not possible to achieve those modes with current support in shadow plugin.

My understanding is that currently there is a new variant registered into java component. Users now have to provide appropriate attributes to pick which one is supposed to be used otherwise gradle fails dependency resolution because it cannot decide which one should be used. This maps in my eyes to the use case two but one and three are not achievable.

I was thinking about following proposal.

  • Leave component java as is without any extra variant.
  • Introduce shadow component which would have only one variant with shaded jar.
  • Introduce javaAndShadow which would have both. Basically what now java component does.

Use cases now would use following combinations:

  1. Just shadow component
  2. Just javaAndShadow component, classifier selection will be replaced with attribute selection.
  3. Both java and shadow components with different artifact name configured.

What do you think. If this is something you would like to pursue I'm happy to look into details and attempt some PR for it.

I can see argument that we are changing meaning of java. I can see that we could keep java as it is and introduce javaOnly component. However I personally would keep java as Gradle itself defines it.

@johnrengelman
Copy link
Collaborator

On the surface, I think @chali's analysis is correct. I haven't educated myself fully on how the variants should actually be used. The current implementation was provided by @melix so I'd defer to him on what the idiomatic implementation should be here. (if he doesn't mind chiming in ;) )

@netvl
Copy link

netvl commented Sep 30, 2021

From my experience, correctly setting up a custom component like java seems to be not very easy to do correctly. Instead, it seems to me that it would be easier to just add some configuration for the existing java component, and provide some switches to turn publishing of various configurations off, when necessary. I wrote a more extensive explanation here: #693 (comment)

However, if you find that creating new components is not that hard, then different components seems like the best way to go, since it would allow doing something like

publishing {
    publications {
        register<MavenPublication>("regular") {
            from(components.java)
        }
        register<MavenPublication>("shadow") {
            from(components.shadow)
            artifactId = artifactId + "_shaded"
        }
    }
}

@netvl
Copy link

netvl commented Sep 30, 2021

Also, I don't think it is a good idea to promote the "shadow jar as classifier" approach. The problem with it is that the shadow jar usually has a different set of dependencies than the regular jar (which is the whole point of shading), but with the shadow jar published under a non-default classifier, it will still use the "primary" POM descriptor and all dependencies which are coming with it. Which kind of makes the point of shading moot.

@melix
Copy link
Contributor

melix commented Oct 1, 2021

I think @chali 's analysis makes sense. However, this:

Users now have to provide appropriate attributes to pick which one is supposed to be used otherwise gradle fails dependency resolution because it cannot decide which one should be used.

doesn't look correct. In the Java ecosystem there are disambiguation rules which prefer the normal variant over the shadow one, which I think is the correct behavior. There shouldn't be any error if the Java plugin is applied.

Now as to creating several components so that the producer has the choice, it's one option. Another one is to expose configuration options to tell what to do, and only use the java component. For example:

shadow {
    publicationMode = PublicationMode.STANDARD_JAR_ONLY
       // or STANDARD_AND_SHADOW
      // or SHADOW_ONLY
}

Ideally it should make the difference between the shaded (repackaged) versions and fat jar (jars are merged but packages don't change).

Also, I don't think it is a good idea to promote the "shadow jar as classifier" approach

I think this approach is significantly better than having a separate publication with different GAV coordinates: effectively the shadow jar and the normal jar are the same component. There's no reason they should use different GAV coordinates. When published this way with Gradle, the module file contains the right metadata and a user consuming the normal jar will see the transitive dependencies, while one consuming the "shadow" jar will only see the jar without dependencies. It's important because Gradle will be able to tell if the same component is twice on the graph with different variants and let the user choose the right one. For Maven users, the situation is not any worse than before, in the sense that they will see a classifier with dependencies. That's bad luck, they should move to a superior build tool ;)

@netvl
Copy link

netvl commented Oct 1, 2021

For Maven users, the situation is not any worse than before, in the sense that they will see a classifier with dependencies. That's bad luck, they should move to a superior build tool ;)

Yeah, that's what I meant. For Maven users, a shaded jar published with a classifier is worse than a separate artifact id, precisely because the POM descriptor will have "non-shaded" dependencies.

For better or for worse, Maven POM descriptors are the interchange format accepted by all build tools in the JVM ecosystem, and I personally think it would be a bad idea to provide an option which works okay in Gradle but produces subpar results for everyone else. There are other build tools which are superior to plain Maven (SBT, Bazel, Leiningen, ...) which naturally don't understand Gradle modules and most likely will not, and their use is not insignificant, and sometimes there is a requirement to integrate with them. Also, in some cases Gradle modules just can't be used even with Gradle (due to particular Artifactory setup which does not accept .module files).

@melix
Copy link
Contributor

melix commented Oct 4, 2021

For better or for worse, Maven POM descriptors are the interchange format accepted by all build tools in the JVM ecosystem, and I personally think it would be a bad idea to provide an option which works okay in Gradle but produces subpar results for everyone else.

By reasoning like this the industry would never make any progress. Classifiers are the way Maven models variants. Folks are used to this. Gradle supports variants natively and models their dependencies properly. There's a correct model that other tools should use, and I think it's our responsibility, as users, as developers, to encourage the use of tools which fix problems.

@m8nmueller
Copy link
Contributor

I was thinking about following proposal.

* Leave component `java` as is without any extra variant.

* Introduce `shadow` component which would have only one variant with shaded jar.

* Introduce `javaAndShadow` which would have both. Basically what now `java` component does.

I do like this proposal by @chali because users get the freedom to decide themselves (e. g. whether they want to support Maven consumers). The Gradle-idiomatic way - as to my understanding - is one component with multiple variants but then, you can't publish shadow/basic jar to different GAV coordinates (would be an RFE for Gradle publishing). Therefore, to allow different artifacts for the two jars, separate components are necessary.

@netvl
Copy link

netvl commented Oct 6, 2021

By reasoning like this the industry would never make any progress.

Gradle supports variants natively and models their dependencies properly. There's a correct model that other tools should use, and I think it's our responsibility, as users, as developers, to encourage the use of tools which fix problems.

I agree with you, making progress is very important, but unfortunately some things just can't be changed right away, or even in some close perspective. For example, look at so many attempts to replace C, with all its inherent problems. Some went quite a long way, even, like Rust (which I personally really like!). But C is not even close to replacement by anything. Same with Java. Same, I would suppose, is with Maven. The inertia behind its ecosystem is simply huge. And making people who depend on legacy stuff suffer in the name of progress, well, I think is not a good way to go.

Classifiers are the way Maven models variants. Folks are used to this.

I'm not quite sure about this. Classifiers certainly have their uses, but with regards specifically to shading, Maven-based libraries mostly use name suffixes. Just look at the number of artifacts with the -shaded suffix in their name, and this is not the only naming pattern out there. And the reason is precisely that classifiers can't provide a different set of dependencies compared to the "main" artifact. Mapping from Gradle variants to classifiers just can't be done in a sufficiently powerful way.

@melix
Copy link
Contributor

melix commented Oct 6, 2021

I don't really want to go into arguments, but IMO any solution which goes against the supported Gradle model isn't a good choice. So I'd recommend doing what @chali suggested.

@soloturn
Copy link

soloturn commented Feb 13, 2022

@melix , @johnrengelman you think @MaxM123 pull request would appropriately address the challenge?

switching to the 5.2.0 version allows to run publsh, but other tasks are not picking it up, like signing all artifacts before uploading them to github.

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

Successfully merging a pull request may close this issue.

7 participants