jamesfredley commented on code in PR #16011: URL: https://github.com/apache/grails-core/pull/16011#discussion_r3630816726
########## 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: Direct reply to the detect/warn / default-off proposal: Understood. Current code does **not** implement default-off + detect-and-warn. It implements: - `legacyCommandSupport` default **on** - execution-only bridge artifact - modern CLI still controlled by `cliAutoProvision` If we adopt your preferred policy, the intended behavior would be: 1. default `legacyCommandSupport = false` 2. still scan for legacy `META-INF/grails.factories` ApplicationCommand entries 3. emit a one-time loud warning with the exact enable switch when found 4. only provision `grails-core-cli-legacy` after the user opts in That is doable on top of the current split. I have not flipped the default yet because the product call was default-on compatibility after the #15948 break. Happy to flip it if that is the decision. ########## grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/commands/GrailsCliGradlePlugin.groovy: ########## @@ -359,7 +372,63 @@ class GrailsCliGradlePlugin implements Plugin<Project> { } } catch (IOException ignored) { - // unreadable jar — skip + // unreadable jar - skip + } + } + names + } + + /** + * Loads legacy command class names from the {@code META-INF/grails.factories} files of the + * resolved {@code runtimeClasspath} jars, registered under the deprecated + * {@code grails.dev.commands.ApplicationCommand} key. Unchanged Grails 7 command plugins ship + * their commands (and this registration) in their normal runtime jar, so this backwards-compat + * scan registers their per-command Gradle tasks without requiring the plugin to be re-released + * or split into a {@code -cli} companion. Resolution is lenient; unreadable or unbuilt jars are + * skipped (the generic {@code runCommand} task can always execute those commands regardless). + */ + @CompileDynamic + protected Collection<String> loadLegacyCommandNamesFromRuntimeClasspath(Project project) { + Set<String> names = new LinkedHashSet<String>() + Configuration runtimeClasspath = project.configurations.findByName('runtimeClasspath') + if (runtimeClasspath == null) { + return names + } + // Resolving runtimeClasspath at configuration time can race with the configuration of + // sibling source projects in a large multi-project build ("components not calculated yet"). + // A real application resolves this against the module cache without that race and still + // gets its legacy per-command tasks; degrade gracefully (the generic runCommand task can + // always execute the command) rather than failing the whole build if resolution is not yet + // possible - matching the lenient, skip-on-failure handling used for the cli classpath. + Collection<File> files + try { + files = runtimeClasspath.incoming.artifactView { it.lenient(true) }.files.files Review Comment: Clarification so this thread is not oversold: `loadLegacyCommandNamesFromRuntimeClasspath` is still present and still runs when both `cliAutoProvision` and `legacyCommandSupport` are true. So the nondeterministic named-task path is not fully deleted yet. What is true now: - it is gated by the new legacy flag - the intended durable user surface for unmigrated G7 application commands is `runCommand` / shell routing through `runCommand` - modern companion commands still get named tasks from the CLI classpath path Removing the legacy runtimeClasspath named-task scan entirely (so legacy only ever goes through `runCommand`) is still the cleaner end state and is still open work on this PR if we want that closed before merge. -- 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]
