jamesfredley commented on code in PR #16011: URL: https://github.com/apache/grails-core/pull/16011#discussion_r3639913619
########## grails-core-cli-legacy/src/main/groovy/grails/dev/commands/ApplicationCommand.groovy: ########## @@ -0,0 +1,77 @@ +/* + * 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 grails.dev.commands + +import groovy.transform.CompileStatic + +import org.springframework.context.ConfigurableApplicationContext + +import grails.util.Described +import grails.util.GrailsNameUtils +import grails.util.Named + +/** + * Represents a command that runs with access to the + * {@link org.springframework.context.ApplicationContext}. + * + * @author Graeme Rocher + * @since 3.0 + * @deprecated since 8.0, use {@link org.apache.grails.core.cli.ApplicationCommand}. Retained only for backwards compatibility with Grails 7 command plugins and slated for removal in a future major release. + */ +@Deprecated +@CompileStatic +trait ApplicationCommand implements Named, Described { Review Comment: Implemented this runtime-only design in `75ed39351b`. The neutral registry reads `META-INF/grails.factories` properties without loading legacy classes or resolving anything during Gradle configuration, logs each originating resource with the exact opt-in/upgrade remediation when no provider handles the key, and records the hint. Both the runner and shell unknown-command paths append that hint. Tests cover duplicate resources, malformed/enumeration failures, no false positive when the bridge provider is present, and both user-facing paths. ########## grails-console/src/main/groovy/grails/ui/command/GrailsApplicationContextCommandRunner.groovy: ########## @@ -45,8 +46,9 @@ class GrailsApplicationContextCommandRunner extends DevelopmentGrailsApplication ConfigurableApplicationContext run(String... args) { def command = ApplicationContextCommandRegistry.instance.findCommand(commandName) if (command) { + Object autowireTarget = resolveAutowireTarget(command) Review Comment: The blocker is the precompiled JVM ABI that the legacy module must restore. A Grails 7 binary implements `handle(grails.dev.commands.ExecutionContext)`, while the Grails 8 contract requires `handle(org.apache.grails.core.cli.ExecutionContext)`; those are different JVM descriptors, and inheritance does not adapt the argument type. Groovy 4 also bakes the legacy trait helper/field-accessor owners and `GrailsApplicationCommand` forwarders into plugin bytecode. Making the legacy trait inherit the new trait would therefore leave existing Grails 7 binaries without the new method and break the compatibility promise. The optional `LegacyApplicationCommandAdapter` is the boundary that translates execution contexts, while `ApplicationCommandTargetAware` lets the current runner autowire the underlying legacy target without exposing the legacy API to current code. ########## grails-test-examples/legacy-g7-command-plugin/build.gradle: ########## @@ -0,0 +1,56 @@ +/* + * 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. + */ + +// Standalone composite-build fixture compiled against published Grails 7.0.10 / Groovy 4.0.30. +// Included by the monorepo root so grails-test-examples-legacy-commands can consume a real +// precompiled Grails 7 ApplicationCommand binary instead of recompiling sources under Grails 8. +plugins { + id 'java-library' + id 'groovy' +} + +group = 'legacy.g7.commands' +version = '0.0.1' + +java { + toolchain { + languageVersion = JavaLanguageVersion.of(17) + } +} + +repositories { + mavenCentral() +} + +dependencies { + implementation platform('org.apache.grails:grails-bom:7.0.10') Review Comment: All four tightenings are in `75ed39351b`: the fixture enforces the Grails `7.0.14` BOM, derives Grails/Groovy manifest stamps from resolved dependencies, and the integration spec asserts them. A second genuinely precompiled `GrailsApplicationCommand` exercises Groovy 4 trait forwarders through `file(...)`, trait metadata, and `applicationContext`, producing `G7-CONTEXT-true`; the plain command also covers additional contract members. The standalone artifact reports Grails 7.0.14 and Groovy 4.0.32. ########## grails-core-cli-legacy/src/main/groovy/org/apache/grails/core/cli/compat/LegacyApplicationCommandProvider.groovy: ########## @@ -0,0 +1,88 @@ +/* + * 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.core.cli.compat + +import java.lang.reflect.InvocationTargetException + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j + +import grails.dev.commands.ApplicationCommand as LegacyApplicationCommand +import org.apache.grails.core.cli.ApplicationCommand +import org.apache.grails.core.cli.ApplicationCommandProvider +import org.apache.grails.core.cli.ApplicationCommandRegistrar +import org.grails.core.io.support.GrailsFactoriesLoader +import org.grails.io.support.FactoriesLoaderSupport + +/** + * Loads commands implemented against the deprecated Grails 7 command contract. + */ +@Slf4j +@CompileStatic +class LegacyApplicationCommandProvider implements ApplicationCommandProvider { + + private boolean warningLogged + + @Override + @SuppressWarnings('deprecation') + void contributeCommands( + ClassLoader registryClassLoader, + ClassLoader contextClassLoader, + ApplicationCommandRegistrar registrar) { + Set<Class<? extends LegacyApplicationCommand>> legacyClasses = new LinkedHashSet<>() + legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses( + LegacyApplicationCommand, registryClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION)) + if (contextClassLoader != null && contextClassLoader != registryClassLoader) { + legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses( + LegacyApplicationCommand, contextClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION)) + } + + for (Class<? extends LegacyApplicationCommand> legacyClass : legacyClasses) { + try { + LegacyApplicationCommand legacyCommand = instantiate(legacyClass) + ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand) + String installedName = registrar.register(command) + if (installedName != null && !warningLogged) { + log.warn('Command \'{}\' from a Grails 7 plugin was loaded through the deprecated grails.dev.commands compatibility layer. Ask the plugin author to migrate to the org.apache.grails.core.cli command API and publish a -cli companion artifact; this compatibility path will be removed in a future major release.', installedName) + warningLogged = true + } + } + catch (LinkageError e) { Review Comment: Addressed all three points in `75ed39351b`. Direct and wrapped `VirtualMachineError`/`ThreadDeath` now propagate through modern commands, providers, legacy commands, and the shell boundary. Load-time and instantiation-time linkage errors identify the command/provider plus resource origin or code-source location, use the Grails binary-compatibility/report-to-framework wording, and continue loading remaining entries. Public-path tests cover each boundary. ########## grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/commands/GrailsCliGradlePlugin.groovy: ########## @@ -307,6 +380,10 @@ class GrailsCliGradlePlugin implements Plugin<Project> { if (grails.cliAutoProvision.get()) { commandClassNames.addAll(loadCommandNamesFromCliClasspath(project)) } + // Legacy Grails 7 application commands intentionally do not get named Gradle tasks. Review Comment: Thanks for confirming. The consistently absent named task and always-present `runCommand` contract remains unchanged in `75ed39351b`. ########## grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/commands/GrailsCliGradlePlugin.groovy: ########## @@ -254,7 +296,38 @@ class GrailsCliGradlePlugin implements Plugin<Project> { GrailsCliArtifactGradlePlugin.CLI_ARTIFACT_MANIFEST_ATTRIBUTE, advertised, project.name) return null } - project.dependencies.create(advertised) + // Manifests advertise group:artifact only. Prefer the producer module's own version so + // third-party plugins (versioned independently of Grails) resolve their matching companion. + // Fall back to the current Grails version for framework modules / unversioned components. + String companionVersion = resolveAdvertisedCompanionVersion(artifact, advertised, project) + project.dependencies.create("${advertised}:${companionVersion}") + } + + protected static String resolveAdvertisedCompanionVersion( + ResolvedArtifactResult artifact, String advertised, Project project) { + def componentIdentifier = artifact.id.componentIdentifier + if (componentIdentifier instanceof ModuleComponentIdentifier) { + String moduleVersion = ((ModuleComponentIdentifier) componentIdentifier).version + if (moduleVersion) { + return moduleVersion + } + } + // Included-build / project components have no module version. The resolved file is usually + // the producer runtime jar (my-plugin-1.2.0-SNAPSHOT.jar), not the companion jar, so peel a + // trailing Maven-style version from whatever jar we have rather than assuming the companion + // artifactId is the filename prefix. + File file = artifact.file + if (file != null && file.name.endsWith('.jar')) { + String baseName = file.name.substring(0, file.name.length() - 4) + // First "-<digit>..." from the left is the version start, so + // my-plugin-1.2.0-rc-1 and my-plugin-cli-1.0.0-SNAPSHOT both work. + for (int i = 0; i < baseName.length() - 1; i++) { + if (baseName.charAt(i) == '-' && Character.isDigit(baseName.charAt(i + 1))) { Review Comment: Implemented option 2 in `75ed39351b`. Included-build companions now use the matching `ResolvedComponentResult.moduleVersion` from Gradle resolution metadata, so there is no ambiguous filename peel. The TestKit composite covers digit-bearing artifact IDs with versions `1`, `1-SNAPSHOT`, and `1-20260101.123456-1`; it also proves Gradle `unspecified` falls back to the Grails version. -- 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]
