jdaugherty commented on code in PR #16082:
URL: https://github.com/apache/grails-core/pull/16082#discussion_r3699569658
##########
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:
DuplicatesStrategy.INCLUDE is a strong smell here - it means we've not
properly setup the input. We should error by default as it's a gradle
configuration problem
##########
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:
The unique `Sync` output for `copyCommands` is the right change. This filter
isn't.
It runs for every entry in the runtime jar on every build and does a
filesystem stat per `META-INF/commands` entry, and the only thing it
accomplishes is hiding files that a *previous version* of this plugin left in
`build/resources/main`. On a clean build there is nothing to hide — with a
companion, `src/main/scripts` never reaches `build/resources/main` at all. I
removed the filter (and the `seedStaleRuntimeCommands` fixture) and both
`PluginScriptCommandPackagingSpec` cases still pass,
`RUNTIME_HAS_HAND_AUTHORED=true` included:
`src/main/resources/META-INF/commands/hand-authored.yml` ships in the runtime
jar on its own because it is an ordinary main resource. So the filter's net
effect is to exclude everything *else*.
Two problems with that:
1. It doesn't remove the stale state, it masks the archive. The leftovers
stay in `build/resources/main`, which is `sourceSets.main.output`, so they
remain on the plugin's own `test` / integration-test / `bootRun` classpath. I
confirmed they survive the migration there.
2. It silently drops legitimate entries. Any `META-INF/commands` file not
backed by a real file under `src/main/resources/META-INF/commands` is excluded,
including command resources a plugin generates with its own task and wires in
through `processResources.from(...)`. No warning and no way to opt out.
A `build/` directory holding output from an older version of the build logic
is what `clean` is for. A packaging-time filter is a clean task in disguise,
just paid on every build instead of once. Please drop it and note the one-time
`clean` in `upgrading80x.adoc` next to the `grails-plugin-cli` instructions.
##########
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:
`seedStaleRuntimeCommands` writes into `build/resources/main`, which is
`processResources`' output directory — the exact side-write this PR removes
from the plugin itself. The fixture manufactures the only condition under which
the runtime jar filter does anything, so `RUNTIME_HAS_SCRIPT=false` and
`RUNTIME_HAS_REMOVED=false` are asserting the workaround rather than the
packaging behavior.
Drop this block along with the filter. With both gone the remaining
assertions still pass and cover what the change is actually for: scripts land
only in the companion, and hand-authored `META-INF/commands` plus templates
stay in the runtime jar.
--
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]
