codeconsole commented on code in PR #39:
URL:
https://github.com/apache/grails-gradle-publish/pull/39#discussion_r4115830779
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -806,6 +931,28 @@ Note: if project properties are used, the properties must
be defined prior to ap
] : null
}
+ /**
+ * The extra artifact usually lives in a classes directory, such as
META-INF/grails-plugin.xml. Publishing it from
+ * there would write its signature into that directory, which other tasks
read, such as the jar tasks, so publish a
+ * copy instead.
+ */
+ private static void addExtraArtifact(Project project, MavenPublication
publication, Map<String, String> extraArtifact) {
+ File source = new File(extraArtifact.source)
+ TaskProvider<Copy> copyTask =
project.tasks.register('grailsPublishExtraArtifact', Copy) { Copy copy ->
+ copy.from(source)
+
copy.into(project.layout.buildDirectory.dir('grails-publish/extra-artifact'))
+ // the file is written by the task producing the classes directory
it lives in, and the jar tasks package
+ // those directories, so depending on them builds it
+ copy.dependsOn(project.tasks.withType(Jar))
Review Comment:
`withType(Jar)` includes `testSourcesJar`, which `dependsOn('testClasses')`
even when `publishTestSources` is off. The `onlyIf` skips the jar but not its
dependencies. So a snapshot `publish` or `publishToMavenLocal` of any project
with a `grails-plugin.xml` now compiles its tests. Before, only release builds
did that, through the removed `Sign` → every `Jar` dependency. This also pulls
in any other `Jar` in the project, such as `bootJar`.
I checked with the `grails-plugin-project` fixture plus one test class,
running `publishToMavenLocal --dry-run` as a snapshot:
- `1.0.x`: no test tasks in the graph
- this branch: adds `compileTestJava`, `compileTestGroovy`, `testClasses`
and `testSourcesJar`
Depending on `classes` covers both extra artifacts. `compileGroovy` writes
`grails-plugin.xml` (`GlobalGrailsClassInjectorTransformation`). In
grails-core, `GrailsProfileGradlePlugin` makes `classes` depend on
`compileProfile`, which writes `profile.yml`.
```suggestion
// the file is written while building the main classes:
grails-plugin.xml by compileGroovy, and
// grails-core's profile.yml by compileProfile, which its
profile plugin adds to `classes`
copy.dependsOn(project.tasks.named('classes'))
```
With this change, the fixture still publishes the `-plugin.xml`, without the
test tasks. The full functional suite also passes locally: 60 tests, 0
failures, and the same 6 skipped.
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishExtension.groovy:
##########
@@ -194,7 +197,9 @@ class GrailsPublishExtension {
testRepositoryPath = objects.directoryProperty().convention(null as
Directory)
pomCustomization = objects.property(Closure).convention(null as
Closure)
addComponents = objects.property(Boolean).convention(true)
- publicationName = objects.property(String).convention('maven')
+ publicationName = objects.property(String).convention(project.provider
{
+ isGradlePluginProject(project) ? 'pluginMaven' : 'maven'
Review Comment:
Nit: this checks only whether `java-gradle-plugin` is applied, but
`isComponentAddedByJavaGradlePlugin` also requires `automatedPublishing`. With
`gradlePlugin { automatedPublishing = false }` there's no `pluginMaven`
publication to reuse, yet the primary publication is still named `pluginMaven`,
so its task names change for no benefit. This provider is only read in
`afterEvaluate`, so it can use the same check, and `isGradlePluginProject` can
go:
```suggestion
GrailsPublishGradlePlugin.isComponentAddedByJavaGradlePlugin(project,
'pluginMaven') ? 'pluginMaven' : 'maven'
```
The README's "When it is applied, `publicationName` defaults to
`pluginMaven`" would then need an "unless `automatedPublishing` is disabled". I
checked that this compiles and that the unit tests and the
gradle-plugin-project specs pass.
##########
plugin/src/functionalTest/groovy/org/apache/grails/gradle/publish/ReleaseSigningSpec.groovy:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.gradle.publish
+
+import org.gradle.testkit.runner.GradleRunner
+import spock.lang.Requires
+import spock.lang.Shared
+
+import java.nio.file.Files
+import java.nio.file.Path
+import java.util.concurrent.TimeUnit
+
+/**
+ * Signs and publishes releases with a throwaway GPG key, to cover the sign
and publish task wiring.
+ */
+@Requires({ ReleaseSigningSpec.gpgAvailable() })
+class ReleaseSigningSpec extends GradleSpecification {
+
+ @Shared
+ Path gnupgHome
+
+ @Shared
+ String keyId
+
+ List<File> toCleanup = []
+
+ void setupSpec() {
+ // keep the path short, since gpg-agent's socket path is limited in
length
+ gnupgHome = Files.createTempDirectory('gpg')
+ gpg('--batch', '--passphrase', '', '--quick-gen-key', 'Throwaway Test
Key <[email protected]>', 'rsa2048', 'sign', 'never')
+ keyId = gpg('--list-keys', '--with-colons').readLines()
+ .find { it.startsWith('pub:') }
+ .split(':')[4]
+ .takeRight(8)
+ }
+
+ void cleanup() {
+ toCleanup.each { it.deleteDir() }
+ }
+
+ void cleanupSpec() {
+ runGpgconf('--kill', 'gpg-agent')
+ gnupgHome.toFile().deleteDir()
+ }
+
+ def "a signed release signs every published file - #description"() {
+ given:
+ File repository = File.createTempDir('release-repository')
+ File mavenLocal = File.createTempDir('release-maven-local')
+ toCleanup << repository << mavenLocal
+
+ and:
+ GradleRunner runner = setupTestResourceProject('other-artifacts',
fixture)
+ runner = setGradleProperty('projectVersion', '0.0.1', runner)
+ runner = setGradleProperty('releasePublishType', 'MAVEN_PUBLISH',
runner)
+ runner = setGradleProperty('mavenPublishUrl', repository.absolutePath,
runner)
+ runner = addEnvironmentVariable('GRAILS_PUBLISH_RELEASE', 'true',
runner)
+ // the build environment is otherwise empty, and signing runs the gpg
command
+ runner = addEnvironmentVariable('PATH', System.getenv('PATH'), runner)
+ runner = addEnvironmentVariable('GNUPGHOME',
gnupgHome.toAbsolutePath().toString(), runner)
+ runner = addEnvironmentVariable('SIGNING_KEY', keyId, runner)
+ environment.each { String key, String value ->
+ runner = addEnvironmentVariable(key, value, runner)
+ }
+
+ when:
+ executeTask('publish', ['publishToMavenLocal',
"-Dmaven.repo.local=${mavenLocal.absolutePath}".toString()], runner)
+
+ then:
+ List<File> published = publishedFiles(repository)
+ published
+ published.findAll { !signatureVerifies(it) } == []
+
+ and:
+ List<File> publishedLocally = publishedFiles(mavenLocal)
+ publishedLocally
+ publishedLocally.findAll { !signatureVerifies(it) } == []
+
+ where:
+ fixture | environment |
description
+ 'simple-project' | [:] |
'one publication'
+ 'additional-publication' | [:] |
'an additional publication'
+ 'gradle-plugin-project' | [:] |
'Gradle plugin project, java-gradle-plugin applied last'
+ 'gradle-plugin-project' | [APPLY_JAVA_GRADLE_PLUGIN_FIRST: 'true'] |
'Gradle plugin project, java-gradle-plugin applied first'
+ 'gradle-plugin-project' | [PUBLISH_SHARED_ARTIFACTS: 'true'] |
'publications sharing artifacts'
+ }
+
+ def "a signed release publishes the grails-plugin.xml without signing it
inside the classes directory"() {
+ given:
+ File repository = File.createTempDir('release-repository')
+ toCleanup << repository
+
+ and:
+ GradleRunner runner = setupTestResourceProject('other-artifacts',
'grails-plugin-project')
+ runner = setGradleProperty('projectVersion', '0.0.1', runner)
+ runner = setGradleProperty('releasePublishType', 'MAVEN_PUBLISH',
runner)
+ runner = setGradleProperty('mavenPublishUrl', repository.absolutePath,
runner)
+ runner = addEnvironmentVariable('GRAILS_PUBLISH_RELEASE', 'true',
runner)
+ runner = addEnvironmentVariable('PATH', System.getenv('PATH'), runner)
+ runner = addEnvironmentVariable('GNUPGHOME',
gnupgHome.toAbsolutePath().toString(), runner)
+ runner = addEnvironmentVariable('SIGNING_KEY', keyId, runner)
+
+ and: 'the grails-plugin.xml exists, as the plugin only publishes it
when it does at configuration time'
+ executeTask('classes', runner)
+
+ when:
+ executeTask('publish', runner)
+
+ then: 'the grails-plugin.xml is published and signed'
+ File publishedPluginXml = new File(repository,
'org/grails/example/grails-plugin-project/0.0.1/grails-plugin-project-0.0.1-plugin.xml')
+ publishedPluginXml.text == '<plugin name="grails-plugin-project"/>'
+ signatureVerifies(publishedPluginXml)
+
+ and: 'its signature is not written into the classes directory, which
other tasks read'
+ !new File(runner.projectDir,
'build/classes/groovy/main/META-INF/grails-plugin.xml.asc').exists()
+ }
+
+ private static List<File> publishedFiles(File directory) {
+ List<File> files = []
+ directory.eachFileRecurse { File file ->
+ if (file.name ==~ /.*\.(jar|pom|module)/) {
+ files << file
+ }
+ }
+ files
+ }
+
+ /**
+ * Whether the file's .asc signature exists and verifies against the
throwaway key.
+ */
+ private boolean signatureVerifies(File file) {
+ File signature = new File("${file.path}.asc")
+ if (!signature.exists()) {
+ return false
+ }
+ try {
+ gpg('--batch', '--verify', signature.path, file.path)
+ true
+ } catch (IllegalStateException ignored) {
+ false
+ }
+ }
+
+ private String gpg(String... arguments) {
+ run(['gpg', '--homedir', gnupgHome.toString()] + arguments.toList())
+ }
+
+ private void runGpgconf(String... arguments) {
+ try {
+ run(['gpgconf', '--homedir', gnupgHome.toString()] +
arguments.toList())
+ } catch (Exception ignored) {
+ // the agent is gone with the temporary directory either way
+ }
+ }
+
+ private static String run(List<String> command) {
+ Process process = new
ProcessBuilder(command).redirectErrorStream(true).start()
+ String output = process.inputStream.text
+ if (!process.waitFor(2, TimeUnit.MINUTES) || process.exitValue() != 0)
{
+ throw new IllegalStateException("${command.join(' ')}
failed:\n${output}")
+ }
+ output
Review Comment:
Nit: `inputStream.text` returns only when gpg closes its output, which is
when it exits, so `waitFor(2, TimeUnit.MINUTES)` never times out. If gpg hangs,
for example on the agent or a pinentry prompt, the spec hangs until the CI job
times out. Reading the output on a separate thread keeps the limit:
```suggestion
StringBuilder output = new StringBuilder()
Thread reader = process.consumeProcessOutputStream(output)
if (!process.waitFor(2, TimeUnit.MINUTES)) {
process.destroyForcibly()
throw new IllegalStateException("${command.join(' ')} timed
out:\n${output}")
}
reader.join()
if (process.exitValue() != 0) {
throw new IllegalStateException("${command.join(' ')}
failed:\n${output}")
}
output.toString()
```
`ReleaseSigningSpec` passes locally with this.
##########
plugin/src/main/groovy/org/apache/grails/gradle/publish/GrailsPublishGradlePlugin.groovy:
##########
@@ -137,89 +171,127 @@ The credentials and connection url must be specified as
a project property or an
When using `NEXUS_PUBLISH`, either the property `signing.secretKeyRingFile`
must be set to the path of the GPG keyring file or local gpg must be configured
to sign artifacts.
-Note: if project properties are used, the properties must be defined prior to
applying this plugin.
+Note: properties are read from the root project's gradle.properties, the one
in the Gradle user home, -P or ORG_GRADLE_PROJECT_ environment variables, or
from the project applying this plugin: its own gradle.properties, or its build
script before the plugin is applied. Properties set on parent projects,
including in the gradle.properties of a parent project's directory, are not
read.
"""
}
+ /**
+ * Finds a property set via `ext` on the given project itself, or a Gradle
property.
+ *
+ * The project's own extra properties are checked first, so a value set in
its build script overrides a Gradle property,
+ * as it does with {@link Project#findProperty}. Properties set on parent
projects are deliberately not read: resolving
+ * them implicitly is removed in Gradle 10, and reading them explicitly is
not allowed with Isolated Projects.
+ */
+ @PackageScope
+ static Object findProjectProperty(Project project, String name) {
+ def extraProperties = project.extensions.extraProperties
+ if (extraProperties.has(name)) {
+ return extraProperties.get(name)
+ }
+ project.providers.gradleProperty(name).orNull
+ }
+
+ /**
+ * Finds a publish type property. These fall back to a default when unset,
so a value that is only set on a parent
+ * project, which is no longer read, would silently change where artifacts
are published. Fail the build instead.
+ */
+ private Object findPublishTypeProperty(Project project, String name) {
+ Object value = findProjectProperty(project, name)
+ // with Isolated Projects, parent projects cannot be inspected, and
such builds never relied on reading them
+ if (value == null && !buildFeatures.isolatedProjects.active.get()) {
+ for (Project parent = project.parent; parent != null; parent =
parent.parent) {
+ if (parent.extensions.extraProperties.has(name)) {
+ throw new InvalidUserDataException("The property `${name}`
is set on ${parent} but not on ${project}. " +
+ 'The Grails Publish plugin does not read
properties from parent projects. ' +
+ "Set `${name}` in the root project's
gradle.properties, with -P${name}=..., or on ${project} " +
+ 'itself, in its gradle.properties or in its build
script before applying the plugin.')
+ }
+ }
+ }
+ value
+ }
+
@Override
void apply(Project project) {
- project.rootProject.logger.info("Applying Grails Publish Gradle Plugin
for `${project.name}`...");
+ LOG.info('Applying Grails Publish Gradle Plugin for `{}`...',
project.name)
if (project.extensions.findByName('grailsPublish') == null) {
project.extensions.create('grailsPublish', GrailsPublishExtension)
}
- final String nexusPublishUrl = project.findProperty('nexusPublishUrl')
?: System.getenv('NEXUS_PUBLISH_URL') ?: ''
- final String nexusPublishSnapshotUrl =
project.findProperty('nexusPublishSnapshotUrl') ?:
System.getenv('NEXUS_PUBLISH_SNAPSHOT_URL') ?: ''
- final String nexusPublishUsername =
project.findProperty('nexusPublishUsername') ?:
System.getenv('NEXUS_PUBLISH_USERNAME') ?: ''
- final String nexusPublishPassword =
project.findProperty('nexusPublishPassword') ?:
System.getenv('NEXUS_PUBLISH_PASSWORD') ?: ''
- final String nexusPublishStagingProfileId =
project.findProperty('nexusPublishStagingProfileId') ?:
System.getenv('NEXUS_PUBLISH_STAGING_PROFILE_ID') ?: ''
- final String nexusPublishDescription =
project.findProperty('nexusPublishDescription') ?:
System.getenv('NEXUS_PUBLISH_DESCRIPTION') ?: ''
+ final String nexusPublishUrl = findProjectProperty(project,
'nexusPublishUrl') ?: System.getenv('NEXUS_PUBLISH_URL') ?: ''
+ final String nexusPublishSnapshotUrl = findProjectProperty(project,
'nexusPublishSnapshotUrl') ?: System.getenv('NEXUS_PUBLISH_SNAPSHOT_URL') ?: ''
Review Comment:
Question: the reasoning behind `findPublishTypeProperty` applies to these
two as well. If `nexusPublishUrl` or `nexusPublishSnapshotUrl` is set only on a
parent project's `ext`, it's now ignored and never set on the repository. The
Nexus plugin's `sonatype {}` then falls back to `https://oss.sonatype.org/...`
(`DefaultNexusRepositoryContainer.kt` in 2.0.0). If the credentials come from
`NEXUS_PUBLISH_USERNAME`/`NEXUS_PUBLISH_PASSWORD`, the release goes to
oss.sonatype.org with those credentials instead of failing.
Should these also fail when they're set on a parent, but not on the project
or in the environment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
