jamesfredley commented on code in PR #16082:
URL: https://github.com/apache/grails-core/pull/16082#discussion_r3699658489
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsPluginGradlePlugin.groovy:
##########
@@ -244,22 +248,71 @@ class GrailsPluginGradlePlugin extends GrailsGradlePlugin
{
}
}
+ /**
+ * Packages plugin templates into the runtime jar and routes command
scripts into either the
+ * runtime jar or the companion {@code -cli} jar.
+ *
+ * <p>When {@link GrailsCliArtifactGradlePlugin} is applied, {@code
src/main/scripts} is copied
+ * into the cli source set as {@code META-INF/commands} so
Groovy/YAML/JSON command resources
+ * ship only on {@code grailsCliClasspath} and stay out of {@code
runtimeClasspath},
+ * {@code bootJar}, and {@code bootWar}. Without a companion, the
historical behavior is
+ * preserved: scripts remain in the runtime plugin jar so unmigrated
Grails 7 plugins and
+ * {@code legacyCommandSupport} consumers keep discovering them on the
application classpath.
+ * Templates always stay on the runtime jar.</p>
+ *
+ * <p>{@code copyCommands} and {@code copyTemplates} are {@link Sync}
tasks with unique output
+ * directories under the build dir (never writing into {@code
processResources.destinationDir} as
+ * a side effect). The appropriate {@code process*Resources} task consumes
that output via
+ * {@code from(...)}, so Gradle owns each path and can drop removed
sources without a forced
+ * clean task wired into every build.</p>
+ */
@CompileDynamic
protected void configurePluginResources(Project project) {
project.afterEvaluate() {
ProcessResources processResources = (ProcessResources)
project.tasks.getByName('processResources')
+ boolean hasCliCompanion =
project.pluginManager.hasPlugin(GrailsCliArtifactGradlePlugin.PLUGIN_ID)
- TaskProvider<Copy> copyCommands =
project.tasks.register('copyCommands', Copy) {
- from("${project.projectDir}/src/main/scripts")
- into("${processResources.destinationDir}/META-INF/commands")
+ // Unique Sync outputs - never side-write into
processResources.destinationDir.
+ TaskProvider<Sync> copyCommands =
project.tasks.register('copyCommands', Sync) { Sync sync ->
+ sync.from("${project.projectDir}/src/main/scripts")
+
sync.into(project.layout.buildDirectory.dir('tmp/grails-plugin-commands'))
}
- TaskProvider<Copy> copyTemplates =
project.tasks.register('copyTemplates', Copy) {
- from("${project.projectDir}/src/main/templates")
- into("${processResources.destinationDir}/META-INF/templates")
+ TaskProvider<Sync> copyTemplates =
project.tasks.register('copyTemplates', Sync) { Sync sync ->
+ sync.from("${project.projectDir}/src/main/templates")
+
sync.into(project.layout.buildDirectory.dir('tmp/grails-plugin-templates'))
}
+
processResources.setDuplicatesStrategy(DuplicatesStrategy.INCLUDE)
- processResources.dependsOn(copyCommands, copyTemplates)
+ processResources.from(copyTemplates) { into 'META-INF/templates' }
+
+ if (hasCliCompanion) {
+ SourceSet cliSourceSet =
project.extensions.getByType(SourceSetContainer)
+
.getByName(GrailsCliArtifactGradlePlugin.CLI_SOURCE_SET_NAME)
+ ProcessResources commandResources = (ProcessResources)
project.tasks
+ .getByName(cliSourceSet.processResourcesTaskName)
+
commandResources.setDuplicatesStrategy(DuplicatesStrategy.INCLUDE)
+ commandResources.from(copyCommands) { into 'META-INF/commands'
}
+ // Prior builds may have left script copies under
processResources.destinationDir
+ // (Copy does not purge foreign files). Filter the runtime jar
so only hand-authored
+ // src/main/resources/META-INF/commands entries remain - no
forced clean task.
+ File handAuthoredCommands =
project.file('src/main/resources/META-INF/commands')
+ project.tasks.named('jar', Jar).configure { Jar jarTask ->
+ jarTask.eachFile { FileCopyDetails details ->
+ String path = details.path.replace('\\', '/')
+ if (path.startsWith('META-INF/commands/') && path !=
'META-INF/commands/') {
+ String relative =
path.substring('META-INF/commands/'.length())
+ if (!new File(handAuthoredCommands,
relative).file) {
+ details.exclude()
+ }
+ }
+ }
+ }
Review Comment:
Addressed in the latest push - pure unique-`Copy` design, no packaging-time
filter:
1. **`Copy` not `Sync`** - unique dirs under
`build/tmp/grails-plugin-{commands,templates}`; end apps / asset-pipeline keep
`tasks.named('copyTemplates', Copy)`
2. **`process*Resources` only composes unique outputs** via `from(...)` - no
side-writes into `processResources.destinationDir`
3. **Dropped `DuplicatesStrategy.INCLUDE`** - overlapping paths fail as
configuration errors
4. **Dropped the runtime jar `eachFile` filter** and
`seedStaleRuntimeCommands` fixture
5. **`upgrading80x`**: one-time `./gradlew clean` after first applying
`grails-plugin-cli` on a previously built tree
`PluginScriptCommandPackagingSpec` asserts companion vs non-companion
packaging + unique Copy output path only.
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsPluginGradlePlugin.groovy:
##########
@@ -244,22 +248,71 @@ class GrailsPluginGradlePlugin extends GrailsGradlePlugin
{
}
}
+ /**
+ * Packages plugin templates into the runtime jar and routes command
scripts into either the
+ * runtime jar or the companion {@code -cli} jar.
+ *
+ * <p>When {@link GrailsCliArtifactGradlePlugin} is applied, {@code
src/main/scripts} is copied
+ * into the cli source set as {@code META-INF/commands} so
Groovy/YAML/JSON command resources
+ * ship only on {@code grailsCliClasspath} and stay out of {@code
runtimeClasspath},
+ * {@code bootJar}, and {@code bootWar}. Without a companion, the
historical behavior is
+ * preserved: scripts remain in the runtime plugin jar so unmigrated
Grails 7 plugins and
+ * {@code legacyCommandSupport} consumers keep discovering them on the
application classpath.
+ * Templates always stay on the runtime jar.</p>
+ *
+ * <p>{@code copyCommands} and {@code copyTemplates} are {@link Sync}
tasks with unique output
+ * directories under the build dir (never writing into {@code
processResources.destinationDir} as
+ * a side effect). The appropriate {@code process*Resources} task consumes
that output via
+ * {@code from(...)}, so Gradle owns each path and can drop removed
sources without a forced
+ * clean task wired into every build.</p>
+ */
@CompileDynamic
protected void configurePluginResources(Project project) {
project.afterEvaluate() {
ProcessResources processResources = (ProcessResources)
project.tasks.getByName('processResources')
+ boolean hasCliCompanion =
project.pluginManager.hasPlugin(GrailsCliArtifactGradlePlugin.PLUGIN_ID)
- TaskProvider<Copy> copyCommands =
project.tasks.register('copyCommands', Copy) {
- from("${project.projectDir}/src/main/scripts")
- into("${processResources.destinationDir}/META-INF/commands")
+ // Unique Sync outputs - never side-write into
processResources.destinationDir.
+ TaskProvider<Sync> copyCommands =
project.tasks.register('copyCommands', Sync) { Sync sync ->
+ sync.from("${project.projectDir}/src/main/scripts")
+
sync.into(project.layout.buildDirectory.dir('tmp/grails-plugin-commands'))
}
- TaskProvider<Copy> copyTemplates =
project.tasks.register('copyTemplates', Copy) {
- from("${project.projectDir}/src/main/templates")
- into("${processResources.destinationDir}/META-INF/templates")
+ TaskProvider<Sync> copyTemplates =
project.tasks.register('copyTemplates', Sync) { Sync sync ->
+ sync.from("${project.projectDir}/src/main/templates")
+
sync.into(project.layout.buildDirectory.dir('tmp/grails-plugin-templates'))
}
+
processResources.setDuplicatesStrategy(DuplicatesStrategy.INCLUDE)
- processResources.dependsOn(copyCommands, copyTemplates)
+ processResources.from(copyTemplates) { into 'META-INF/templates' }
+
+ if (hasCliCompanion) {
+ SourceSet cliSourceSet =
project.extensions.getByType(SourceSetContainer)
+
.getByName(GrailsCliArtifactGradlePlugin.CLI_SOURCE_SET_NAME)
+ ProcessResources commandResources = (ProcessResources)
project.tasks
+ .getByName(cliSourceSet.processResourcesTaskName)
+
commandResources.setDuplicatesStrategy(DuplicatesStrategy.INCLUDE)
Review Comment:
Agreed - `DuplicatesStrategy.INCLUDE` was covering up overlapping inputs.
Removed. With unique Copy outputs composed only via `from(...)`, duplicate
paths should surface as a configuration error rather than being silently merged.
##########
grails-gradle/plugins/src/test/resources/test-projects/plugin-script-commands/plugin-with-cli/build.gradle:
##########
@@ -0,0 +1,67 @@
+// Plugin that publishes a companion -cli artifact. src/main/scripts must land
in the companion
+// jar (META-INF/commands) and must not ship in the runtime plugin jar.
+plugins {
+ id 'org.apache.grails.gradle.grails-plugin'
+ id 'org.apache.grails.gradle.grails-plugin-cli'
+}
+
+group = 'org.example.test'
+
+grails {
+ bom = null
+ cliAutoProvision = false
+}
+
+cliArtifact {
+ defaultDependencies = false
+ automaticModuleName = 'org.example.test.plugin.with.cli'
+}
+
+// Seed leftover script copies under main resources (prior non-companion
packaging). The runtime
+// jar filter must drop them without a forced clean task while keeping
hand-authored commands.
+tasks.register('seedStaleRuntimeCommands') {
+ def staleDir =
layout.buildDirectory.dir('resources/main/META-INF/commands')
+ doLast {
+ def dir = staleDir.get().asFile
+ dir.mkdirs()
+ new File(dir, 'example-script.groovy').text = "description('stale')
{}\n"
+ new File(dir, 'removed-before-migration.groovy').text =
"description('gone') {}\n"
+ }
+}
+
+tasks.named('jar', Jar).configure { jarTask ->
+ jarTask.dependsOn('seedStaleRuntimeCommands')
+}
Review Comment:
Dropped `seedStaleRuntimeCommands` and the filter it was justifying. The
fixture now only asserts the packaging contract on a clean composition of
unique Copy outputs.
--
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]
