AbstractPublishToMaven tasks depend on signing tasks #368

crogoz · 2024-02-16T16:10:48Z

Before this PR

Failures in gradle 8.5 projects that are publishing multiple external publications: see circleCi failure

Some problems were found with the configuration of task ':gradle-failure-reports:signPluginMavenPublication' (type 'Sign').
  - Gradle detected a problem with the following location: '/home/circleci/project/gradle-failure-reports/build/libs/gradle-failure-reports-1.1.0-javadoc.jar.asc'.
    
    Reason: Task ':gradle-failure-reports:publishMavenPublicationToSonatypeRepository' uses this output of task ':gradle-failure-reports:signPluginMavenPublication' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed.

In gradle-failure-reports we have 2 publications: maven and pluginMaven which both generate the same artifact. Each publication has a sign task, both sign tasks write to the same output and clobber each other. Gradle thinks each one of the publish tasks for the publication is using the output of the wrong sign task so complains.

After this PR

Workaround for implicit dependency gradle#26091

If the signing plugin is applied, then the publishing tasks AbstractPublishToMaven will depend on Sign tasks.
==COMMIT_MSG==
AbstractPublishToMaven tasks depend on signing tasks
==COMMIT_MSG==

Possible downsides?

changelog-app · 2024-02-16T16:10:51Z

Generate changelog in `changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

AbstractPublishToMaven tasks depend on signing tasks

Check the box to generate changelog(s)

Generate changelog entry

crogoz · 2024-02-19T13:33:59Z

...t/groovy/com/palantir/gradle/externalpublish/ExternalPublishRootPluginIntegrationSpec.groovy

+        publishGradlePlugin()
+
+        when:
+        ExecutionResult result = runSuccessfullyWithSigning('-P__TESTING_CIRCLE_TAG=tag', 'publishToMavenLocal', "--info", "--warning-mode=fail")


"--warning-mode=fail" making sure there are no gradle warnings anymore.

I think it's be better to just test with Gradle 8 using gradleVersion = '8.6'.

crogoz · 2024-02-19T15:11:32Z

...t/groovy/com/palantir/gradle/externalpublish/ExternalPublishRootPluginIntegrationSpec.groovy

@@ -120,13 +121,18 @@ class ExternalPublishRootPluginIntegrationSpec extends IntegrationSpec {

        if (type == 'gradle-plugin') {
            subprojectBuildGradle << '''
+                apply plugin: 'java-gradle-plugin'
+                apply plugin: 'groovy'
+                apply plugin: 'com.palantir.external-publish-jar'


shouldn't this be applied by default by com.palantir.external-publish-gradle-plugin ? The com.palantir.external-publish-jar sets up the MavenPublication see:

gradle-external-publish-plugin/src/main/java/com/palantir/gradle/externalpublish/ExternalPublishJarPlugin.java

Line 33 in f66f7f2

ExternalPublishBasePlugin.applyTo(project)

If I remove this line, then some of the tests will fail with:

* Exception is: org.gradle.internal.execution.WorkValidationException: A problem was found with the configuration of task ':gradle-plugin:publishPluginMavenPublicationToSonatypeRepository' (type 'PublishToMavenRepository'). - In plugin 'org.gradle.maven-publish' type 'org.gradle.api.publish.maven.tasks.PublishToMavenRepository' property 'credentials.username' doesn't have a configured value. Reason: This property isn't marked as optional and no value has been configured. Possible solutions: 1. Assign a value to 'credentials.username'. 2. Mark property 'credentials.username' as optional.```

I think the gradle plugin publish plugin also sets up a publication to publish to the Gradle plugins portal? At least they end up in different places (gradle plugin portal vs maven central). I wonder if this is bit is now wrong/has changed:

gradle-external-publish-plugin/src/main/java/com/palantir/gradle/externalpublish/ExternalPublishGradlePluginPlugin.java

Lines 51 to 57 in f125d70

ExtraPropertiesExtension extraProperties = project.getExtensions().getExtraProperties();

extraProperties.set(

"gradle.publish.key",

envVars.envVarOrFromTestingProperty("GRADLE_KEY").getOrNull());

extraProperties.set(

"gradle.publish.secret",

envVars.envVarOrFromTestingProperty("GRADLE_SECRET").getOrNull());

Honestly I thought it looked weird the other day as ExtraPropertiesExtension is super old/deprecated.

Apparently this is still the way? https://docs.gradle.org/current/userguide/publishing_gradle_plugins.html

I think the credentials setup is done per repo level (sonatypeRepo) and is configured by the nexus-publish plugin: https://github.com/palantir/gradle-external-publish-plugin/blob/develop/src/main/java/com/palantir/gradle/externalpublish/ExternalPublishRootPlugin.java#L48-L57. This means that publish<publicationName>toSonatypeRepository should get the credentials for the sonatypeRepository.
If we were to add the mavenPublication in the external-publish-gradle-plugin similar to:
https://github.com/palantir/gradle-external-publish-plugin/blob/develop/src/main/java/com/palantir/gradle/externalpublish/ExternalPublishJarPlugin.java#L33-L35
it works. I guess this is the part we are missing in the plugin: https://github.com/gradle-nexus/publish-plugin?tab=readme-ov-file#add-metadata.

Hmm, I'm a bit confused by all this. I think the Gradle plugin publish plugin is setting up a publication for us (pluginMaven?). Publishing to the Gradle plugin portal shouldn't require anything to do to with sonatype though - that's just for publishing to maven central. I guess this only need to hold up this PR if we now need to add the jar... I'll take a look and investigate.

Ok, I think I worked out what's going wrong. The Gradle plugin did not apply the ExternalPublishBasePlugin, which has a method disableOtherPublicationsFromPublishingToSonatype that stops non-sonatype publications from trying to publish to sonatype. The Gradle plugin publish plugin's publication was trying to be published to sonatype, but didn't have the creds as it never got them from the sonatype plugin. Now it is appropriately ignored.

CRogers · 2024-02-20T13:33:53Z

...t/groovy/com/palantir/gradle/externalpublish/ExternalPublishRootPluginIntegrationSpec.groovy

+
+        then:
+        result.success
+        result.wasExecuted(":gradle-plugin:publishPluginMavenPublicationToMavenLocal")


Gradle plugin portal does not need signing, we don't do it and so it doesn't need to be checked here.

CRogers · 2024-02-20T14:12:43Z

This was a nasty one

CRogers · 2024-02-20T14:12:46Z

👍🏻

crogoz · 2024-02-20T14:25:44Z

👍

svc-autorelease · 2024-02-20T14:29:27Z

Released 1.15.0

crogoz force-pushed the cr/fix-output branch from 71a1d03 to ff44f48 Compare February 19, 2024 13:32

crogoz commented Feb 19, 2024

View reviewed changes

crogoz force-pushed the cr/fix-output branch 2 times, most recently from 697d90d to d984383 Compare February 19, 2024 13:35

crogoz commented Feb 19, 2024

View reviewed changes

add dependency between sign and publish tasks

220f4a3

crogoz force-pushed the cr/fix-output branch from d984383 to 220f4a3 Compare February 19, 2024 15:15

crogoz marked this pull request as ready for review February 19, 2024 15:15

crogoz requested a review from CRogers February 19, 2024 15:16

crogoz and others added 5 commits February 19, 2024 17:26

fix name

7c977ae

test for 7.6 and 8.5

2950aec

only 8.6

3cc069e

Improve test

6ca6038

Fix issue with credentials

99f472c

CRogers reviewed Feb 20, 2024

View reviewed changes

ugh

95ad6e5

Add generated changelog entries

365516b

CRogers added autorelease merge when ready labels Feb 20, 2024

CRogers merged commit 032f988 into develop Feb 20, 2024
4 of 5 checks passed

CRogers deleted the cr/fix-output branch February 20, 2024 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AbstractPublishToMaven tasks depend on signing tasks #368

AbstractPublishToMaven tasks depend on signing tasks #368

crogoz commented Feb 16, 2024 •

edited

Loading

changelog-app bot commented Feb 16, 2024 •

edited by CRogers

Loading

crogoz Feb 19, 2024

CRogers Feb 19, 2024

crogoz Feb 19, 2024

CRogers Feb 19, 2024 •

edited

Loading

CRogers Feb 19, 2024

crogoz Feb 20, 2024

CRogers Feb 20, 2024

CRogers Feb 20, 2024

CRogers Feb 20, 2024

CRogers commented Feb 20, 2024

CRogers commented Feb 20, 2024

crogoz commented Feb 20, 2024

svc-autorelease commented Feb 20, 2024

	ExtraPropertiesExtension extraProperties = project.getExtensions().getExtraProperties();
	extraProperties.set(
	"gradle.publish.key",
	envVars.envVarOrFromTestingProperty("GRADLE_KEY").getOrNull());
	extraProperties.set(
	"gradle.publish.secret",
	envVars.envVarOrFromTestingProperty("GRADLE_SECRET").getOrNull());

AbstractPublishToMaven tasks depend on signing tasks #368

AbstractPublishToMaven tasks depend on signing tasks #368

Conversation

crogoz commented Feb 16, 2024 • edited Loading

Before this PR

After this PR

Possible downsides?

changelog-app bot commented Feb 16, 2024 • edited by CRogers Loading

Generate changelog in changelog/@unreleased

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CRogers Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CRogers commented Feb 20, 2024

CRogers commented Feb 20, 2024

crogoz commented Feb 20, 2024

svc-autorelease commented Feb 20, 2024

crogoz commented Feb 16, 2024 •

edited

Loading

changelog-app bot commented Feb 16, 2024 •

edited by CRogers

Loading

Generate changelog in `changelog/@unreleased`

CRogers Feb 19, 2024 •

edited

Loading