You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/07/19 10:36:36 UTC
[PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
jamesfredley opened a new pull request, #16011:
URL: https://github.com/apache/grails-core/pull/16011
## Summary
[#15948](https://github.com/apache/grails-core/pull/15948) split the CLI command tier off the application runtime classpath. As part of that split the command contract moved from `grails.dev.commands.*` to `org.apache.grails.core.cli.*` (with no forwarding alias) and command registration moved from `META-INF/grails.factories` (key `grails.dev.commands.ApplicationCommand`) to `META-INF/grails-cli.factories`.
The classpath split is good and is preserved here. The side effect is that an **unchanged, already‑published Grails 7 command plugin silently loses all of its application commands on Grails 8**: the command classes no longer link (their `grails.dev.commands.*` supertypes are gone) and their factory entries are never read. Every plugin that ships an application command would otherwise have to be re‑released just to work on Grails 8, and Grails 8 is arriving late enough that the plugin ecosystem will not have caught up when users start upgrading.
This PR adds a **deprecated backwards‑compatibility layer**, entirely inside the `grails-core-cli` tier (so it stays off the application runtime classpath and the split is preserved), so unchanged Grails 7 command plugins keep working **without a re‑release**, while nudging authors to migrate.
## The full picture: every way a plugin provides a command, and what happens on Grails 8
There are several distinct mechanisms for authoring/shipping a command in a Grails plugin. This PR audited all of them so nothing is silently left broken:
| # | Mechanism (how authored) | Packaged as | Discovered by | Broken by #15948? | This PR |
|---|---|---|---|---|---|
| 1 | `create-command` → `grails.dev.commands.ApplicationCommand` / `GrailsApplicationCommand` (runs with an application context) | class + `META-INF/grails.factories` (key `grails.dev.commands.ApplicationCommand`) | `ApplicationContextCommandRegistry` (now reads `grails-cli.factories` / new key) | **Yes — dead** (types removed, key + file changed, no alias) | **Fixed** — restored deprecated contract, adapter, registry dual‑load, runner unwrap, legacy per‑command Gradle tasks |
| 2 | `create-script` → codegen script in `src/main/scripts/*.groovy` (runs without an app context) | `META-INF/commands/*.groovy` | `GroovyScriptCommandFactory` (compiled from source at runtime vs `GroovyScriptCommand`) | No (packaging + resolver unchanged) — **but** a separate Groovy 5 name regression broke hyphenated scripts | **Fixed** the hyphenated‑name regression (see below) |
| 3 | YAML / multi‑step commands | `META-INF/commands/*.yml` | `YamlCommandFactory` | No | Unaffected — verified, documented |
| 4 | `ServiceLoader` `CommandFactory` / profile commands | `META-INF/services` / profile jar | `ServiceCommandFactory` / profile mechanism | No | Unaffected — verified, documented |
| 5 | Legacy Gant `scripts/*.groovy` (pre‑Grails 3) | — | removed in Grails 3 | Already gone pre‑8 | Out of scope — noted |
So the only mechanism actually broken by the CLI split is **#1 (application commands)**; that is the bulk of this PR. **#2 (codegen scripts)** are not broken by the split, but testing them surfaced a separate, real Grails 8 regression (below), which is also fixed here because it otherwise breaks every hyphenated `*-quickstart`‑style script command without a re‑release.
## What this PR does
**Application‑command compatibility (mechanism #1):**
- **Restore** the deprecated `grails.dev.commands.*` command contract (`ApplicationCommand`, `GrailsApplicationCommand`, `ExecutionContext`, and the `io` / `template` helpers) into the `grails-core-cli` tier so pre‑compiled Grails 7 command classes link again. The restored public signatures were confirmed against Grails 7.1.1 bytecode.
- **Adapt** legacy commands to the new contract via `LegacyApplicationCommandAdapter` / `LegacyApplicationCommandAware`, and discover them from the legacy `META-INF/grails.factories` key in `ApplicationContextCommandRegistry`. Each legacy command is instantiated **in isolation**, so one faulty command cannot suppress the rest; a real new‑contract command always wins a name clash. A one‑time deprecation warning nudges migration.
- **Unwrap** the adapter in `GrailsApplicationContextCommandRunner` so Spring autowiring and the `skipBootstrap` lookup target the real legacy command that is subsequently executed.
- **Register per‑command Gradle tasks** for legacy commands found on the runtime classpath (so `grails <command>`, which routes through a Gradle task, keeps working), degrading gracefully when the classpath cannot be resolved at configuration time (the generic `runCommand` task always works).
**Codegen‑script fix (mechanism #2):** a hyphenated script filename (e.g. `audit-quickstart`) produced a class name without the hyphen under Groovy 5, so the command resolved under the wrong name (`auditquickstart`). `GroovyScriptCommandFactory` now uses the filename‑derived command name (a no‑op for non‑hyphenated commands). This restores `audit-quickstart`, `s2-quickstart`, `dbm-*`, `create-job`, etc.
## Testing
- **Unit:** `LegacyApplicationCommandAdapterSpec`, `LegacyCommandRegistryLoadingSpec` (incl. an exception‑isolation regression where one throwing legacy command does not suppress valid legacy/new commands), `GrailsApplicationContextCommandRunnerSpec` (adapter unwrap), `LegacyPluginScriptCompatSpec` (hyphenated‑script name resolution).
- **End‑to‑end functional example** under `grails-test-examples/legacy-commands` (+ `-plugin`): a plugin that ships a legacy `ApplicationCommand`, a legacy `GrailsApplicationCommand`, and a hyphenated codegen script; an `@Integration` spec boots a real Grails 8 application and executes them through the registry. Verified running (`Grails: 8.0.0-SNAPSHOT | Groovy: 5.0.7 | Spring Boot: 4.1.0`).
- `codeStyle` passes on the changed modules.
## Migration path
The compatibility layer is `@Deprecated` and logs a one‑time warning. Plugin authors should migrate to the `org.apache.grails.core.cli.*` command API and publish a `-cli` companion artifact for full support (classpath split, first‑class per‑command task discovery). The layer is intended to be removed in a future major release.
## Follow‑up (minor, non‑blocking)
Reviewers flagged one minor gap: there is no dedicated TestKit test asserting the Gradle legacy per‑command task registration in isolation (its behavior is covered by the runtime functional test and confirmed via a task dry‑run). A focused TestKit fixture could be added later.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5121493887
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails-Core \(macos-latest, 21\)](https://github.com/apache/grails-core/actions/runs/30475522646/job/90656157528?pr=16011) > :grails-core:test
| Test | Runs | Flakiness |
|---|---|--:|
| GrailsUtilStackFiltererSpec > installed DefaultStackTraceFilterer emits Full Stack Trace by default | ❌ | 0% 🟢 |
🏷️ Commit: b5d87ae5ea51b985b7c02eb24429c14728be7b5f
▶️ Tests: 1001 executed
🟡 Checks: 5/52 completed
### Test Failures
<details>
<summary><strong>GrailsUtilStackFiltererSpec > installed DefaultStackTraceFilterer emits Full Stack Trace by default</strong> (:grails-core:test in <a href="https://github.com/apache/grails-core/actions/runs/30475522646/job/90656157528?pr=16011">CI / Build Grails-Core (macos-latest, 21)</a>)</summary>
```
Condition not satisfied:
baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
| | | | |
| "" false | Full Stack Trace:
| interface org.grails.exceptions.reporting.StackTraceFilterer
java.io.ByteArrayOutputStream@28e86e34
at grails.util.GrailsUtilStackFiltererSpec.installed DefaultStackTraceFilterer emits Full Stack Trace by default(GrailsUtilStackFiltererSpec.groovy:143)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 GrailsUtilStackFiltererSpec > installed DefaultStackTraceFilterer emits Full Stack Trace by default <!-- uniqueId=[engine:spock]/[spec:grails.util.GrailsUtilStackFiltererSpec]/[feature:$spock_feature_0_6] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5112419840
I think AI double posted my responses, but https://github.com/apache/grails-core/pull/16059 fixes all of my issues with this branch. If we merge that into this, we should merge this change.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610389201
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -46,6 +53,37 @@ class ApplicationContextCommandRegistry {
commands[cmd.name] = cmd
}
}
+
+ loadLegacyCommands(ApplicationContextCommandRegistry.classLoader)
+ loadLegacyCommands(Thread.currentThread().contextClassLoader)
Review Comment:
`loadLegacyCommands` is invoked for both `ApplicationContextCommandRegistry.classLoader` and the thread context classloader unconditionally. If those are the same (or both can see the same legacy command), this can instantiate the same legacy command twice (constructors may have side effects) even though the second instance will be discarded due to the name collision check. Consider only calling the second load when the classloader differs to avoid duplicate instantiation.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -46,6 +53,37 @@ class ApplicationContextCommandRegistry {
commands[cmd.name] = cmd
}
}
+
+ loadLegacyCommands(ApplicationContextCommandRegistry.classLoader)
+ loadLegacyCommands(Thread.currentThread().contextClassLoader)
+ }
+
+ @SuppressWarnings('deprecation')
+ private void loadLegacyCommands(ClassLoader classLoader) {
+ // Instantiate each legacy command in isolation: a single stale Grails 7 command whose
+ // no-arg constructor (or getName()) throws under Grails 8 must be skipped with a warning,
+ // never abort the whole registry and take valid legacy and new-contract commands down with it.
+ List<Class<grails.dev.commands.ApplicationCommand>> legacyClasses = GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, classLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION)
+ for (Class<grails.dev.commands.ApplicationCommand> legacyClass : legacyClasses) {
+ try {
+ grails.dev.commands.ApplicationCommand legacyCommand = legacyClass.getDeclaredConstructor().newInstance()
+ ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand)
+ String name = command.name
+ if (commands.containsKey(name)) {
+ continue
+ }
+ commands[name] = command
+ if (!legacyCommandWarningLogged) {
+ 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.", name)
+ legacyCommandWarningLogged = true
+ }
+ }
+ catch (Throwable e) {
+ LOG.warn("Failed to load a Grails 7 legacy command from class '{}' through the deprecated grails.dev.commands compatibility layer; skipping it. Cause: {}",
+ legacyClass?.name, e.message)
+ }
Review Comment:
When a legacy command fails to load, the warning log omits the exception itself, so there’s no stack trace to diagnose the root cause (only `e.message`). Passing the throwable as the final argument lets SLF4J include the stack trace while keeping the warning message the same.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610902214
##########
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:
Making a single flag to enable legacy compatibility is exactly how we have handed other major release changes and settings. We changed the bom handling for example in this release. Why is this suddenly different? I disagree with the default setting - not even Spring Boot takes this path (jackson 2 vs 3 as an example)
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5062092534
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI - Groovy Joint Validation Build / build\_grails](https://github.com/apache/grails-core/actions/runs/30027257102/job/89279064161?pr=16011) > :grails-test-examples-spring-security-ui-extended:integrationTest
| Test | Runs | Flakiness |
|---|---|--:|
| ExtendedSecurityInfoSpec > testConfig | ❌ | [1% 🟡](https://github.com/apache/grails-core/issues/16030) |
🏷️ Commit: 75ed39351bfe4fb43fcce2d60e75aac50a883833
▶️ Tests: 64951 executed
🟡 Checks: 54/56 completed
### Test Failures
<details>
<summary><strong>ExtendedSecurityInfoSpec > testConfig</strong> (:grails-test-examples-spring-security-ui-extended:integrationTest in <a href="https://github.com/apache/grails-core/actions/runs/30027257102/job/89279064161?pr=16011">CI - Groovy Joint Validation Build / build_grails</a>)</summary>
```
Condition not satisfied:
contains('adh.ajaxErrorPage')
|
false
at extended.spec.ExtendedSecurityInfoSpec.testConfig_closure1(ExtendedSecurityInfoSpec.groovy:39)
at spock.lang.Specification.with(Specification.java:208)
at extended.spec.ExtendedSecurityInfoSpec.testConfig(ExtendedSecurityInfoSpec.groovy:38)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 ExtendedSecurityInfoSpec > testConfig <!-- uniqueId=[engine:spock]/[spec:extended.spec.ExtendedSecurityInfoSpec]/[feature:$spock_feature_2_0] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610984575
##########
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:
"The non-legacy path resolves at configuration time too" doesn't carry the weight this deferral puts on it. The cli path's cost is a pre-existing issue; what this PR adds is new *nondeterminism*, and the deferral means shipping it knowingly.
The pattern being merged here — eager cross-project classpath resolution in `afterEvaluate`, wrapped in `catch (Throwable)` with an info-level log as the only signal — is precisely the class of Gradle logic I spent Grails 7 removing. Gradle is a highly parallel system: project configuration order is not guaranteed, parallel configuration actively exploits that freedom, and resolution that reaches across project boundaries at configuration time is the canonical way to get behavior that depends on scheduling. A `catch (Throwable)` around it doesn't handle the failure — it converts a scheduling accident into a silently different **task graph**. Whether `helloLegacyApp` exists becomes a function of which sibling projects happened to configure first; the same command line succeeds on one machine and dies with "task not found" on another. There is no error to act on — the user's first contact with any of this is a missing task, and our first contact is a bug report we cannot rep
roduce.
It also moves us backwards on the roadmap. We don't support the configuration cache yet, but getting there is the goal — and configuration-cache compatibility is fundamentally about eliminating exactly this: order-dependent configuration-time resolution whose outcome can differ run to run. Every instance of this pattern we merge is one more thing that has to be found and unwound before we can turn config-cache support on, and this one is being added with the defect already identified.
These are not hypothetical failure modes. Ordering-dependent `afterEvaluate` wiring and swallowed configuration-time failures are exactly what made the pre-7 Grails Gradle integration fragile enough that I rewrote most of it — the explicit goal of that rewrite, and of this release line, is that Grails builds behave like plain, predictable Gradle builds. Merging a new feature built on the pattern we just spent a major release eliminating walks that back, and it does so in the compatibility layer — the code that will be exercised precisely by users mid-upgrade, who are least equipped to distinguish "Gradle scheduling artifact" from "my plugin is broken on Grails 8."
You offered two options in your reply, and either one resolves this **in this PR**:
1. **Drop the legacy per-command tasks** and document `runCommand -Pargs=...` for legacy plugins. That's a deletion, not new machinery — the generic task already works, it's deterministic, and per-command tasks can arrive later with marker-based discovery done properly across both paths. A consistently-absent task beats a sometimes-there one, which was the original point.
2. Source the legacy names from the same `grailsCli.withDependencies` lenient-view mechanism the companion discovery uses, scoped to the legacy branch only.
Option 1 is my steer — it's smaller than the code being deferred. What I'm not willing to do is merge the eager-resolution + `catch (Throwable)` version with its removal tracked as a follow-up: this PR's follow-ups are already accumulating pieces of the headline claim that the merged code doesn't actually hold, and this one would be merged with the defect already on the record.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
+
private final Map<String, ApplicationCommand> commands = [:]
+ private boolean legacyCommandWarningLogged
ApplicationContextCommandRegistry() {
+ ClassLoader registryClassLoader = ApplicationContextCommandRegistry.classLoader
+ ClassLoader contextClassLoader = Thread.currentThread().contextClassLoader
+
for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- ApplicationContextCommandRegistry.classLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ registryClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
if (!commands.containsKey(cmd.name)) {
commands[cmd.name] = cmd
}
}
- // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is also used to pull any commands that are loaded from the gradle classpath
- for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- Thread.currentThread().contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
- if (!commands.containsKey(cmd.name)) {
- commands[cmd.name] = cmd
+ // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is
+ // also used to pull any commands that are loaded from the gradle classpath. Only when it is a distinct
+ // classloader: repeating the scan for the same classloader would re-instantiate every command (whose
+ // constructor may have side effects) just to discard it on the name-collision check below.
+ if (contextClassLoader != registryClassLoader) {
+ for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
+ contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ if (!commands.containsKey(cmd.name)) {
+ commands[cmd.name] = cmd
+ }
+ }
+ }
+
+ loadLegacyCommands(registryClassLoader, contextClassLoader)
+ }
+
+ @SuppressWarnings('deprecation')
+ private void loadLegacyCommands(ClassLoader registryClassLoader, ClassLoader contextClassLoader) {
+ // Gather the legacy command classes from the registry classloader and, when it is a distinct
+ // classloader, the thread context classloader, de-duplicated by Class identity before any are
+ // instantiated. A child context classloader delegates to its parent, so it also reports the
+ // parent's grails.factories entries; de-duplicating by the resolved Class avoids instantiating a
+ // parent-visible legacy command twice (its constructor may have side effects) only to discard the
+ // duplicate on the name-collision check below.
+ Set<Class<? extends grails.dev.commands.ApplicationCommand>> legacyClasses = new LinkedHashSet<>()
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, registryClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ if (contextClassLoader != null && contextClassLoader != registryClassLoader) {
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, contextClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ }
+ // Instantiate each in isolation: a single stale Grails 7 command whose no-arg constructor (or
+ // getName()) throws under Grails 8 must be skipped with a warning, never abort the whole registry
+ // and take valid legacy and new-contract commands down with it.
+ for (Class<? extends grails.dev.commands.ApplicationCommand> legacyClass : legacyClasses) {
+ try {
+ grails.dev.commands.ApplicationCommand legacyCommand = legacyClass.getDeclaredConstructor().newInstance()
+ ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand)
+ String name = command.name
+ if (commands.containsKey(name)) {
+ continue
+ }
+ commands[name] = command
+ if (!legacyCommandWarningLogged) {
+ 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.", name)
+ legacyCommandWarningLogged = true
+ }
+ }
+ catch (Throwable e) {
Review Comment:
Answering the question you asked — and asking that the answer land in this PR rather than the follow-up list, because it's a few lines, not a design effort:
Split the catch. `LinkageError` (`NoSuchMethodError`, `NoClassDefFoundError`, `IncompatibleClassChangeError`) is not "a faulty command" — it's the compatibility layer itself failing to deliver its contract for a command the upgrade docs promise will work. Catch it separately from `Exception` and log at **error** level with the command class, the originating jar, and explicit wording that this is a Grails 7 binary-compatibility failure that should be reported as a framework bug — while still isolating it so other commands register. Ordinary `Exception` (a command constructor with side effects blowing up) can stay a warning; that genuinely is the plugin's fault and per-command isolation is the right call there. Don't fail the whole run — a broken command in one plugin shouldn't take out `run-app` — but the two failure classes must not share a log line, because one is diagnosable by the user and the other is only diagnosable by us.
The reason this can't wait: until the precompiled Grails 7 fixture exists, a linkage failure in the wild is the *first* signal we'd get that the trait ABI doesn't hold — and in the current code that signal is a generic warning. The distinct-error version is exactly the `catch` block you already have, split in two.
Reopening this thread: the structural half is fixed, but the loud-linkage-failure half is still open, so resolving was premature.
##########
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:
The discoverability argument — "an unmigrated plugin silently contributes no commands until the user finds a switch they don't know exists" — is solvable without defaulting the shim on, and solving it that way is strictly better. Keep the legacy `grails.factories` **detection** always-on but make **provisioning** opt-in: when the registry (or the Gradle plugin) sees a `grails.dev.commands.ApplicationCommand` entry on the classpath and `legacyCommandSupport` is not enabled, it fails or errors loudly with the exact remediation — *"plugin X ships Grails 7 commands; set `grails { legacyCommandSupport = true }` or upgrade the plugin."* Nothing is silent, the user flips one documented switch during the upgrade they are already performing, and nobody who doesn't need the shim carries it.
That matches how we've handled every comparable transition in this release line — the BOM handling change shipped behind an explicit setting this very release, indy before it — and it matches the wider ecosystem: Spring Boot did not keep Jackson 2 on the classpath by default alongside 3; the old line is an explicit dependency you add if you need it. I'm not aware of a precedent for default-provisioning a deprecated contract onto every Grails 8 build, test runtime included.
Default-on also has costs that fall on people who *don't* use legacy plugins: every application gets `grails-core-cli-legacy` and the dual-classloader factories scan on its command and test-runtime classpaths for the lifetime of the window, and — as you noted yourself — when the artifact is eventually dropped, plugins that never migrated break at that point with no action ever having been asked of anyone. Default-on removes the only pressure that makes the deprecation window converge; a visible opt-in is the signal that tells both users and plugin authors migration is actually expected. The one-time runtime warning doesn't do that — plugin authors never see their users' logs.
So: keep everything you built in d1fe4956a6 — the isolation is right — but gate provisioning on an explicit `legacyCommandSupport = true`, with loud, actionable detection when it's off. That keeps zero-*confusion* upgrades without committing the framework to zero-*touch* support of a deprecated ABI by default.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5016608774
@jamesfredley Please stop resolving comments when they're only half completed. It makes extremely hard to understand what's done and what isn't.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3630652190
##########
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:
Update after the latest push (`c798c93778`):
There are now **two** flags, both default **on**:
| Flag | Default | Controls |
|---|---|---|
| `cliAutoProvision` | `true` | Modern CLI tier only: `grails-core-cli`, `grails-console`, discovered companion `-cli` artifacts |
| `legacyCommandSupport` | `true` | Grails 7 application-command bridge only: `grails-core-cli-legacy` |
Rules in the current code:
- Modern CLI auto-provisions when `cliAutoProvision = true`
- Legacy bridge auto-provisions only when **both** `cliAutoProvision = true` **and** `legacyCommandSupport = true`
- `legacyCommandSupport = false` turns off only the Grails 7 bridge; modern companions still auto-provision
- `cliAutoProvision = false` turns off the whole auto-provisioned CLI tier, including the legacy bridge
DSL / properties:
```groovy
grails {
legacyCommandSupport = false // keep modern CLI, disable only G7 bridge
}
// or
grails {
cliAutoProvision = false // disable all CLI auto-provisioning
}
```
Also: `-PgrailsLegacyCommandSupport=false` / `-PgrailsCliAutoProvision=false`
On the ABI point from this thread: that half is already in place. Deprecated `grails.dev.commands.*` lives only in `grails-core-cli-legacy` (execution-only `grailsCliLegacy` bucket). `grails-core-cli` has no compile ABI for the deprecated package.
Default remains **on** for `legacyCommandSupport` so unchanged published G7 command plugins keep working with zero user action. The separate flag is there so people who want modern companions without the bridge can opt out cleanly.
##########
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:
Update after the latest push:
Agreed this was a real problem. Current branch direction for legacy execution is:
- keep the execution-only `grails-core-cli-legacy` bridge
- do **not** rely on configuration-time `runtimeClasspath` scans to invent sometimes-present per-command Gradle tasks for legacy plugins
- run legacy application commands through the generic `runCommand` path (shell adapter routes legacy adapters there)
So the nondeterministic "task exists only if sibling projects configured first" path is no longer the intended user surface for legacy commands. Modern companion commands still get named tasks from the CLI classpath discovery path.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
+
private final Map<String, ApplicationCommand> commands = [:]
+ private boolean legacyCommandWarningLogged
ApplicationContextCommandRegistry() {
+ ClassLoader registryClassLoader = ApplicationContextCommandRegistry.classLoader
+ ClassLoader contextClassLoader = Thread.currentThread().contextClassLoader
+
for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- ApplicationContextCommandRegistry.classLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ registryClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
if (!commands.containsKey(cmd.name)) {
commands[cmd.name] = cmd
}
}
- // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is also used to pull any commands that are loaded from the gradle classpath
- for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- Thread.currentThread().contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
- if (!commands.containsKey(cmd.name)) {
- commands[cmd.name] = cmd
+ // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is
+ // also used to pull any commands that are loaded from the gradle classpath. Only when it is a distinct
+ // classloader: repeating the scan for the same classloader would re-instantiate every command (whose
+ // constructor may have side effects) just to discard it on the name-collision check below.
+ if (contextClassLoader != registryClassLoader) {
+ for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
+ contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ if (!commands.containsKey(cmd.name)) {
+ commands[cmd.name] = cmd
+ }
+ }
+ }
+
+ loadLegacyCommands(registryClassLoader, contextClassLoader)
+ }
+
+ @SuppressWarnings('deprecation')
+ private void loadLegacyCommands(ClassLoader registryClassLoader, ClassLoader contextClassLoader) {
+ // Gather the legacy command classes from the registry classloader and, when it is a distinct
+ // classloader, the thread context classloader, de-duplicated by Class identity before any are
+ // instantiated. A child context classloader delegates to its parent, so it also reports the
+ // parent's grails.factories entries; de-duplicating by the resolved Class avoids instantiating a
+ // parent-visible legacy command twice (its constructor may have side effects) only to discard the
+ // duplicate on the name-collision check below.
+ Set<Class<? extends grails.dev.commands.ApplicationCommand>> legacyClasses = new LinkedHashSet<>()
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, registryClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ if (contextClassLoader != null && contextClassLoader != registryClassLoader) {
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, contextClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ }
+ // Instantiate each in isolation: a single stale Grails 7 command whose no-arg constructor (or
+ // getName()) throws under Grails 8 must be skipped with a warning, never abort the whole registry
+ // and take valid legacy and new-contract commands down with it.
+ for (Class<? extends grails.dev.commands.ApplicationCommand> legacyClass : legacyClasses) {
+ try {
+ grails.dev.commands.ApplicationCommand legacyCommand = legacyClass.getDeclaredConstructor().newInstance()
+ ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand)
+ String name = command.name
+ if (commands.containsKey(name)) {
+ continue
+ }
+ commands[name] = command
+ if (!legacyCommandWarningLogged) {
+ 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.", name)
+ legacyCommandWarningLogged = true
+ }
+ }
+ catch (Throwable e) {
Review Comment:
Update after the latest push:
The catch is split as requested:
- `LinkageError` (including unwrapped constructor linkage failures): logged at **error** with class + origin, isolated so siblings still load
- ordinary `Exception`: warning + skip
- `VirtualMachineError` / `ThreadDeath`: rethrown
- registry provider boundary also rethrows VM/ThreadDeath before its normal provider failure catch
So a binary-incompatible G7 command is loud and named, not a generic swallowed warning, without taking down the whole command set.
##########
grails-test-examples/legacy-commands-plugin/build.gradle:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+plugins {
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.dependency-validator'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-plugin'
+}
+
+version = '0.0.1'
+group = 'legacy.commands.plugin'
+
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ // This fixture recompiles legacy command sources against Grails 8's grails-core-cli to
+ // validate discovery, adapter, registry, and runner wiring end-to-end. It does not
+ // re-validate a pre-compiled Grails 7 binary's Groovy-trait ABI. That relies on Groovy's
+ // stable trait encoding across 4->5 and could be strengthened later with a prebuilt Grails 7
+ // fixture jar.
+ compileOnly 'org.apache.grails:grails-core-cli'
Review Comment:
Update / status on the precompiled G7 fixture:
Agreed the composite-build approach is the right way to prove real Groovy 4 trait-consumer bytecode. That is still the plan for proving the headline binary-compat claim end-to-end.
What is on the branch now is the isolation + flag + default-on bridge wiring. The dedicated Grails 7 / Groovy 4 included-build fixture pair remains the outstanding proof item called out in the PR body, not something I am claiming is already closed by the recompiled in-repo example alone.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5052666363
Pushed `116b0efab3`: flipped `legacyCommandSupport` default from **true → false** everywhere.
**Code**
- `GrailsExtension.legacyCommandSupport` convention is now `Boolean.FALSE` when `grailsLegacyCommandSupport` is unset
- Javadoc / plugin property docs describe opt-in (`= true`), not opt-out
**Docs**
- Upgrade guide §35 / §35.1 and What's New: default `false`, enable examples, consumer checklist
**Tests / fixtures**
- `CliAutoDiscoverySpec`: default-false, explicit false, explicit true, and `cliAutoProvision=false` still wins when legacy is opted in
- `grails-test-examples/legacy-commands` opts in with `grails { legacyCommandSupport = true }`
**PR description** updated to match (default-off bridge, opt-in migration note).
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5015790977
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails Forge \(Java 21, indy\=true\)](https://github.com/apache/grails-core/actions/runs/29687031989/job/88193053305?pr=16011) > :test-core:test [grails-forge]
| Test | Runs | Flakiness |
|---|---|--:|
| CreateRestApiSpec > test rest-api app with default json-views | ❌ | 0% 🟢 |
🏷️ Commit: 5069bef718a2c636038e3d2735eb12c5552f243c
▶️ Tests: 19288 executed
🟡 Checks: 25/56 completed
### Test Failures
<details>
<summary><strong>CreateRestApiSpec > test rest-api app with default json-views</strong> (:test-core:test [grails-forge] in <a href="https://github.com/apache/grails-core/actions/runs/29687031989/job/88193053305?pr=16011">CI / Build Grails Forge (Java 21, indy=true)</a>)</summary>
```
org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /tmp/testrestapi4155656592737981264 with arguments [build]
Output:
> Task :compileJava NO-SOURCE
> Task :compileGroovy FAILED
[Incubating] Problems report is available at: file:///tmp/testrestapi4155656592737981264/build/reports/problems/problems-report.html
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':compileGroovy' (registered by plugin class 'org.gradle.api.plugins.GroovyBasePlugin').
> Could not resolve all files for configuration ':compileClasspath'.
> Could not find org.apache.grails:grails-data-hibernate5-cli:.
Required by:
root project 'testrestapi4155656592737981264'
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights from a Build Scan (powered by Develocity).
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/9.6.0/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD FAILED in 57s
1 actionable task: 1 executed
at org.gradle.testkit.runner.internal.DefaultGradleRunner.lambda$build$2(DefaultGradleRunner.java:275)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.run(DefaultGradleRunner.java:374)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.build(DefaultGradleRunner.java:273)
at org.grails.forge.utils.CommandSpec.executeGradle(CommandSpec.groovy:79)
at org.grails.forge.create.CreateRestApiSpec.test rest-api app with default json-views(CreateRestApiSpec.groovy:30)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 CreateRestApiSpec > test rest-api app with default json-views <!-- uniqueId=[engine:spock]/[spec:org.grails.forge.create.CreateRestApiSpec]/[feature:$spock_feature_1_0] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley merged PR #16011:
URL: https://github.com/apache/grails-core/pull/16011
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5016441305
@jamesfredley I strongly disagree with the legacy compatibility by default. A single setting can be changed as part of the upgrade if people use legacy plugins - this matches previous decisions like indy, and many more. Making this a default incentivizes people to not adopt the new api and when we remove it, it will break outside of a major release lifecycle.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3634939275
##########
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:
Confirmed the `loadLegacyCommandNamesFromRuntimeClasspath` scan is fully deleted and `LegacyCommandTaskDiscoverySpec` asserts both halves of the contract (`LEGACY_COMMAND_TASK_PRESENT=false`, `RUN_COMMAND_TASK_PRESENT=true`). This resolves the nondeterministic-task-graph concern exactly the way I'd hoped — a consistently-absent task plus the always-present `runCommand`. Thanks for taking option 1 rather than deferring it.
##########
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:
What prevents the legacy cli from inheriting the cli commands? Wouldn't then all of the current code then be able to reference the new command class?
##########
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:
On the default-on vs. opt-in question: I've moved that to the dev list so it's decided by the project deliberately rather than in this PR — let's keep this thread to mechanics. On the mechanics, one important sizing correction to "the flip is one convention change": my proposal is default-off **plus always-on detection that fails loudly**, and the detection half doesn't exist yet in any form — under `legacyCommandSupport = false` today, an unmigrated plugin's commands vanish silently, which is precisely the failure mode you (rightly) objected to.
And the detection must be designed carefully or it reintroduces the problem we just removed: a configuration-time `runtimeClasspath` scan for legacy `grails.factories` entries would be the same eager cross-project resolution the named-task scan had. The detection belongs on the **runtime side**, where the classpath is already resolved and the check is deterministic:
1. **In `ApplicationContextCommandRegistry` (neutral tier, no `grails.dev.commands` reference needed):** after provider loading, enumerate `META-INF/grails.factories` resources on the registry/context classloaders and check for the `grails.dev.commands.ApplicationCommand` key — a cheap properties read, no class loading, no linkage risk. If entries exist and no legacy provider contributed (i.e. `grails-core-cli-legacy` absent), log a single **error** naming each plugin jar and the exact remediation: `Plugin <jar> ships Grails 7 commands; set grails { legacyCommandSupport = true } or upgrade the plugin.`
2. **In the unknown-command path:** when `runCommand`/the shell adapter fails to find a command name and legacy factories were detected, append the same hint — that's the moment the user is actually staring at the failure.
3. **Nothing at Gradle configuration time.** If we want a build-side signal at all, it goes in the runner task's execution action, never configuration.
That design keeps zero-*confusion* regardless of which default the list lands on — and it's worth building even if the decision is default-on, since it also covers the `legacyCommandSupport = false` opt-out case. I'm happy to contribute this piece onto your branch once the list settles the default, so it doesn't block anything else here.
##########
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:
The left-to-right "first `-<digit>` starts the version" heuristic misfires on artifact names that legitimately contain a digit after a hyphen: `my-plugin-2fa-1.0.0.jar` yields version `2fa-1.0.0`, and something like `grails-oauth2-provider` style names are only safe by luck of where the digit falls. Since this branch is already the fallback for included-build/project components, two hardening options:
1. Scan from the **right**: take the last `-` whose following segment parses as a plausible version (`\d+(\.\d+)*([.-].+)?`), which handles `my-plugin-2fa-1.0.0` and `my-plugin-cli-1.0.0-SNAPSHOT` correctly; or
2. For `ProjectComponentIdentifier` cases, read the producer project's version through the artifact's variant/module metadata rather than the filename at all, and keep the filename peel only as a last resort.
Either way, a unit test with a digit-bearing plugin name would lock the behavior in.
##########
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:
The `LinkageError` split is in and structured the way I asked — thanks. Three follow-ons on this exact block:
1. **The thread description says `VirtualMachineError`/`ThreadDeath` are rethrown, but I can't find that on the tip** — this `catch (Throwable)` at line 70 and the provider-boundary `catch (Throwable)` in `ApplicationContextCommandRegistry.loadCommandProviders` both swallow everything that isn't a `LinkageError`, so an OOM during command loading degrades to a warning. If the rethrow guard was dropped in the style pass, please restore it (`if (e instanceof VirtualMachineError) throw e` before the generic handling, in both places); if it was never pushed, the same fix applies.
2. **Name the originating jar in the error.** `legacyClass.protectionDomain?.codeSource?.location` is right there and turns "which of my 12 plugins is broken?" into a one-line answer for the user reporting it.
3. **Reword the error away from plugin blame.** Under the compatibility contract this PR documents, a `LinkageError` against the restored traits is the *bridge* failing to deliver — the plugin binary was valid on Grails 7 and the docs promise it keeps working. The message should say this is a Grails binary-compatibility issue and ask the user to report it to the framework (with class + jar), rather than "the plugin is likely binary-incompatible... must be recompiled", which sends the user to the wrong maintainer for what is our bug to fix.
##########
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:
This is a genuine precompiled fixture and I'm glad it landed in the PR — and I accept the `GradleBuild` rationale over `includeBuild`: composite auto-substitution would indeed rewrite `org.apache.grails:grails-core:7.0.10` to the local Groovy 5 project, which is exactly what the fixture must prevent. Four tightenings to make it prove what the docs will claim:
1. **`platform` → `enforcedPlatform`, and pin `7.0.14`.** With a plain `platform`, the BOM's Groovy 4 constraint is only a preference — anything else on the fixture's classpath could win an upgrade and the fixture would silently compile under a different Groovy than advertised. `enforcedPlatform('org.apache.grails:grails-bom:7.0.14')` makes drift impossible, and 7.0.14 is the current 7.0.x — the fixture should represent what plugins in the wild were most recently built against.
2. **Derive the manifest stamps instead of hand-typing them.** `Grails-Compile-Version: 7.0.10` / `Groovy-Compile-Version: 4.0.30` are string literals that nothing checks; if the BOM pin changes, they lie. Resolve them at jar time from the compile classpath (the resolved `grails-core` and `org.apache.groovy:groovy` module versions) so the manifest is evidence, not assertion — and have the integration spec assert the attributes, not just the jar name.
3. **Cover `GrailsApplicationCommand` with a second precompiled command.** The current fixture only implements `ApplicationCommand`, but the riskiest ABI surface — the one that motivated this fixture — is `GrailsApplicationCommand`: its `@Delegate`-generated forwarders to `TemplateRenderer`/`FileSystemInteraction` and the `$Trait$FieldHelper` accessors that the Groovy 4 compiler baked into the consumer class. A second command in this same fixture whose `handle()` actually calls `templateRenderer`/`file(...)`/render-style DSL and reads `applicationContext` from inside the command would exercise the forwarder bytecode paths. Right now that trait is only proven by the recompiled in-repo plugin, which is the gap this fixture exists to close.
4. Minor: `handle()` here only touches `executionContext.baseDir` — even for the plain `ApplicationCommand` case, reading a couple more contract members from inside the precompiled code (e.g. `name`/`description` via the trait, the `applicationContext` getter) widens the linked surface for free.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5046901726
On the Forge failures (`Could not find org.apache.grails:grails-scaffolding-cli:.` / `grails-data-hibernate5-cli:.` with empty versions):
That was auto-provision creating companion coordinates without a version. The latest push pins framework CLI artifacts (`grails-core-cli`, `grails-console`, `grails-core-cli-legacy`) to the current Grails version, and derives external companion versions from the producer module/jar instead of leaving them versionless.
Watching the post-`c798c93778` CI run for Forge. If anything still fails there I will chase it separately from the default-on policy discussion.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5016417922
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails Forge \(Java 21, indy\=false\)](https://github.com/apache/grails-core/actions/runs/29693475129/job/88210146422?pr=16011) > :test-core:test [grails-forge]
| Test | Runs | Flakiness |
|---|---|--:|
| CreateRestApiSpec > test rest-api app with default json-views | ❌ | 0% 🟢 |
🏷️ Commit: d1fe4956a6d59ed6ac68a71bafe2020d813b11f6
▶️ Tests: 15735 executed
🟡 Checks: 23/55 completed
### Test Failures
<details>
<summary><strong>CreateRestApiSpec > test rest-api app with default json-views</strong> (:test-core:test [grails-forge] in <a href="https://github.com/apache/grails-core/actions/runs/29693475129/job/88210146422?pr=16011">CI / Build Grails Forge (Java 21, indy=false)</a>)</summary>
```
org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /tmp/testrestapi7950302997098314377 with arguments [build]
Output:
> Task :compileJava NO-SOURCE
> Task :compileGroovy FAILED
[Incubating] Problems report is available at: file:///tmp/testrestapi7950302997098314377/build/reports/problems/problems-report.html
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':compileGroovy' (registered by plugin class 'org.gradle.api.plugins.GroovyBasePlugin').
> Could not resolve all files for configuration ':compileClasspath'.
> Could not find org.apache.grails:grails-data-hibernate5-cli:.
Required by:
root project 'testrestapi7950302997098314377'
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights from a Build Scan (powered by Develocity).
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/9.6.0/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD FAILED in 41s
1 actionable task: 1 executed
at org.gradle.testkit.runner.internal.DefaultGradleRunner.lambda$build$2(DefaultGradleRunner.java:275)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.run(DefaultGradleRunner.java:374)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.build(DefaultGradleRunner.java:273)
at org.grails.forge.utils.CommandSpec.executeGradle(CommandSpec.groovy:79)
at org.grails.forge.create.CreateRestApiSpec.test rest-api app with default json-views(CreateRestApiSpec.groovy:30)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 CreateRestApiSpec > test rest-api app with default json-views <!-- uniqueId=[engine:spock]/[spec:org.grails.forge.create.CreateRestApiSpec]/[feature:$spock_feature_1_0] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5123747768
## ✅ All tests passed ✅
🏷️ Commit: bdb98a663601037ca7fb70a818f3dd29726e1c26
▶️ Tests: 41416 executed
⚪️ Checks: 60/60 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3670045936
##########
grails-core/src/main/groovy/org/grails/core/io/support/GrailsFactoriesLoader.groovy:
##########
@@ -101,6 +101,56 @@ class GrailsFactoriesLoader extends FactoriesLoaderSupport {
results
}
+ /**
+ * Loads factory class names grouped by the resource that declares them without loading the
+ * classes themselves. This lets callers that need diagnostic context apply stricter class
+ * loading semantics than the framework-wide best-effort factory loader.
+ */
+ static Map<String, List<String>> loadFactoryDeclarations(
+ Class<?> factoryClass,
+ ClassLoader classLoader = GrailsFactoriesLoader.classLoader,
+ String resourceLocation = FACTORIES_RESOURCE_LOCATION) {
+ Assert.notNull(factoryClass, "'factoryClass' must not be null")
+ loadFactoryDeclarations(factoryClass.name, classLoader, resourceLocation)
+ }
+
+ static Map<String, List<String>> loadFactoryDeclarations(
+ String factoryClassName,
+ ClassLoader classLoader = GrailsFactoriesLoader.classLoader,
+ String resourceLocation = FACTORIES_RESOURCE_LOCATION) {
+ Assert.hasText(factoryClassName, "'factoryClassName' must not be empty")
+ ClassLoader factoryClassLoader = classLoader ?: GrailsFactoriesLoader.classLoader
+ Map<String, List<String>> declarations = new LinkedHashMap<>()
+ Enumeration<URL> resources = factoryClassLoader.getResources(resourceLocation)
+ while (resources.hasMoreElements()) {
+ URL resource = resources.nextElement()
+ Properties properties = new Properties()
+ try {
+ resource.openStream().withCloseable { InputStream input ->
+ properties.load(input)
+ }
+ }
+ catch (IOException | IllegalArgumentException ignored) {
Review Comment:
`catch (IOException | IllegalArgumentException ignored) { continue }` drops the entire resource with no log line at any level — the class has no logger.
That would be defensible if this only backed the legacy diagnostic, but it doesn't. `loadFactoryDeclarations` is now the reader for modern discovery too: `ApplicationContextCommandRegistry:64` for commands and `:159` for providers, both against `CLI_FACTORIES_RESOURCE_LOCATION`. So a single plugin shipping a `META-INF/grails-cli.factories` with a stray backslash — `Properties.load` throws `IllegalArgumentException("Malformed \\uxxxx encoding")` — loses *every* one of that plugin's Grails 8 commands and providers. The user sees commands that simply aren't there, with nothing to chase.
It's also a loudness regression against what it replaced. `FactoriesLoaderSupport.loadFactoryNames` (`grails-gradle/model/.../FactoriesLoaderSupport.groovy:99-101`) rethrows `IOException` as `IllegalArgumentException` and doesn't catch `IllegalArgumentException` at all, so a malformed factories file propagated instead of vanishing. `ApplicationCommandProviderSpec:301` currently locks the new silence in — it asserts only `missingCommandHint == null` and `noExceptionThrown()`.
This is the same failure mode you and I already agreed on for linkage errors: a command the user expects, silently absent, diagnosable by nobody. Give the class a logger and `warn` with the resource URL and the cause before continuing.
Implemented on [test/8.0.x-legacy-command-e2e](https://github.com/jdaugherty/grails-core/tree/test/8.0.x-legacy-command-e2e): the class gets `@Slf4j` and the catch warns with the resource URL and cause, keeping the per-resource `continue`. The existing malformed-resource test now asserts the warning instead of only `noExceptionThrown()`, and a new one proves a malformed `grails-cli.factories` in one plugin no longer costs a *different* plugin its modern commands. `:grails-core:test --tests ApplicationCommandProviderSpec` green (23 features).
##########
grails-test-examples/legacy-g7-command-plugin/src/main/groovy/legacy/g7/commands/HelloG7PrecompiledCommand.groovy:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 legacy.g7.commands
+
+import groovy.transform.CompileStatic
+
+import grails.dev.commands.ApplicationCommand
+import grails.dev.commands.ExecutionContext
+
+/**
+ * Precompiled against Grails 7 / Groovy 4 so the monorepo can prove that an unchanged
+ * published Grails 7 application-command binary still links and runs through the Grails 8
+ * compatibility bridge.
+ */
+@CompileStatic
+class HelloG7PrecompiledCommand implements ApplicationCommand {
+
+ @Override
+ String getName() {
Review Comment:
Overriding `getName()` here is a legitimate Grails 7 shape and this command should keep it. The gap is that it's the *only* shape covered.
The deprecated trait ships two behaviours. One is "the command declares its own name", which this fixture exercises. The other is "the command declares no name and the trait derives one" — `ApplicationCommand.getName()` at `grails-core-cli-legacy/.../grails/dev/commands/ApplicationCommand.groovy:50-57`, which runs `GrailsNameUtils.getScriptName(getLogicalName(...))` and, from a Groovy 4 binary, reaches it through `ApplicationCommand$Trait$Helper`. That second path has no coverage anywhere in the tree: both fixture commands override the getters (here at :35/:39, `HelloG7PrecompiledGrailsCommand` at :29/:33), `LegacyApplicationCommandAdapterSpec:54-55` overrides them, and so do all eight classes in `LegacyCommandRegistryLoadingSpec` (:332, :354, :373, :395, :415, :440, :460, :484).
It matters because `class FooBarCommand implements ApplicationCommand` with no `getName()` is the shape `create-command` generated, so a large share of the published Grails 7 commands this bridge exists to keep working never declare a name at all. For those the derivation *is* the registration key — if the Groovy 5 recompile of the trait changes how that default is woven or dispatched into Groovy-4-compiled implementors, they register under the wrong key or not at all. That's precisely the class of failure this fixture was built to catch, and it's the one variant it currently can't see.
So: keep this command as-is, and add a third precompiled one alongside it that declares neither getter, letting the trait derive the name. The spec then looks it up under whatever the derivation produces, which only passes if the default survives the version boundary.
A unit test over in `grails-core-cli-legacy` wouldn't substitute for this — that would be a Groovy 5 implementor against a Groovy 5 trait, which isn't the cross-version question. It has to be a Groovy 4 binary to mean anything, which is why it belongs in this fixture.
There's a working version of this on [test/8.0.x-legacy-command-e2e](https://github.com/jdaugherty/grails-core/tree/test/8.0.x-legacy-command-e2e): a third precompiled `HelloDerivedNameCommand` matching the `create-command` template exactly - implements `GrailsApplicationCommand`, declares neither getter - which reads its own `name` inside `handle()` so the derivation is exercised from the Groovy 4 bytecode rather than only from the recompiled trait. It registers as `hello-derived-name` (confirmed by running the trait against the precompiled class), and the integration spec asserts both the registration key and the rendered output. Green.
##########
grails-console/src/main/groovy/grails/ui/command/GrailsApplicationContextCommandRunner.groovy:
##########
@@ -43,10 +44,12 @@ class GrailsApplicationContextCommandRunner extends DevelopmentGrailsApplication
@Override
ConfigurableApplicationContext run(String... args) {
- def command = ApplicationContextCommandRegistry.instance.findCommand(commandName)
+ ApplicationContextCommandRegistry commandRegistry = ApplicationContextCommandRegistry.instance
+ def command = commandRegistry.findCommand(commandName)
if (command) {
+ Object autowireTarget = resolveAutowireTarget(command)
- Object skipBootstrap = command.hasProperty('skipBootstrap')?.getProperty(command)
+ Object skipBootstrap = autowireTarget.hasProperty('skipBootstrap')?.getProperty(autowireTarget)
Review Comment:
Reading `skipBootstrap` off the unwrapped target is the right call — the adapter forwards `handle`, not arbitrary properties — but nothing tests it. `grep -rn skipBootstrap` across the repo returns only production code: this file, and the `skipBootstrap = true` declarations in the hibernate5/7 `SchemaExportCommand` and `ApplicationContextDatabaseMigrationCommand`, scaffolding, and spring-security. No spec mentions it.
That matters more than a coverage count, because the flag decides whether `BootStrap.groovy` runs. If a later change collapses this back to `command.hasProperty('skipBootstrap')`, a legacy command declaring `Boolean skipBootstrap = true` silently stops setting `Settings.SETTING_SKIP_BOOTSTRAP` and BootStrap executes during something like `dbm-update` against whatever datasource is configured — with the whole suite still green.
`run()` has the same hole: `LegacyCommandCompatibilityIntegrationSpec` performs the autowire unwrap by hand at :85-98 instead of driving the runner, so reverting line 79 back to `command` also leaves every test passing.
Small fix: extract the flag lookup into a static helper next to `resolveAutowireTarget` and cover adapter-target / plain-command / no-property in `GrailsApplicationContextCommandRunnerSpec`, which this commit already added.
Implemented on [test/8.0.x-legacy-command-e2e](https://github.com/jdaugherty/grails-core/tree/test/8.0.x-legacy-command-e2e): the lookup moves into `static Boolean resolveSkipBootstrap(Object)` beside `resolveAutowireTarget`, with `run()` behaviour unchanged (the `System.getProperty` guard stays put). Four features in `GrailsApplicationContextCommandRunnerSpec` cover plain command, no-property, adapter-target, and non-Boolean. Mutation-checked: pointing the helper back at `command` instead of the resolved target fails exactly the adapter-target case. `:grails-console:test --tests GrailsApplicationContextCommandRunnerSpec` green (34 features).
##########
grails-test-examples/legacy-g7-command-plugin/build.gradle:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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 / Groovy 4.
+// 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)
Review Comment:
Commenting on this line, though the Java version itself isn't the issue — 17 is correct and should stay. It's the minimum JDK for a Grails 7 app (`7.0.x` pins `java=17.0.18-librca` in `.sdkmanrc` and `javaVersion=17` in `gradle.properties`), and matching what a Grails 7 app would actually be built with is the entire point of this fixture; anything newer would weaken what it proves. The issue is *where* that requirement is expressed.
Right now it's a Gradle toolchain, and this is the only production build in the repo that pins one — every other `JavaLanguageVersion` is a TestKit resource substituted with `__CURRENT_JDK__` or a gated java-compat test project. We've deliberately stayed off toolchain configuration, and I don't want this fixture to be what introduces it into the core build.
As written it also can't be satisfied. Nothing provisions a JDK 17: there's no `foojay-resolver` or `toolchainManagement` block in this build's `settings.gradle` (which is just `rootProject.name`) or in the root `settings.gradle`, and no `org.gradle.java.installations.*` property anywhere. The root `.sdkmanrc` pins `java=21.0.7-librca` and nothing else, and `etc/bin/Dockerfile` is `FROM bellsoft/liberica-openjdk-debian:21.0.7` with only a secondary JDK 25 at `/opt/liberica-jdk25`.
And it isn't confined to running the integration test. The fixture jar is an `implementation` dependency (`legacy-commands/build.gradle:45`), so `buildLegacyG7CommandFixture` is in the task graph for plain compilation:
```
$ ./gradlew :grails-test-examples-legacy-commands:compileGroovy --dry-run -PskipTests
:grails-test-examples-legacy-commands:buildLegacyG7CommandFixture SKIPPED
:grails-test-examples-legacy-commands:compileGroovy SKIPPED
```
That's reached by `etc/bin/test-reproducible-builds.sh:42` (`./gradlew build --rerun-tasks -PskipTests ...`). With JDK 17 hidden:
```
$ ./gradlew -p grails-test-examples/legacy-g7-command-plugin jar \
-Dorg.gradle.java.installations.auto-detect=false
> Cannot find a Java installation on your machine matching:
{languageVersion=17, vendor=any vendor, ...}. Toolchain auto-provisioning is not enabled.
```
So a contributor whose only JDK is the one we document can't build the repo. CI can't catch it — it's green only because the GitHub runner images happen to ship a JDK 17 that Gradle auto-detects.
The fix isn't a different Java version or a resolver plugin. It's to stop wiring a second JDK requirement into the core build at all, and instead use the composition model we already use for every other build in this repo that has its own needs: `grails-forge`, `grails-gradle`, `build-logic`. Those are independent builds with their own `settings.gradle` and their own `gradlew`, and the dependency runs one way — `grails-forge/settings.gradle:75` does `includeBuild('..')`, so Forge composes grails-core and grails-core's own `./gradlew build` never reaches Forge. CI runs each as its own job (`working-directory: 'grails-forge'`, `working-directory: 'grails-gradle'`).
I'd like a new top-level `end-to-end` build to serve as the base for this kind of test, with the Grails 7 fixture and the `legacy-commands` example that consumes it as its first occupants:
- drop the `java { toolchain { ... } }` block;
- move the fixture and its consuming example into `end-to-end`, out of the root build graph, so `./gradlew build -PskipTests` never triggers `buildLegacyG7CommandFixture` and the core build never needs a JDK it doesn't have;
- declare the Java 17 requirement in a `.sdkmanrc` next to the Grails 7 half — same mechanism the root and `7.0.x` already use, and it keeps 17 an explicit, greppable statement of intent rather than something Gradle has to go find;
- run it from its own CI workflow, the way Forge and the Gradle plugins are run.
That also gives us an obvious home for the next end-to-end compatibility test instead of finding a new corner for each one.
Two things worth planning for in that layout, both of which a dedicated workflow handles cleanly and the core build can't:
1. **Two JDKs, explicitly.** The Grails 7 fixture wants 17; the Grails 8 app consuming it needs 21+. In a dedicated workflow that's just two `setup-java` steps around two build invocations, which is more honest than asking one Gradle build to straddle both — and it's what removes the auto-detection dependency that's making CI pass by luck today.
2. **Keep the Grails 7 half outside dependency substitution.** If `includeBuild('..')` reaches it, `org.apache.grails:grails-core:7.0.14` gets rewritten to the local Groovy 5 project and the proof evaporates — which is exactly why you chose `GradleBuild` over `includeBuild` originally, and I agreed with that reasoning. The Grails 8 consuming app can compose the root normally; the fixture can't.
I've put this together as a working branch rather than leave it as a sketch: [test/8.0.x-legacy-command-e2e](https://github.com/jdaugherty/grails-core/tree/test/8.0.x-legacy-command-e2e) ([single commit](https://github.com/jdaugherty/grails-core/commit/afa4d80440)).
The three projects move to a top-level `end-to-end` build and the toolchain block is gone. Only the Grails 7 fixture carries a `.sdkmanrc` (17); the Grails 8 side tracks the repository's root `.sdkmanrc` rather than re-pinning 21, since it has to run on whatever the core build it consumes runs on. `.github/workflows/end-to-end.yml` reads both versions out of those two files, so it cannot drift from what `sdk env` gives a developer.
The part worth a look is how it resolves Grails. Rather than `includeBuild('..')`, it consumes the artifacts the core build publishes — the same `build/local-maven` that grails-forge points its generated applications at, populated by `publishAllPublicationsToTestCaseMavenRepoRepository` in the root and grails-gradle builds. That is what makes these end-to-end: they go through real poms and module metadata the way an application does. `settings.gradle` scopes the repository with `exclusiveContent` so a remote snapshot can't quietly satisfy an `org.apache.grails` request and leave the suite testing something other than the working tree.
It also disposes of the CLI companion problem instead of working around it. `grails-core-cli` is a secondary capability of `:grails-core` rather than a project, so composite substitution can't express it and dies on a capability self-conflict — but it's a first-class published module whose metadata `CliPublishingSupport` already rewrites for external consumers, so resolving from the repository just works. Verified byte-for-byte: the `grails-core-cli` jar the suite resolves is sha1-identical to the one the publish task had just written to `build/local-maven`.
Verified end to end: `end-to-end/./gradlew check` green with all six integration tests passing, the root `./gradlew` configures with zero references to the moved projects, and the fixture builds under JDK 17 with `-Dorg.gradle.java.installations.auto-detect=false`. Take it as a proposal, not a demand.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5046901223
@jdaugherty On M4 timing: yes, I want this compatibility path landed before M4 if we can.
Current pushed state:
- separate execution-only `grails-core-cli-legacy` artifact
- two flags (`cliAutoProvision`, `legacyCommandSupport`), both default on
- linkage failures isolated/loud
- docs updated
Still open for a stronger green light:
- real Grails 7 / Groovy 4 precompiled fixture via composite build
- whether `legacyCommandSupport` default should stay on or flip off
- remaining Forge CI failures from empty `-cli` versions should be helped by the version-pin work in `c798c93778`; watching the new CI run
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5046901440
@jdaugherty Agreed. I will not resolve review threads unless the full ask is actually done and you are good with it.
For the current open items I am only replying with status, not marking resolved.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5015698968
## 🚨 TestLens detected 2 failed tests 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails Forge \(Java 25, indy\=false\)](https://github.com/apache/grails-core/actions/runs/29685948214/job/88190091556?pr=16011) > :test-core:test [grails-forge]
| Test | Runs | Flakiness |
|---|---|--:|
| CreateAppSpec > test basic create-app build task | ❌ | 0% 🟢 |
| CreateRestApiSpec > test rest-api app with default json-views | ❌ | 0% 🟢 |
🏷️ Commit: be98c6228a3d901f13a18449754ac404f23fb3d8
▶️ Tests: 26851 executed
🟡 Checks: 23/55 completed
### Test Failures
<details>
<summary><strong>CreateAppSpec > test basic create-app build task</strong> (:test-core:test [grails-forge] in <a href="https://github.com/apache/grails-core/actions/runs/29685948214/job/88190091556?pr=16011">CI / Build Grails Forge (Java 25, indy=false)</a>)</summary>
```
org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /tmp/test-app5241058619850452531 with arguments [build, -x, integrationTest]
Output:
> Task :assetCompile FAILED
[Incubating] Problems report is available at: file:///tmp/test-app5241058619850452531/build/reports/problems/problems-report.html
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':assetCompile' (registered by plugin 'cloud.wondrify.asset-pipeline').
> Could not resolve all files for configuration ':compileClasspath'.
> Could not find org.apache.grails:grails-scaffolding-cli:.
Required by:
root project 'test-app5241058619850452531'
> Could not find org.apache.grails:grails-data-hibernate5-cli:.
Required by:
root project 'test-app5241058619850452531'
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights from a Build Scan (powered by Develocity).
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/9.6.0/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD FAILED in 1m 3s
1 actionable task: 1 executed
at org.gradle.testkit.runner.internal.DefaultGradleRunner.lambda$build$2(DefaultGradleRunner.java:275)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.run(DefaultGradleRunner.java:374)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.build(DefaultGradleRunner.java:273)
at org.grails.forge.utils.CommandSpec.executeGradle(CommandSpec.groovy:79)
at org.grails.forge.create.CreateAppSpec.test basic create-app build task(CreateAppSpec.groovy:49)
```
</details>
<details>
<summary><strong>CreateRestApiSpec > test rest-api app with default json-views</strong> (:test-core:test [grails-forge] in <a href="https://github.com/apache/grails-core/actions/runs/29685948214/job/88190091556?pr=16011">CI / Build Grails Forge (Java 25, indy=false)</a>)</summary>
```
org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /tmp/testrestapi6225535858331965300 with arguments [build]
Output:
> Task :compileJava NO-SOURCE
> Task :compileGroovy FAILED
[Incubating] Problems report is available at: file:///tmp/testrestapi6225535858331965300/build/reports/problems/problems-report.html
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':compileGroovy' (registered by plugin class 'org.gradle.api.plugins.GroovyBasePlugin').
> Could not resolve all files for configuration ':compileClasspath'.
> Could not find org.apache.grails:grails-data-hibernate5-cli:.
Required by:
root project 'testrestapi6225535858331965300'
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights from a Build Scan (powered by Develocity).
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/9.6.0/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD FAILED in 53s
1 actionable task: 1 executed
at org.gradle.testkit.runner.internal.DefaultGradleRunner.lambda$build$2(DefaultGradleRunner.java:275)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.run(DefaultGradleRunner.java:374)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.build(DefaultGradleRunner.java:273)
at org.grails.forge.utils.CommandSpec.executeGradle(CommandSpec.groovy:79)
at org.grails.forge.create.CreateRestApiSpec.test rest-api app with default json-views(CreateRestApiSpec.groovy:30)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 CreateAppSpec > test basic create-app build task <!-- uniqueId=[engine:spock]/[spec:org.grails.forge.create.CreateAppSpec]/[feature:$spock_feature_1_0] -->
🔲 CreateRestApiSpec > test rest-api app with default json-views <!-- uniqueId=[engine:spock]/[spec:org.grails.forge.create.CreateRestApiSpec]/[feature:$spock_feature_1_0] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610546265
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
Review Comment:
Good suggestion - switched to the annotation in 5069bef718. I used plain `@Slf4j` (which generates a `log` field) rather than `@Slf4j('LOG')`, to match the dominant convention across the codebase: unqualified `@Slf4j` is used 162 times - including the sibling `ConfigReportCommand` in this same package - while `@Slf4j('LOG')` appears zero times. The two call sites now use `log.warn(...)` and the manual `Logger`/`LoggerFactory` field is gone.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
Review Comment:
Good suggestion - switched to the annotation in 5069bef718. I used plain `@Slf4j` (which generates a `log` field) rather than `@Slf4j('LOG')`, to match the dominant convention across the codebase: unqualified `@Slf4j` is used 162 times - including the sibling `ConfigReportCommand` in this same package - while `@Slf4j('LOG')` appears zero times. The two call sites now use `log.warn(...)` and the manual `Logger`/`LoggerFactory` field is gone.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5053242993
## ✅ All tests passed ✅
🏷️ Commit: 116b0efab3a28f5f13470f1f76d522e1c3208faa
▶️ Tests: 34361 executed
⚪️ Checks: 61/61 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610494970
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -46,6 +53,37 @@ class ApplicationContextCommandRegistry {
commands[cmd.name] = cmd
}
}
+
+ loadLegacyCommands(ApplicationContextCommandRegistry.classLoader)
+ loadLegacyCommands(Thread.currentThread().contextClassLoader)
Review Comment:
Good catch - fixed. The context-classloader pass is now guarded so it only runs when it is a distinct classloader (71f73fc7a7), and the legacy commands are additionally gathered from both classloaders into a Class-identity-deduplicated set before any are instantiated (be98c6228a). A child context classloader delegates to its parent and so reports the parent's `grails.factories` entries too; de-duplicating by the resolved `Class` collapses that delegated duplicate while still keeping genuinely distinct classes from separate classloaders, so a parent-visible legacy command is no longer instantiated twice.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -46,6 +53,37 @@ class ApplicationContextCommandRegistry {
commands[cmd.name] = cmd
}
}
+
+ loadLegacyCommands(ApplicationContextCommandRegistry.classLoader)
+ loadLegacyCommands(Thread.currentThread().contextClassLoader)
+ }
+
+ @SuppressWarnings('deprecation')
+ private void loadLegacyCommands(ClassLoader classLoader) {
+ // Instantiate each legacy command in isolation: a single stale Grails 7 command whose
+ // no-arg constructor (or getName()) throws under Grails 8 must be skipped with a warning,
+ // never abort the whole registry and take valid legacy and new-contract commands down with it.
+ List<Class<grails.dev.commands.ApplicationCommand>> legacyClasses = GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, classLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION)
+ for (Class<grails.dev.commands.ApplicationCommand> legacyClass : legacyClasses) {
+ try {
+ grails.dev.commands.ApplicationCommand legacyCommand = legacyClass.getDeclaredConstructor().newInstance()
+ ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand)
+ String name = command.name
+ if (commands.containsKey(name)) {
+ continue
+ }
+ commands[name] = command
+ if (!legacyCommandWarningLogged) {
+ 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.", name)
+ legacyCommandWarningLogged = true
+ }
+ }
+ catch (Throwable e) {
+ LOG.warn("Failed to load a Grails 7 legacy command from class '{}' through the deprecated grails.dev.commands compatibility layer; skipping it. Cause: {}",
+ legacyClass?.name, e.message)
+ }
Review Comment:
Fixed in 71f73fc7a7 - the warning now passes the `Throwable` as the final SLF4J argument instead of `e.message`, so the stack trace is logged for diagnosis.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610549387
##########
grails-test-examples/legacy-commands-plugin/build.gradle:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+plugins {
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.dependency-validator'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-plugin'
+}
+
+version = '0.0.1'
+group = 'legacy.commands.plugin'
+
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ // This fixture recompiles legacy command sources against Grails 8's grails-core-cli to
+ // validate discovery, adapter, registry, and runner wiring end-to-end. It does not
+ // re-validate a pre-compiled Grails 7 binary's Groovy-trait ABI. That relies on Groovy's
+ // stable trait encoding across 4->5 and could be strengthened later with a prebuilt Grails 7
+ // fixture jar.
+ compileOnly 'org.apache.grails:grails-core-cli'
Review Comment:
This comment concedes the point the PR's headline claim rests on: nothing in the suite executes a command class that was actually compiled by Grails 7 / Groovy 4. Trait consumers get the trait implementation woven in at *their* compile time — a published plugin's class carries static calls into `ApplicationCommand$Trait$Helper`, the `$Trait$FieldHelper` accessors for `applicationContext`, and (for `GrailsApplicationCommand`) the `@Delegate`-generated forwarders to `TemplateRenderer`/`FileSystemInteraction` that the Grails 7 compiler baked into the plugin class. Whether all of that links against these re-authored traits as compiled by Groovy 5 is exactly the question this PR exists to answer, and recompiling the fixture answers a different one.
The fixture needs to compile against a hard-coded, published Grails 7 release (e.g. `org.apache.grails:grails-core:7.1.1` with its Groovy 4 toolchain) — not left open to resolve this repo's Groovy 5 `grails-core-cli`, which is what happens here and is what makes the test answer the wrong question. And this really needs two example apps, not one:
1. a **legacy** fixture: the command plugin built against the pinned Grails 7 release, producing a genuine Groovy 4 trait-consumer binary, exactly as published plugins exist in the wild today;
2. an **upgraded** fixture: a Grails 8 application that consumes that unchanged legacy plugin binary and executes its commands through the registry/runner/Gradle-task path.
Until that combination passes, "unchanged published Grails 7 plugins keep working without a re-release" is unverified — and if it doesn't hold, the layer only serves plugins that recompile anyway, which could migrate to the new API for the same effort. Given we'd be committing to a deprecation/compat contract in the upgrade docs, this needs to be proven before merge, not strengthened later.
Separately: the fact that `compileOnly 'org.apache.grails:grails-core-cli'` is sufficient to compile `grails.dev.commands.*` sources is itself the ABI-leak problem flagged on `ApplicationCommand.groovy`.
##########
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:
This resolves `runtimeClasspath` during configuration (`afterEvaluate`) for every project applying the plugin, on every invocation — including `gradle help` — which the split's own companion discovery deliberately avoids by staying inside `grailsCli.withDependencies` with lenient artifact views. Beyond the cost, the `catch (Throwable)` fallback makes the *task set* nondeterministic: whether `helloLegacyApp` exists depends on whether sibling projects happened to be configured yet, so the same command line can succeed locally and fail on CI with "task not found", and a configuration-cache entry can bake in either outcome. An info-level log line is the only signal that half the feature silently disengaged.
If legacy per-command tasks are kept, they need a resolution-order-independent source (the marker/`withDependencies` mechanism the cli companion discovery already uses), or should be dropped in favor of documenting `runCommand -Pargs=...` for legacy plugins — a sometimes-there task is worse than a consistently-absent one. This is also an argument for hosting the whole legacy path in a separate extension-applied compat project, where its wiring can be opt-in rather than a tax on every build.
##########
grails-core/src/cli/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:
Restoring `grails.dev.commands.*` inside `grails-core-cli` makes the deprecated contract part of the new artifact's compile ABI: any plugin that compiles against `grails-core-cli` (as this PR's own `legacy-commands-plugin` fixture does via `compileOnly`) can keep authoring *new* commands against the old package with nothing but a deprecation warning to stop them. That sets plugin authors up to be switched twice — once now if they migrate, and again in the next major when these types are deleted out of `grails-core-cli` itself, which at that point is a second breaking change to a core artifact rather than the removal of a shim.
Suggest moving the restored types plus `LegacyApplicationCommandAdapter` into a separate compatibility project (e.g. `grails-cli-compat7`) that is never part of `grails-core-cli`'s API surface. The Grails Gradle plugin then auto-applies that artifact to the command classpath, gated by an extension flag that **defaults to `false`** — e.g. `grails { legacyCommandSupport = true }` next to `cliAutoProvision` — so a user with an unmigrated Grails 7 command plugin flips one switch, and nothing legacy is provisioned for anyone else. That way:
- the legacy surface is off by default: new builds cannot silently compile or run against `grails.dev.commands.*` without an explicit, visible opt-in in their build script;
- a plugin updating to Grails 8 compiles against the new API only, so the rename impacts it exactly once;
- retiring the layer means dropping one artifact and one flag from the extension wiring, not breaking `grails-core-cli` again;
- the dual-load logic added to `ApplicationContextCommandRegistry` can live in the compat artifact behind a hook instead of permanently inside the new registry.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
+
private final Map<String, ApplicationCommand> commands = [:]
+ private boolean legacyCommandWarningLogged
ApplicationContextCommandRegistry() {
+ ClassLoader registryClassLoader = ApplicationContextCommandRegistry.classLoader
+ ClassLoader contextClassLoader = Thread.currentThread().contextClassLoader
+
for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- ApplicationContextCommandRegistry.classLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ registryClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
if (!commands.containsKey(cmd.name)) {
commands[cmd.name] = cmd
}
}
- // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is also used to pull any commands that are loaded from the gradle classpath
- for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- Thread.currentThread().contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
- if (!commands.containsKey(cmd.name)) {
- commands[cmd.name] = cmd
+ // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is
+ // also used to pull any commands that are loaded from the gradle classpath. Only when it is a distinct
+ // classloader: repeating the scan for the same classloader would re-instantiate every command (whose
+ // constructor may have side effects) just to discard it on the name-collision check below.
+ if (contextClassLoader != registryClassLoader) {
+ for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
+ contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ if (!commands.containsKey(cmd.name)) {
+ commands[cmd.name] = cmd
+ }
+ }
+ }
+
+ loadLegacyCommands(registryClassLoader, contextClassLoader)
+ }
+
+ @SuppressWarnings('deprecation')
+ private void loadLegacyCommands(ClassLoader registryClassLoader, ClassLoader contextClassLoader) {
+ // Gather the legacy command classes from the registry classloader and, when it is a distinct
+ // classloader, the thread context classloader, de-duplicated by Class identity before any are
+ // instantiated. A child context classloader delegates to its parent, so it also reports the
+ // parent's grails.factories entries; de-duplicating by the resolved Class avoids instantiating a
+ // parent-visible legacy command twice (its constructor may have side effects) only to discard the
+ // duplicate on the name-collision check below.
+ Set<Class<? extends grails.dev.commands.ApplicationCommand>> legacyClasses = new LinkedHashSet<>()
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, registryClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ if (contextClassLoader != null && contextClassLoader != registryClassLoader) {
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, contextClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ }
+ // Instantiate each in isolation: a single stale Grails 7 command whose no-arg constructor (or
+ // getName()) throws under Grails 8 must be skipped with a warning, never abort the whole registry
+ // and take valid legacy and new-contract commands down with it.
+ for (Class<? extends grails.dev.commands.ApplicationCommand> legacyClass : legacyClasses) {
+ try {
+ grails.dev.commands.ApplicationCommand legacyCommand = legacyClass.getDeclaredConstructor().newInstance()
+ ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand)
+ String name = command.name
+ if (commands.containsKey(name)) {
+ continue
+ }
+ commands[name] = command
+ if (!legacyCommandWarningLogged) {
+ 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.", name)
+ legacyCommandWarningLogged = true
+ }
+ }
+ catch (Throwable e) {
Review Comment:
Two concerns here:
1. Swallowing `Throwable` means the most likely real-world failure — a Grails 7 binary whose trait-woven bytecode does not link against the restored traits (`NoSuchMethodError`/`NoClassDefFoundError` at instantiation) — degrades to a warning in build output and a silently missing command. That is the same user-visible symptom as having no compatibility layer at all, but harder to diagnose because the docs now promise the command should work. If linkage failure is possible, it should surface loudly for the named command, not blend into log noise.
2. More structurally, this hardwires the deprecated discovery path into the new registry's constructor for the lifetime of the compat window. If the legacy contract lived in a separate extension-applied compat artifact instead, this could be a lookup through an extension hook (or a second factories key contributed by that artifact), keeping `ApplicationContextCommandRegistry` free of `grails.dev.commands` references — and its eventual removal wouldn't touch this class at all.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5016440743
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16011?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `16.82692%` with `173 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 51.4430%. Comparing base ([`7804f4b`](https://app.codecov.io/gh/apache/grails-core/commit/7804f4b011f04504d94c5fdf2fd8b8c0624f42e3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d1fe495`](https://app.codecov.io/gh/apache/grails-core/commit/d1fe4956a6d59ed6ac68a71bafe2020d813b11f6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 5 commits behind head on 8.0.x.
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16011?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../dev/commands/template/TemplateRendererImpl.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fdev%2Fcommands%2Ftemplate%2FTemplateRendererImpl.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL2Rldi9jb21tYW5kcy90ZW1wbGF0ZS9UZW1wbGF0ZVJlbmRlcmVySW1wbC5ncm9vdnk=) | 5.7971% | [65 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...s/dev/commands/io/FileSystemInteractionImpl.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fdev%2Fcommands%2Fio%2FFileSystemInteractionImpl.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL2Rldi9jb21tYW5kcy9pby9GaWxlU3lzdGVtSW50ZXJhY3Rpb25JbXBsLmdyb292eQ==) | 10.6061% | [59 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...radle/plugin/commands/GrailsCliGradlePlugin.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-gradle%2Fplugins%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fgradle%2Fplugin%2Fcommands%2FGrailsCliGradlePlugin.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWdyYWRsZS9wbHVnaW5zL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2dyYWRsZS9wbHVnaW4vY29tbWFuZHMvR3JhaWxzQ2xpR3JhZGxlUGx1Z2luLmdyb292eQ==) | 0.0000% | [37 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...cli/compat/LegacyApplicationCommandProvider.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fapache%2Fgrails%2Fcore%2Fcli%2Fcompat%2FLegacyApplicationCommandProvider.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvb3JnL2FwYWNoZS9ncmFpbHMvY29yZS9jbGkvY29tcGF0L0xlZ2FjeUFwcGxpY2F0aW9uQ29tbWFuZFByb3ZpZGVyLmdyb292eQ==) | 66.6667% | [0 Missing and 5 partials :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rails/dev/commands/io/FileSystemInteraction.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fdev%2Fcommands%2Fio%2FFileSystemInteraction.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL2Rldi9jb21tYW5kcy9pby9GaWxlU3lzdGVtSW50ZXJhY3Rpb24uZ3Jvb3Z5) | 0.0000% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...mmand/GrailsApplicationContextCommandRunner.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-console%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fui%2Fcommand%2FGrailsApplicationContextCommandRunner.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvbnNvbGUvc3JjL21haW4vZ3Jvb3Z5L2dyYWlscy91aS9jb21tYW5kL0dyYWlsc0FwcGxpY2F0aW9uQ29udGV4dENvbW1hbmRSdW5uZXIuZ3Jvb3Z5) | 40.0000% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16011 +/- ##
================================================
+ Coverage 0 51.4430% +51.4430%
- Complexity 0 17775 +17775
================================================
Files 0 2044 +2044
Lines 0 95702 +95702
Branches 0 16600 +16600
================================================
+ Hits 0 49232 +49232
- Misses 0 39155 +39155
- Partials 0 7315 +7315
```
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16011?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../cli/compat/LegacyApplicationCommandAdapter.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fapache%2Fgrails%2Fcore%2Fcli%2Fcompat%2FLegacyApplicationCommandAdapter.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvb3JnL2FwYWNoZS9ncmFpbHMvY29yZS9jbGkvY29tcGF0L0xlZ2FjeUFwcGxpY2F0aW9uQ29tbWFuZEFkYXB0ZXIuZ3Jvb3Z5) | `100.0000% <100.0000%> (ø)` | |
| [...commands/factory/GroovyScriptCommandFactory.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-shell-cli%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fcli%2Fprofile%2Fcommands%2Ffactory%2FGroovyScriptCommandFactory.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNoZWxsLWNsaS9zcmMvbWFpbi9ncm9vdnkvb3JnL2dyYWlscy9jbGkvcHJvZmlsZS9jb21tYW5kcy9mYWN0b3J5L0dyb292eVNjcmlwdENvbW1hbmRGYWN0b3J5Lmdyb292eQ==) | `90.0000% <100.0000%> (ø)` | |
| [...mmand/GrailsApplicationContextCommandRunner.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-console%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fui%2Fcommand%2FGrailsApplicationContextCommandRunner.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvbnNvbGUvc3JjL21haW4vZ3Jvb3Z5L2dyYWlscy91aS9jb21tYW5kL0dyYWlsc0FwcGxpY2F0aW9uQ29udGV4dENvbW1hbmRSdW5uZXIuZ3Jvb3Z5) | `7.6923% <40.0000%> (ø)` | |
| [...rails/dev/commands/io/FileSystemInteraction.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fdev%2Fcommands%2Fio%2FFileSystemInteraction.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL2Rldi9jb21tYW5kcy9pby9GaWxlU3lzdGVtSW50ZXJhY3Rpb24uZ3Jvb3Z5) | `0.0000% <0.0000%> (ø)` | |
| [...cli/compat/LegacyApplicationCommandProvider.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fapache%2Fgrails%2Fcore%2Fcli%2Fcompat%2FLegacyApplicationCommandProvider.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvb3JnL2FwYWNoZS9ncmFpbHMvY29yZS9jbGkvY29tcGF0L0xlZ2FjeUFwcGxpY2F0aW9uQ29tbWFuZFByb3ZpZGVyLmdyb292eQ==) | `66.6667% <66.6667%> (ø)` | |
| [...radle/plugin/commands/GrailsCliGradlePlugin.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-gradle%2Fplugins%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fgradle%2Fplugin%2Fcommands%2FGrailsCliGradlePlugin.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWdyYWRsZS9wbHVnaW5zL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2dyYWRsZS9wbHVnaW4vY29tbWFuZHMvR3JhaWxzQ2xpR3JhZGxlUGx1Z2luLmdyb292eQ==) | `0.0000% <0.0000%> (ø)` | |
| [...s/dev/commands/io/FileSystemInteractionImpl.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fdev%2Fcommands%2Fio%2FFileSystemInteractionImpl.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL2Rldi9jb21tYW5kcy9pby9GaWxlU3lzdGVtSW50ZXJhY3Rpb25JbXBsLmdyb292eQ==) | `10.6061% <10.6061%> (ø)` | |
| [.../dev/commands/template/TemplateRendererImpl.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16011?src=pr&el=tree&filepath=grails-core-cli-legacy%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fdev%2Fcommands%2Ftemplate%2FTemplateRendererImpl.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWNvcmUtY2xpLWxlZ2FjeS9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL2Rldi9jb21tYW5kcy90ZW1wbGF0ZS9UZW1wbGF0ZVJlbmRlcmVySW1wbC5ncm9vdnk=) | `5.7971% <5.7971%> (ø)` | |
... and [2036 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16011/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5048908946
## Latest push summary (`1d8edac99e`)
Addressed remaining review/proof gaps and the Code Style CI failure.
### 1. Removed legacy named-task scan
- Deleted `loadLegacyCommandNamesFromRuntimeClasspath` and the configuration-time `runtimeClasspath` scan for `grails.dev.commands.ApplicationCommand`
- Legacy application commands no longer invent flaky named Gradle tasks
- They stay available via `runCommand` / shell adapter once `grails-core-cli-legacy` is provisioned
- TestKit now asserts `LEGACY_COMMAND_TASK_PRESENT=false` and `RUN_COMMAND_TASK_PRESENT=true`
- Docs updated to show the `runCommand` path
### 2. Real Grails 7 / Groovy 4 precompiled fixture
- New standalone project: `grails-test-examples/legacy-g7-command-plugin`
- Compiles against published `grails-core:7.0.10` / Groovy `4.0.30`
- Manifest stamps: `Grails-Compile-Version: 7.0.10`, `Groovy-Compile-Version: 4.0.30`
- Built via `GradleBuild` (not monorepo `includeBuild`) so monorepo BOM/JVM substitution cannot rewrite the fixture
- Wired into `grails-test-examples/legacy-commands`
- Integration test proves:
- Adapter targets `legacy.g7.commands.HelloG7PrecompiledCommand`
- Class loads from the G7 fixture jar
- Command executes and writes `G7-RAN`
- `LegacyCommandCompatibilityIntegrationSpec`: **5/5 green** locally, including the new G7 feature
### 3. Dual flags (already on tip before this note)
| Flag | Default | Scope |
|---|---|---|
| `cliAutoProvision` | on | modern CLI tier + companions |
| `legacyCommandSupport` | on | only `grails-core-cli-legacy` |
Legacy provisions only when **both** are true.
### 4. Code style CI fix
- Single-quoted SLF4J templates
- Dropped unnecessary `?.` on non-null loop variables
- Empty catch variable renamed to `ignored` (EmptyCatchBlock)
- Local: `:grails-core-cli-legacy:codenarcMain` + `:grails-core:codenarcCli` green with `ignoreFailures=false`
### Still open for reviewer preference
- Default-on vs default-off + detect/warn policy for `legacyCommandSupport` (product call currently remains default-on; flip is one convention change if decided)
- Remaining script/YAML packaging pollution tracked in #16035
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "sbglasius (via GitHub)" <gi...@apache.org>.
sbglasius commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610517852
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
Review Comment:
Why not use the `@Slf4j('LOG')` annotation instead?
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
Review Comment:
Why not use the `@Slf4j('LOG')` annotation instead?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610969298
##########
grails-test-examples/legacy-commands-plugin/build.gradle:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+plugins {
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.dependency-validator'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-plugin'
+}
+
+version = '0.0.1'
+group = 'legacy.commands.plugin'
+
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ // This fixture recompiles legacy command sources against Grails 8's grails-core-cli to
+ // validate discovery, adapter, registry, and runner wiring end-to-end. It does not
+ // re-validate a pre-compiled Grails 7 binary's Groovy-trait ABI. That relies on Groovy's
+ // stable trait encoding across 4->5 and could be strengthened later with a prebuilt Grails 7
+ // fixture jar.
+ compileOnly 'org.apache.grails:grails-core-cli'
Review Comment:
@jamesfredley The premise that the monorepo precludes this fixture isn't right — the build already solves exactly this problem with composite builds. The root `settings.gradle` does `includeBuild('./grails-gradle')` and `includeBuild('./build-logic')`, and `grails-forge` is a fully independent build in the same repo. An included build brings its own `settings.gradle`, its own dependency/version management, and its own toolchain configuration; nothing from the root build's BOM, Groovy 5 pins, or convention plugins leaks into it.
So the "Groovy 4 toolchain island" isn't something we'd have to stand up against the build architecture — it's the architecture's existing pattern:
1. a `legacy-commands-grails7` that is a Grails Plugin & **included build** pinned to the published `org.apache.grails:grails-core:7.1.1` (resolved from Maven Central, not this repo) with the Grails 7 Groovy 4 toolchain, producing a genuine trait-consumer plugin jar exactly as plugins exist in the wild;
2. the existing Grails 8 `legacy-commands` test app consuming that jar unchanged and executing its commands through the registry/runner/Gradle-task path.
That's the two-app pair you've already agreed is the right test, and the composite-build boundary is what makes it *prove* the claim: the legacy half is compiled by a real Grails 7 toolchain with no possibility of accidentally resolving this repo's Groovy 5 artifacts. It also costs nothing elsewhere — `grails-test-examples` aren't part of the reproducibility/verification surface, so this adds no burden there.
On scope: this PR's headline claim is that unchanged, already-published Grails 7 command plugins keep working. Merging a backwards-compatibility layer without the test that demonstrates backwards compatibility means committing to the contract on faith and discovering any trait-ABI gap later, after the layer's design is settled. You've already agreed the test must be green before release — since the mechanism to build it is the repo's existing composite-build pattern rather than a new toolchain island, I don't see what deferring buys us. It belongs in this PR.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610932474
##########
grails-test-examples/legacy-commands-plugin/build.gradle:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+plugins {
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.dependency-validator'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-plugin'
+}
+
+version = '0.0.1'
+group = 'legacy.commands.plugin'
+
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ // This fixture recompiles legacy command sources against Grails 8's grails-core-cli to
+ // validate discovery, adapter, registry, and runner wiring end-to-end. It does not
+ // re-validate a pre-compiled Grails 7 binary's Groovy-trait ABI. That relies on Groovy's
+ // stable trait encoding across 4->5 and could be strengthened later with a prebuilt Grails 7
+ // fixture jar.
+ compileOnly 'org.apache.grails:grails-core-cli'
Review Comment:
Follow-up on scope: the ABI-leak half of this is fixed (the fixture now compiles against `grails-core-cli-legacy`, not `grails-core-cli`). The precompiled-binary half I'm tracking as an explicit pre-release item under "Precompiled Grails 7 / Groovy 4 fixture" in the PR follow-ups rather than adding it in this change, and I want to be transparent about why.
Building the genuine artifact you describe - a command plugin compiled by the Grails 7 / Groovy 4 toolchain, producing real `$Trait$Helper` / `$Trait$FieldHelper` / `@Delegate` forwarder bytecode - means standing up a Groovy 4 toolchain island that resolves `org.apache.grails:grails-core:7.1.1` inside a monorepo whose version management, BOM, and compilation are all Groovy 5 / Java 21. That fixture actively fights the current build architecture (pinning a second Groovy major for one subproject, against a different Grails major), so I don't think it belongs as a bolt-on inside this PR's build.
I agree it's the right test to prove the headline claim, and that it should be green before the compat contract is relied on in the upgrade docs - which is exactly why I've flagged it as a pre-release blocker in the follow-ups rather than quietly strengthening it later. My plan is a dedicated fixture pair (a pinned 7.1.1/Groovy 4 "legacy" plugin binary + a Grails 8 "upgraded" app consuming it unchanged) done as its own change where the toolchain isolation can be set up properly, not wedged into this one.
##########
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:
Follow-up on scope: I assessed whether to fix this in the current change and am deferring it deliberately, tracked under "Legacy per-command Gradle task discovery" in the PR follow-ups.
Reasoning: the `afterEvaluate` config-time resolution isn't specific to the legacy scan - it's the existing shape of `configureApplicationCommands`. The non-legacy cli path resolves at configuration time too (`loadCommandNamesFromCliClasspath` resolves `grailsCliClasspath`, and the task classpath is built from `runtimeClasspath` + `grailsCliClasspath` in the same `afterEvaluate`). A genuinely resolution-order-independent fix - the marker / `withDependencies` approach you suggested - means plugins *advertising* their command names via a Gradle capability/attribute rather than the framework scanning jar `META-INF` at all, which is a discovery-protocol change that would have to cover both the cli and legacy paths to stay consistent, not a surgical edit to the legacy branch. That's larger than this PR and I don't want to bolt a half-version onto only the legacy path.
For now the legacy scan mirrors the lenient, skip-on-failure handling the cli path already uses, and degrades to the always-present generic `runCommand` task, so no command becomes unrunnable. I'd rather do the deterministic marker-based discovery as its own change across both paths than leave them inconsistent - happy to take that on as a follow-up, and open to your steer on capability-advertisement vs. dropping per-command legacy tasks entirely in favor of documenting `runCommand -Pargs=...`.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5123320358
The TestLens failures for `GrailsUtilStackFiltererSpec > installed DefaultStackTraceFilterer emits Full Stack Trace by default` and `GrailsBootstrapRegistryInitializerSpec > defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer` are a pre-existing bug on `8.0.x`, unrelated to this PR's changes -- reproducible on plain `8.0.x` today. `DefaultStackTraceFilterer.STACK_LOG` routes through a jcl-over-slf4j commons-logging binding, so the tests' `System.setErr()` capture never observes the emitted message.
Fix: #16067 (replaces the `System.err` capture with a Logback appender attached directly to the logger). Once that merges, rebasing onto `8.0.x` should clear this failure here.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5122404102
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails-Core \(macos-latest, 21\)](https://github.com/apache/grails-core/actions/runs/30482684773/job/90680597939?pr=16011) > :grails-core:test
| Test | Runs | Flakiness |
|---|---|--:|
| GrailsBootstrapRegistryInitializerSpec > defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer | ❌ | 0% 🟢 |
🏷️ Commit: ed1961adfc307a008a373391802326c4d1776741
▶️ Tests: 700 executed
🟡 Checks: 4/52 completed
### Test Failures
<details>
<summary><strong>GrailsBootstrapRegistryInitializerSpec > defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer</strong> (:grails-core:test in <a href="https://github.com/apache/grails-core/actions/runs/30482684773/job/90680597939?pr=16011">CI / Build Grails-Core (macos-latest, 21)</a>)</summary>
```
Condition not satisfied:
baos.toString().contains(StackTraceFilterer.FULL_STACK_TRACE_MESSAGE)
| | | | |
| "" false | Full Stack Trace:
| interface org.grails.exceptions.reporting.StackTraceFilterer
java.io.ByteArrayOutputStream@5e1b425
at org.apache.grails.core.GrailsBootstrapRegistryInitializerSpec.defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer(GrailsBootstrapRegistryInitializerSpec.groovy:257)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 GrailsBootstrapRegistryInitializerSpec > defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer <!-- uniqueId=[engine:spock]/[spec:org.apache.grails.core.GrailsBootstrapRegistryInitializerSpec]/[feature:$spock_feature_0_11] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5016660348
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails Forge \(Java 25, indy\=false\)](https://github.com/apache/grails-core/actions/runs/29695638428/job/88215831494?pr=16011) > :test-core:test [grails-forge]
| Test | Runs | Flakiness |
|---|---|--:|
| CreateAppSpec > test basic create-app build task | ❌ | 0% 🟢 |
🏷️ Commit: 7b53bcc56b019b7d6d441e2edee52e75989e03c4
▶️ Tests: 21208 executed
🟡 Checks: 23/55 completed
### Test Failures
<details>
<summary><strong>CreateAppSpec > test basic create-app build task</strong> (:test-core:test [grails-forge] in <a href="https://github.com/apache/grails-core/actions/runs/29695638428/job/88215831494?pr=16011">CI / Build Grails Forge (Java 25, indy=false)</a>)</summary>
```
org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /tmp/test-app1161209494188316895 with arguments [build, -x, integrationTest]
Output:
> Task :assetCompile FAILED
[Incubating] Problems report is available at: file:///tmp/test-app1161209494188316895/build/reports/problems/problems-report.html
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':assetCompile' (registered by plugin 'cloud.wondrify.asset-pipeline').
> Could not resolve all files for configuration ':compileClasspath'.
> Could not find org.apache.grails:grails-scaffolding-cli:.
Required by:
root project 'test-app1161209494188316895'
> Could not find org.apache.grails:grails-data-hibernate5-cli:.
Required by:
root project 'test-app1161209494188316895'
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights from a Build Scan (powered by Develocity).
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/9.6.0/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD FAILED in 57s
1 actionable task: 1 executed
at org.gradle.testkit.runner.internal.DefaultGradleRunner.lambda$build$2(DefaultGradleRunner.java:275)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.run(DefaultGradleRunner.java:374)
at org.gradle.testkit.runner.internal.DefaultGradleRunner.build(DefaultGradleRunner.java:273)
at org.grails.forge.utils.CommandSpec.executeGradle(CommandSpec.groovy:79)
at org.grails.forge.create.CreateAppSpec.test basic create-app build task(CreateAppSpec.groovy:49)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 CreateAppSpec > test basic create-app build task <!-- uniqueId=[engine:spock]/[spec:org.grails.forge.create.CreateAppSpec]/[feature:$spock_feature_1_0] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5015861079
@jamesfredley if you can let me know today, I'm happy to add the proposed compatibility layer. I'd like to proceed with the M4. Are you wanting this done before the M4?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#discussion_r3610868930
##########
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:
Addressed in d1fe4956a6 by adopting the separate-artifact half of this and declining the opt-in-default-`false` half.
**ABI leak — fixed.** The deprecated `grails.dev.commands.*` contract and `LegacyApplicationCommandAdapter` now live in a new published artifact, `grails-core-cli-legacy`, and are gone from `grails-core-cli`. `grails-core-cli` retains **zero** references to `grails.dev.commands` (its registry talks only to the neutral SPI `ApplicationCommandProvider` / `ApplicationCommandRegistrar` / `ApplicationCommandTargetAware`). The legacy artifact is provisioned **execution-only**: `GrailsCliGradlePlugin` puts it in a non-resolvable `grailsCliLegacy` bucket that feeds only `grailsCliClasspath` (the command runner) and the test-runtime classpaths, and **never any compile classpath** — see the new `configurations.matching { it.name in ['testRuntimeClasspath','integrationTestRuntimeClasspath'] }` block and `grailsCliClasspath.extendsFrom(grailsCli, grailsCliLegacy)` in `GrailsCliGradlePlugin.groovy`, plus `CliAutoDiscoverySpec`, which now asserts the artifact is absent from `compileClasspat
h` / `testCompileClasspath` / `integrationTestCompileClasspath`. So a new plugin can no longer compile against `grails.dev.commands.*` through `grails-core-cli`; authoring against the deprecated package now requires an explicit `grails-core-cli-legacy` dependency. Your "second breaking change to a core artifact" concern also goes away: retiring the window is dropping `grails-core-cli-legacy`, not re-breaking `grails-core-cli`.
Your fourth bullet is implemented literally — the dual-load logic is no longer "permanently inside the new registry"; it lives in `LegacyApplicationCommandProvider` in the compat artifact, contributed through a second factories key that the registry discovers generically.
**Opt-in flag defaulting to `false` — declined, deliberately.** This is the one point I'm pushing back on. The goal is a zero-touch upgrade. Spring Boot 3.x is already EOL and Grails 7 is EOL while Grails 8 is not, so users need to move to 8 now — but the plugin ecosystem moves on a 12–24 month horizon (the 3→4/5 transition is still incomplete years on). A flag that defaults to `false` means an unmigrated command plugin **silently contributes no commands** until the user discovers a switch they have no reason to know exists — which is a worse failure than the command simply working behind a one-time deprecation warning. There is also no "switched twice" problem in practice: an author either does nothing (the shim keeps the Grails 7 plugin working) or migrates once to `org.apache.grails.core.cli.*`. The only way to be "switched twice" is to author *brand-new* code against a package that is already `@Deprecated` and warns at every compile — that is an explicit opt-in aga
inst visible warnings, not something the framework does to anyone by default.
So: legacy support stays **on by default** via the existing `cliAutoProvision`, but it is now isolated in its own artifact and off every compile classpath — which I believe answers the real hazard (accidental new adoption + ABI surface) without degrading the upgrade experience for the ecosystem we're trying not to strand.
##########
grails-core/src/cli/groovy/org/apache/grails/core/cli/ApplicationContextCommandRegistry.groovy:
##########
@@ -29,21 +33,73 @@ import org.grails.core.io.support.GrailsFactoriesLoader
@Singleton(strict = false)
class ApplicationContextCommandRegistry {
+ private static final Logger LOG = LoggerFactory.getLogger(ApplicationContextCommandRegistry)
+
private final Map<String, ApplicationCommand> commands = [:]
+ private boolean legacyCommandWarningLogged
ApplicationContextCommandRegistry() {
+ ClassLoader registryClassLoader = ApplicationContextCommandRegistry.classLoader
+ ClassLoader contextClassLoader = Thread.currentThread().contextClassLoader
+
for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- ApplicationContextCommandRegistry.classLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ registryClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
if (!commands.containsKey(cmd.name)) {
commands[cmd.name] = cmd
}
}
- // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is also used to pull any commands that are loaded from the gradle classpath
- for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
- Thread.currentThread().contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
- if (!commands.containsKey(cmd.name)) {
- commands[cmd.name] = cmd
+ // If this is reflectively loaded from the delegating cli, we need to make sure the context class loader is
+ // also used to pull any commands that are loaded from the gradle classpath. Only when it is a distinct
+ // classloader: repeating the scan for the same classloader would re-instantiate every command (whose
+ // constructor may have side effects) just to discard it on the name-collision check below.
+ if (contextClassLoader != registryClassLoader) {
+ for (ApplicationCommand cmd : GrailsFactoriesLoader.loadFactories(ApplicationCommand,
+ contextClassLoader, GrailsFactoriesLoader.CLI_FACTORIES_RESOURCE_LOCATION)) {
+ if (!commands.containsKey(cmd.name)) {
+ commands[cmd.name] = cmd
+ }
+ }
+ }
+
+ loadLegacyCommands(registryClassLoader, contextClassLoader)
+ }
+
+ @SuppressWarnings('deprecation')
+ private void loadLegacyCommands(ClassLoader registryClassLoader, ClassLoader contextClassLoader) {
+ // Gather the legacy command classes from the registry classloader and, when it is a distinct
+ // classloader, the thread context classloader, de-duplicated by Class identity before any are
+ // instantiated. A child context classloader delegates to its parent, so it also reports the
+ // parent's grails.factories entries; de-duplicating by the resolved Class avoids instantiating a
+ // parent-visible legacy command twice (its constructor may have side effects) only to discard the
+ // duplicate on the name-collision check below.
+ Set<Class<? extends grails.dev.commands.ApplicationCommand>> legacyClasses = new LinkedHashSet<>()
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, registryClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ if (contextClassLoader != null && contextClassLoader != registryClassLoader) {
+ legacyClasses.addAll(GrailsFactoriesLoader.loadFactoryClasses(
+ grails.dev.commands.ApplicationCommand, contextClassLoader, FactoriesLoaderSupport.FACTORIES_RESOURCE_LOCATION))
+ }
+ // Instantiate each in isolation: a single stale Grails 7 command whose no-arg constructor (or
+ // getName()) throws under Grails 8 must be skipped with a warning, never abort the whole registry
+ // and take valid legacy and new-contract commands down with it.
+ for (Class<? extends grails.dev.commands.ApplicationCommand> legacyClass : legacyClasses) {
+ try {
+ grails.dev.commands.ApplicationCommand legacyCommand = legacyClass.getDeclaredConstructor().newInstance()
+ ApplicationCommand command = new LegacyApplicationCommandAdapter(legacyCommand)
+ String name = command.name
+ if (commands.containsKey(name)) {
+ continue
+ }
+ commands[name] = command
+ if (!legacyCommandWarningLogged) {
+ 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.", name)
+ legacyCommandWarningLogged = true
+ }
+ }
+ catch (Throwable e) {
Review Comment:
Split response, since the structural half is now done and the `Throwable` half is a judgment call.
**Structural concern — fixed in d1fe4956a6.** This is exactly the redesign you asked for. `ApplicationContextCommandRegistry` no longer hardwires the deprecated discovery path: it now loads `ApplicationCommandProvider` implementations from `META-INF/grails-cli.factories` (a second factories key), dual-classloader, class-identity deduplicated, and failure-isolated, and has **zero** `grails.dev.commands` references. The legacy discovery lives in `LegacyApplicationCommandProvider` inside the separate `grails-core-cli-legacy` artifact and is reached purely through that SPI hook. Eventual removal of the compat window is dropping the artifact; this class is not touched.
**`Throwable` / loud linkage failure — point taken, partially.** The catch is deliberate per-command isolation (one plugin's faulty command must not suppress every other command), and it now logs a warning that names the failing class. I agree that a linkage failure (`NoSuchMethodError` / `NoClassDefFoundError`) for a command the upgrade docs promise should work is more serious than generic "command failed to load" noise, and blending it into `info`/`warn` makes it hard to diagnose. I've listed "loud linkage failures" as a tracked follow-up on the PR (surface linkage errors prominently for the named command rather than as a generic warning), and I'd value your take on the right signal there — fail the whole run, or elevate to a distinct error log keyed to the command name while still isolating it.
##########
grails-test-examples/legacy-commands-plugin/build.gradle:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+plugins {
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.dependency-validator'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-plugin'
+}
+
+version = '0.0.1'
+group = 'legacy.commands.plugin'
+
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ // This fixture recompiles legacy command sources against Grails 8's grails-core-cli to
+ // validate discovery, adapter, registry, and runner wiring end-to-end. It does not
+ // re-validate a pre-compiled Grails 7 binary's Groovy-trait ABI. That relies on Groovy's
+ // stable trait encoding across 4->5 and could be strengthened later with a prebuilt Grails 7
+ // fixture jar.
+ compileOnly 'org.apache.grails:grails-core-cli'
Review Comment:
Two parts here; one is fixed, one is a legitimate open item I've tracked.
**The ABI-leak sub-point — fixed.** "compileOnly `grails-core-cli` is sufficient to compile `grails.dev.commands.*`" is no longer true. As of d1fe4956a6 those types are gone from `grails-core-cli` and live only in `grails-core-cli-legacy`; the fixture now compiles against `grails-core-cli-legacy` explicitly, which is the intended and only way to build against the deprecated contract. `CliAutoDiscoverySpec` asserts the legacy artifact is absent from every compile classpath.
**The precompiled-binary / trait-ABI point — valid, and I'm not going to hand-wave it.** You're right that recompiling the fixture with this repo's Groovy 5 toolchain answers a different question than "does a plugin class that a Grails 7 / Groovy 4 compiler already wove trait bytecode into (`ApplicationCommand$Trait$Helper` calls, `$Trait$FieldHelper` accessors, the `@Delegate` forwarders to `TemplateRenderer` / `FileSystemInteraction`) still link against these traits as recompiled by Groovy 5?" The restored public signatures were checked against 7.1.1 bytecode, but that is not the same as executing a genuine Groovy 4 trait-consumer binary. I've added this as an explicit **pre-release** follow-up on the PR: a fixture pinned to `org.apache.grails:grails-core:7.1.1` with its Groovy 4 toolchain producing a real trait-consumer jar (the "legacy" app), consumed **unchanged** by a Grails 8 app that executes its commands through the registry/runner/Gradle-task path (the "upgraded" app)
— the two-app combination you describe. Agreed it should be green before this is relied on as a compat contract in the upgrade docs.
##########
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:
Valid and separate from the artifact-isolation work; I've tracked it as an explicit follow-up rather than leave it unaddressed.
You're right that resolving `runtimeClasspath` in `afterEvaluate` for every project on every invocation is both a cost (it runs even for `gradle help`) and, worse, a correctness problem: the `catch (Throwable)` fallback makes the *task set* depend on whether sibling projects were configured yet, so `helloLegacyApp` may exist locally and be "task not found" on CI, and the configuration cache can bake in either outcome. A sometimes-there task is worse than a consistently-absent one.
The fix is to make the legacy per-command task discovery resolution-order-independent — sourced from the same marker / `grailsCli.withDependencies` lenient-view mechanism the CLI companion discovery already uses, rather than an eager `afterEvaluate` classpath resolution — or, failing that, to drop the per-command tasks for legacy plugins and document `runCommand -Pargs=...` instead. I've listed this on the PR under follow-ups. Note the artifact isolation in d1fe4956a6 makes this cleaner to do, since the legacy path is now a well-defined provisioned artifact rather than something interleaved with the core wiring.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5046900978
@jdaugherty Update after the latest push (`c798c93778`):
There is now a dedicated `legacyCommandSupport` flag (default **on**), separate from `cliAutoProvision`.
- `cliAutoProvision` controls modern CLI companions only
- `legacyCommandSupport` controls only the Grails 7 bridge (`grails-core-cli-legacy`)
- Bridge provisions only when **both** are true
- `legacyCommandSupport = false` keeps modern companions and disables only the bridge
I kept the default on so already-published G7 command plugins keep working with zero user action after the #15948 split. The separate flag is the opt-out for people who want the modern CLI tier without the compatibility bridge.
If the project decision is still "default off + loud detect/warn when legacy factories are present", that is a policy call and I can flip the default. The wiring is already split so that change is one convention flip, not a redesign.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restore backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16011:
URL: https://github.com/apache/grails-core/pull/16011#issuecomment-5049928109
## ✅ All tests passed ✅
🏷️ Commit: 1d8edac99e370c8594440360bdf84f0c8af33a44
▶️ Tests: 56212 executed
⚪️ Checks: 61/61 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Opt-in backwards compatibility for Grails 7 command plugins on Grails 8 [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org