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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
