You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "paulk-asert (via GitHub)" <gi...@apache.org> on 2025/01/13 13:07:14 UTC

[PR] early draft [groovy]

paulk-asert opened a new pull request, #2140:
URL: https://github.com/apache/groovy/pull/2140

   (no comment)


-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-11871: Support Maven Resolver based version of Grapes [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2140:
URL: https://github.com/apache/groovy/pull/2140#discussion_r2911425586


##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,670 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.graph.DependencyNode
+import org.eclipse.aether.internal.impl.scope.ScopeDependencySelector

Review Comment:
   `ScopeDependencySelector` is imported from `org.eclipse.aether.internal.impl.scope`, which is an internal/implementation API of Maven Resolver. Internal APIs are not covered by the same compatibility guarantees as public APIs and may change between versions. Consider using the public API alternatives for scope-based dependency selection, or document this as a known fragility so it can be updated if the Maven Resolver version is bumped.



##########
subprojects/groovy-grape-maven/src/test/groovy/groovy/grape/maven/GrapeMavenTest.groovy:
##########
@@ -0,0 +1,62 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.Grape
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.Test
+
+final class GrapeMavenTest {
+
+    @BeforeAll
+    static void setUpClass() {
+        System.setProperty('groovy.grape.impl', 'groovy.grape.maven.GrapeMaven')
+    }

Review Comment:
   The test sets the system property `groovy.grape.impl` in `@BeforeAll` but never restores it in an `@AfterAll` method. This could leak into other tests running in the same JVM/Gradle worker, potentially causing them to use the GrapeMaven engine instead of the default GrapeIvy. Consider adding an `@AfterAll` method that clears the system property and resets `Grape.@instance = null`.



##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,670 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.graph.DependencyNode
+import org.eclipse.aether.internal.impl.scope.ScopeDependencySelector
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.resolution.DependencyResolutionException
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.util.artifact.JavaScopes
+import org.eclipse.aether.util.repository.SimpleArtifactDescriptorPolicy
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+    private static final boolean DEBUG_GRAB = Boolean.getBoolean('groovy.grape.debug')
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    Set<String> downloadedArtifacts = []
+    Set<String> resolvedDependencies = []
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw asRuntimeGrabError(e)
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw asRuntimeGrabError(e)
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {
+            def localRepo = new LocalRepository(grapeCacheDir.toPath())
+            String checksumPolicy = args.disableChecksums ?
+                RepositoryPolicy.CHECKSUM_POLICY_IGNORE :
+                RepositoryPolicy.CHECKSUM_POLICY_WARN
+
+            // Extract exclusions from args if provided by @GrabExclude
+            List<Map<String, String>> exclusions = (List<Map<String, String>>) args.get('excludes') ?: []
+
+            try (RepositorySystemSession.CloseableSession session = system
+                .createSessionBuilder()
+                .withLocalRepositories(localRepo)
+                .setChecksumPolicy(checksumPolicy)
+                .setDependencySelector(ScopeDependencySelector.legacy([JavaScopes.COMPILE, JavaScopes.RUNTIME], [JavaScopes.TEST]))
+                .setArtifactDescriptorPolicy(new SimpleArtifactDescriptorPolicy(true, true))
+                // Set Java version properties for profile evaluation
+                .setSystemProperty('java.version', System.getProperty('java.version'))
+                .setSystemProperty('java.home', System.getProperty('java.home'))
+                // Set OS detection properties (similar to os-maven-plugin)
+                .setSystemProperty('os.detected.name', detectOsName())
+                .setSystemProperty('os.detected.arch', detectOsArch())
+                .setSystemProperty('os.detected.classifier', "${detectOsName()}-${detectOsArch()}")
+                // Only the most essential configurations
+                .setSystemProperty('aether.artifactDescriptor.ignoreErrors', 'true')
+                .setConfigProperty('aether.artifactDescriptor.ignoreInvalidActivationExpressions', 'true')
+                .build()) {
+
+                List<URI> results = []
+
+                for (MavenGrabRecord grabRecord : grabRecords) {
+                    // Skip if this grab record is in the exclusion list
+                    if (isExcluded(grabRecord, exclusions)) {
+                        continue
+                    }
+
+                    String coords = "${grabRecord.groupId()}:${grabRecord.module()}"
+                    if (grabRecord.ext()) {
+                        coords += ":${grabRecord.ext()}"
+                    }
+                    if (grabRecord.classifier()) {
+                        coords += ":${grabRecord.classifier()}"
+                    }
+                    coords += ":${grabRecord.version()}"
+
+                    if (DEBUG_GRAB) {
+                        System.err.println("[GrapeDebug] resolving root=${coords} transitive=${grabRecord.transitive()}")
+                    }
+
+                    Artifact artifact = new DefaultArtifact(coords)
+
+                    String scope = grabRecord.conf()?.get(0) ?: JavaScopes.COMPILE
+                    if (scope == 'default') scope = JavaScopes.COMPILE
+
+                    List<ArtifactResult> artifactResults = []
+                    if (grabRecord.transitive()) {
+                        // Create a filter that prevents traversal into excluded artifacts
+                        DependencyFilter exclusionFilter = createExclusionFilter(exclusions)
+
+                        CollectRequest collectRequest = new CollectRequest(
+                            root: new Dependency(artifact, scope),
+                            repositories: repos
+                        )
+                        DependencyRequest dependencyRequest = new DependencyRequest(collectRequest, exclusionFilter)
+                        try {
+                            artifactResults = system.resolveDependencies(session, dependencyRequest).getArtifactResults()
+                        } catch (DependencyResolutionException dre) {
+                            // Keep resolved artifacts; tolerate unresolved non-root transitives.
+                            artifactResults = []
+                            if (dre.result?.artifactResults) {
+                                for (ArtifactResult ar : dre.result.artifactResults) {
+                                    if (ar.resolved) {
+                                        artifactResults << ar
+                                        continue
+                                    }
+                                    Artifact missing = ar?.request?.artifact
+                                    if (!isIgnorableUnresolvedArtifact(system, session, artifact, missing)) {
+                                        throw dre
+                                    }
+                                }
+                            } else {
+                                throw dre
+                            }
+                        }
+                    } else {
+                        // Non-transitive resolution
+                        ArtifactRequest artifactRequest = new ArtifactRequest(artifact, repos, null)
+                        artifactResults = [system.resolveArtifact(session, artifactRequest)]
+                    }
+
+                    for (ArtifactResult found : artifactResults) {
+                        if (found.artifact.file) {
+                            if (DEBUG_GRAB) {
+                                System.err.println("[GrapeDebug] resolved ${found.artifact.groupId}:${found.artifact.artifactId}:${found.artifact.extension}:${found.artifact.version} -> ${found.artifact.file}")
+                            }
+                            results << found.artifact.file.toURI()
+
+                            if (depsInfo != null) {
+                                depsInfo << [
+                                    'group': found.artifact.groupId,
+                                    'module': found.artifact.artifactId,
+                                    'revision': found.artifact.version
+                                ]
+                            }
+                        }
+                    }
+                }
+
+                return results as URI[]
+            }
+        }
+    }
+
+    private DependencyFilter createExclusionFilter(List<Map<String, String>> exclusions) {
+        if (!exclusions) {
+            return null  // No exclusions, no filtering needed
+        }
+
+        return { DependencyNode node, Collection parents ->
+            // Return false to prevent traversal into excluded artifacts
+            // This stops their dependencies from being collected
+            return !isExcludedArtifact(node.artifact.groupId, node.artifact.artifactId, exclusions)
+        } as DependencyFilter
+    }
+
+    private boolean isExcluded(MavenGrabRecord grabRecord, List<Map<String, String>> exclusions) {
+        return isExcludedArtifact(grabRecord.groupId(), grabRecord.module(), exclusions)
+    }
+
+    private boolean isExcludedArtifact(String groupId, String artifactId, List<Map<String, String>> exclusions) {
+        for (Map<String, String> exclusion : exclusions) {
+            String excludeGroup = exclusion.group ?: exclusion.groupId
+            String excludeModule = exclusion.module ?: exclusion.artifactId
+
+            if (excludeGroup && excludeModule) {
+                if (groupId == excludeGroup && artifactId == excludeModule) {
+                    return true  // This artifact is excluded
+                }
+            }
+        }
+        return false
+    }
+
+    MavenGrabRecord createGrabRecord(Map dep) {
+        String module = dep.module ?: dep.artifactId ?: dep.artifact
+        if (!module) {
+            throw new RuntimeException('grab requires at least a module: or artifactId: or artifact: argument')
+        }
+
+        // check for malformed components of the coordinates
+        dep.each { k, v ->
+            if (v instanceof CharSequence) {
+                if (k.toString().contains('v')) { // revision, version, rev
+                    if (!(v ==~ '[^\\\\/:"<>|]*')) {
+                        throw new RuntimeException("Grab: invalid value of '$v' for $k: should not contain any of / \\ : \" < > |")
+                    }
+                } else {
+                    if (!(v ==~ '[-._a-zA-Z0-9]*')) {
+                        throw new RuntimeException("Grab: invalid value of '$v' for $k: should only contain - . _ a-z A-Z 0-9")
+                    }
+                }
+            }
+        }
+
+        // check for mutually exclusive arguments
+        Set<String> keys = (Set<String>) dep.keySet()
+        keys.each { key ->
+            Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS[key]
+            if (badArgs && !badArgs.disjoint(keys)) {
+                throw new RuntimeException("Grab: mutually exclusive arguments: ${keys.intersect(badArgs) + key}")
+            }
+        }
+
+        String groupId = dep.group ?: dep.groupId ?: dep.organisation ?: dep.organization ?: dep.org ?: ''
+        String version = dep.version ?: dep.revision ?: dep.rev ?: '*'
+        if (version == '*') version = 'LATEST'
+        String classifier = dep.classifier ?: null
+        String ext = dep.ext ?: dep.type ?: 'jar'
+        String type = dep.type ?: ''
+
+        boolean force = dep.containsKey('force') ? dep.force : true
+        boolean changing = dep.containsKey('changing') ? dep.changing : false
+        boolean transitive = dep.containsKey('transitive') ? dep.transitive : true
+
+        new MavenGrabRecord(
+            groupId,
+            module,
+            version,
+            getConfList(dep),
+            ext,
+            type,
+            classifier,
+            force,
+            changing,
+            transitive
+        )
+    }
+
+    @CompileDynamic
+    private List<String> getConfList(Map dep) {
+        def conf = dep.conf ?: dep.scope ?: dep.configuration ?: DEFAULT_CONF
+        if (conf instanceof String) {
+            if (conf.startsWith('[') && conf.endsWith(']')) conf = conf[1..-2]
+            conf = conf.tokenize(',')
+        }
+        (conf as List<String>)
+    }
+
+    @Override
+    Map[] listDependencies(ClassLoader classLoader) {
+        List<? extends Map> results = loadedDeps[classLoader]?.collect { MavenGrabRecord grabbed ->
+            def dep = [
+                group  : grabbed.groupId(),
+                module : grabbed.module(),
+                version: grabbed.version()
+            ]
+            if (grabbed.conf() != DEFAULT_CONF) {
+                dep.conf = grabbed.conf()
+            }
+            if (grabbed.changing()) {
+                dep.changing = grabbed.changing()
+            }
+            if (!grabbed.transitive()) {
+                dep.transitive = grabbed.transitive()
+            }
+            if (!grabbed.force()) {
+                dep.force = grabbed.force()
+            }
+            if (grabbed.classifier()) {
+                dep.classifier = grabbed.classifier()
+            }
+            if (grabbed.ext()) {
+                dep.ext = grabbed.ext()
+            }
+            if (grabbed.type()) {
+                dep.type = grabbed.type()
+            }
+            dep
+        }
+        results as Map[]
+    }
+
+    @Override
+    void addResolver(@NamedParams([
+        @NamedParam(value = 'name', type = String, required = true),
+        @NamedParam(value = 'root', type = String, required = true),
+        @NamedParam(value = 'm2Compatible', type = Boolean, required = false)
+    ]) Map<String, Object> args) {
+        String root = (String) (args.root ?: args.value)
+        if (!root) {
+            throw new RuntimeException("GrabResolver requires a root: or value: argument")
+        }
+        String name = (String) (args.name ?: "resolver-${repos.size()}")
+        RemoteRepository resolver = new RemoteRepository.Builder(name, "default", root).build()
+        // Keep parity with Ivy: most recently added resolver should be checked first.
+        repos.add(0, resolver)
+    }
+
+    @Override
+    void setLoggingLevel(int level) {
+        // Maven Resolver uses SLF4J for logging, which is configured externally
+        // We could potentially adjust logging here if needed, but for now
+        // Maven Resolver logging is controlled via SLF4J configuration
+        // Level mapping: 0=quiet/errors, 1=warn, 2=info, 3=verbose, 4=debug
+    }
+
+    static File getGroovyRoot() {
+        String root = System.getProperty('groovy.root')
+        def groovyRoot
+        if (root == null) {
+            groovyRoot = new File(System.getProperty('user.home'), '.groovy')
+        } else {
+            groovyRoot = new File(root)
+        }
+        try {
+            groovyRoot = groovyRoot.getCanonicalFile()
+        } catch (IOException ignore) {
+            // skip canonicalization then, it may not exist yet
+        }
+        groovyRoot
+    }
+
+    static File getGrapeDir() {
+        String root = System.getProperty('grape.root')
+        if (root == null) {
+            return getGroovyRoot()
+        }
+        File grapeRoot = new File(root)
+        try {
+            grapeRoot = grapeRoot.getCanonicalFile()
+        } catch (IOException ignore) {
+            // skip canonicalization then, it may not exist yet
+        }
+        grapeRoot
+    }
+
+    static File getGrapeCacheDir() {
+        File cache = new File(getGrapeDir(), 'grapesM2')
+        if (!cache.exists()) {
+            cache.mkdirs()
+        } else if (!cache.isDirectory()) {
+            throw new RuntimeException("The grape cache dir $cache is not a directory")
+        }
+        cache
+    }
+
+    private ClassLoader chooseClassLoader(Map args) {
+        ClassLoader loader = (ClassLoader) args.classLoader
+        if (!isValidTargetClassLoader(loader)) {
+            Class caller = args.refObject?.getClass() ?:
+                ReflectionUtils.getCallingClass((int) args.calleeDepth ?: 1)
+            loader = caller?.getClassLoader()
+            while (loader && !isValidTargetClassLoader(loader)) {
+                loader = loader.getParent()
+            }
+            if (!isValidTargetClassLoader(loader)) {
+                throw new RuntimeException('No suitable ClassLoader found for grab')
+            }
+        }
+        loader
+    }
+
+    private boolean isValidTargetClassLoader(ClassLoader loader) {
+        isValidTargetClassLoaderClass(loader?.getClass())
+    }
+
+    private boolean isValidTargetClassLoaderClass(Class loaderClass) {
+        loaderClass != null && (loaderClass.getName() == 'groovy.lang.GroovyClassLoader'
+            || loaderClass.getName() == 'org.codehaus.groovy.tools.RootLoader'
+            || isValidTargetClassLoaderClass(loaderClass.getSuperclass()))
+    }
+
+    private static String detectOsName() {
+        String osName = System.getProperty('os.name').toLowerCase()
+        if (osName.contains('win')) {
+            return 'windows'
+        } else if (osName.contains('mac')) {
+            return 'osx'
+        } else if (osName.contains('linux')) {
+            return 'linux'
+        } else if (osName.contains('sunos')) {
+            return 'sunos'
+        } else if (osName.contains('freebsd')) {
+            return 'freebsd'
+        }
+        // Default fallback
+        return osName.replaceAll('[^a-zA-Z0-9]', '')
+    }
+
+    private static String detectOsArch() {
+        String osArch = System.getProperty('os.arch').toLowerCase()
+        if (osArch.matches('.*64.*')) {
+            if (osArch.contains('aarch64') || osArch.contains('arm64')) {
+                return 'aarch_64'
+            }
+            return 'x86_64'
+        }
+        if (osArch.matches('.*86.*') || osArch == 'i386') {
+            return 'x86'
+        }
+        if (osArch.contains('ppc64')) {
+            return 'ppc64'
+        }
+        if (osArch.contains('ppc')) {
+            return 'ppc'
+        }
+        if (osArch.contains('s390')) {
+            return 's390'
+        }
+        if (osArch.contains('sparc')) {
+            return 'sparc'
+        }
+        // Default fallback
+        return osArch.replaceAll('[^a-zA-Z0-9]', '')
+    }
+
+    private boolean isMissingJarButPomExists(RepositorySystem system, RepositorySystemSession session, Artifact artifact) {
+        if (artifact == null) return false
+        // We only treat missing jar-like artifacts as ignorable if corresponding POM exists.
+        if (artifact.extension != 'jar') return false
+        try {
+            Artifact pomArtifact = new DefaultArtifact(artifact.groupId, artifact.artifactId, 'pom', artifact.version)
+            ArtifactRequest pomReq = new ArtifactRequest(pomArtifact, repos, null)
+            ArtifactResult pomRes = system.resolveArtifact(session, pomReq)
+            return pomRes?.resolved
+        } catch (Exception ignore) {
+            return false
+        }
+    }
+
+    private boolean isIgnorableUnresolvedArtifact(RepositorySystem system,
+                                                  RepositorySystemSession session,
+                                                  Artifact rootArtifact,
+                                                  Artifact missingArtifact) {
+        if (missingArtifact == null) return false
+        // Never ignore unresolved root artifact.
+        if (rootArtifact != null
+            && missingArtifact.groupId == rootArtifact.groupId
+            && missingArtifact.artifactId == rootArtifact.artifactId
+            && missingArtifact.version == rootArtifact.version) {
+            return false
+        }
+        // Ignore classifier/native variants that are often platform-specific/optional.
+        if (missingArtifact.classifier) return true
+        // Ignore POM-only coordinates requested as jars.
+        if (isMissingJarButPomExists(system, session, missingArtifact)) return true
+        // For transitive-only misses, keep Ivy-like lenient behavior and continue.
+        true
+    }

Review Comment:
   `isIgnorableUnresolvedArtifact` unconditionally returns `true` at line 648 as a fallback, meaning **all** unresolved transitive dependencies are silently ignored. This could mask real resolution failures (e.g., a genuine missing transitive dependency that the application needs at runtime), causing hard-to-diagnose `ClassNotFoundException`s later. Consider at minimum logging a warning when an unresolved transitive dependency is being ignored, or restricting silent ignoring to well-known patterns (like optional dependencies).



##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,670 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.graph.DependencyNode
+import org.eclipse.aether.internal.impl.scope.ScopeDependencySelector
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.resolution.DependencyResolutionException
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.util.artifact.JavaScopes
+import org.eclipse.aether.util.repository.SimpleArtifactDescriptorPolicy
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+    private static final boolean DEBUG_GRAB = Boolean.getBoolean('groovy.grape.debug')
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    Set<String> downloadedArtifacts = []
+    Set<String> resolvedDependencies = []
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw asRuntimeGrabError(e)
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw asRuntimeGrabError(e)
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {
+            def localRepo = new LocalRepository(grapeCacheDir.toPath())
+            String checksumPolicy = args.disableChecksums ?
+                RepositoryPolicy.CHECKSUM_POLICY_IGNORE :
+                RepositoryPolicy.CHECKSUM_POLICY_WARN
+
+            // Extract exclusions from args if provided by @GrabExclude
+            List<Map<String, String>> exclusions = (List<Map<String, String>>) args.get('excludes') ?: []
+
+            try (RepositorySystemSession.CloseableSession session = system
+                .createSessionBuilder()
+                .withLocalRepositories(localRepo)
+                .setChecksumPolicy(checksumPolicy)
+                .setDependencySelector(ScopeDependencySelector.legacy([JavaScopes.COMPILE, JavaScopes.RUNTIME], [JavaScopes.TEST]))
+                .setArtifactDescriptorPolicy(new SimpleArtifactDescriptorPolicy(true, true))
+                // Set Java version properties for profile evaluation
+                .setSystemProperty('java.version', System.getProperty('java.version'))
+                .setSystemProperty('java.home', System.getProperty('java.home'))
+                // Set OS detection properties (similar to os-maven-plugin)
+                .setSystemProperty('os.detected.name', detectOsName())
+                .setSystemProperty('os.detected.arch', detectOsArch())
+                .setSystemProperty('os.detected.classifier', "${detectOsName()}-${detectOsArch()}")
+                // Only the most essential configurations
+                .setSystemProperty('aether.artifactDescriptor.ignoreErrors', 'true')
+                .setConfigProperty('aether.artifactDescriptor.ignoreInvalidActivationExpressions', 'true')
+                .build()) {
+
+                List<URI> results = []
+
+                for (MavenGrabRecord grabRecord : grabRecords) {
+                    // Skip if this grab record is in the exclusion list
+                    if (isExcluded(grabRecord, exclusions)) {
+                        continue
+                    }
+
+                    String coords = "${grabRecord.groupId()}:${grabRecord.module()}"
+                    if (grabRecord.ext()) {
+                        coords += ":${grabRecord.ext()}"
+                    }
+                    if (grabRecord.classifier()) {
+                        coords += ":${grabRecord.classifier()}"
+                    }
+                    coords += ":${grabRecord.version()}"
+
+                    if (DEBUG_GRAB) {
+                        System.err.println("[GrapeDebug] resolving root=${coords} transitive=${grabRecord.transitive()}")
+                    }
+
+                    Artifact artifact = new DefaultArtifact(coords)
+
+                    String scope = grabRecord.conf()?.get(0) ?: JavaScopes.COMPILE
+                    if (scope == 'default') scope = JavaScopes.COMPILE
+
+                    List<ArtifactResult> artifactResults = []
+                    if (grabRecord.transitive()) {
+                        // Create a filter that prevents traversal into excluded artifacts
+                        DependencyFilter exclusionFilter = createExclusionFilter(exclusions)
+
+                        CollectRequest collectRequest = new CollectRequest(
+                            root: new Dependency(artifact, scope),
+                            repositories: repos
+                        )
+                        DependencyRequest dependencyRequest = new DependencyRequest(collectRequest, exclusionFilter)
+                        try {
+                            artifactResults = system.resolveDependencies(session, dependencyRequest).getArtifactResults()
+                        } catch (DependencyResolutionException dre) {
+                            // Keep resolved artifacts; tolerate unresolved non-root transitives.
+                            artifactResults = []
+                            if (dre.result?.artifactResults) {
+                                for (ArtifactResult ar : dre.result.artifactResults) {
+                                    if (ar.resolved) {
+                                        artifactResults << ar
+                                        continue
+                                    }
+                                    Artifact missing = ar?.request?.artifact
+                                    if (!isIgnorableUnresolvedArtifact(system, session, artifact, missing)) {
+                                        throw dre
+                                    }
+                                }
+                            } else {
+                                throw dre
+                            }
+                        }
+                    } else {
+                        // Non-transitive resolution
+                        ArtifactRequest artifactRequest = new ArtifactRequest(artifact, repos, null)
+                        artifactResults = [system.resolveArtifact(session, artifactRequest)]
+                    }
+
+                    for (ArtifactResult found : artifactResults) {
+                        if (found.artifact.file) {
+                            if (DEBUG_GRAB) {
+                                System.err.println("[GrapeDebug] resolved ${found.artifact.groupId}:${found.artifact.artifactId}:${found.artifact.extension}:${found.artifact.version} -> ${found.artifact.file}")
+                            }
+                            results << found.artifact.file.toURI()
+
+                            if (depsInfo != null) {
+                                depsInfo << [
+                                    'group': found.artifact.groupId,
+                                    'module': found.artifact.artifactId,
+                                    'revision': found.artifact.version
+                                ]
+                            }
+                        }
+                    }
+                }
+
+                return results as URI[]
+            }
+        }
+    }
+
+    private DependencyFilter createExclusionFilter(List<Map<String, String>> exclusions) {
+        if (!exclusions) {
+            return null  // No exclusions, no filtering needed
+        }
+
+        return { DependencyNode node, Collection parents ->
+            // Return false to prevent traversal into excluded artifacts
+            // This stops their dependencies from being collected
+            return !isExcludedArtifact(node.artifact.groupId, node.artifact.artifactId, exclusions)
+        } as DependencyFilter
+    }
+
+    private boolean isExcluded(MavenGrabRecord grabRecord, List<Map<String, String>> exclusions) {
+        return isExcludedArtifact(grabRecord.groupId(), grabRecord.module(), exclusions)
+    }
+
+    private boolean isExcludedArtifact(String groupId, String artifactId, List<Map<String, String>> exclusions) {
+        for (Map<String, String> exclusion : exclusions) {
+            String excludeGroup = exclusion.group ?: exclusion.groupId
+            String excludeModule = exclusion.module ?: exclusion.artifactId
+
+            if (excludeGroup && excludeModule) {
+                if (groupId == excludeGroup && artifactId == excludeModule) {
+                    return true  // This artifact is excluded
+                }
+            }
+        }
+        return false
+    }
+
+    MavenGrabRecord createGrabRecord(Map dep) {
+        String module = dep.module ?: dep.artifactId ?: dep.artifact
+        if (!module) {
+            throw new RuntimeException('grab requires at least a module: or artifactId: or artifact: argument')
+        }
+
+        // check for malformed components of the coordinates
+        dep.each { k, v ->
+            if (v instanceof CharSequence) {
+                if (k.toString().contains('v')) { // revision, version, rev
+                    if (!(v ==~ '[^\\\\/:"<>|]*')) {
+                        throw new RuntimeException("Grab: invalid value of '$v' for $k: should not contain any of / \\ : \" < > |")
+                    }
+                } else {
+                    if (!(v ==~ '[-._a-zA-Z0-9]*')) {
+                        throw new RuntimeException("Grab: invalid value of '$v' for $k: should only contain - . _ a-z A-Z 0-9")
+                    }
+                }
+            }
+        }
+
+        // check for mutually exclusive arguments
+        Set<String> keys = (Set<String>) dep.keySet()
+        keys.each { key ->
+            Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS[key]
+            if (badArgs && !badArgs.disjoint(keys)) {
+                throw new RuntimeException("Grab: mutually exclusive arguments: ${keys.intersect(badArgs) + key}")
+            }
+        }
+
+        String groupId = dep.group ?: dep.groupId ?: dep.organisation ?: dep.organization ?: dep.org ?: ''
+        String version = dep.version ?: dep.revision ?: dep.rev ?: '*'
+        if (version == '*') version = 'LATEST'
+        String classifier = dep.classifier ?: null
+        String ext = dep.ext ?: dep.type ?: 'jar'
+        String type = dep.type ?: ''
+
+        boolean force = dep.containsKey('force') ? dep.force : true
+        boolean changing = dep.containsKey('changing') ? dep.changing : false
+        boolean transitive = dep.containsKey('transitive') ? dep.transitive : true
+
+        new MavenGrabRecord(
+            groupId,
+            module,
+            version,
+            getConfList(dep),
+            ext,
+            type,
+            classifier,
+            force,
+            changing,
+            transitive
+        )
+    }
+
+    @CompileDynamic
+    private List<String> getConfList(Map dep) {
+        def conf = dep.conf ?: dep.scope ?: dep.configuration ?: DEFAULT_CONF
+        if (conf instanceof String) {
+            if (conf.startsWith('[') && conf.endsWith(']')) conf = conf[1..-2]
+            conf = conf.tokenize(',')
+        }
+        (conf as List<String>)
+    }
+
+    @Override
+    Map[] listDependencies(ClassLoader classLoader) {
+        List<? extends Map> results = loadedDeps[classLoader]?.collect { MavenGrabRecord grabbed ->
+            def dep = [
+                group  : grabbed.groupId(),
+                module : grabbed.module(),
+                version: grabbed.version()
+            ]
+            if (grabbed.conf() != DEFAULT_CONF) {
+                dep.conf = grabbed.conf()
+            }
+            if (grabbed.changing()) {
+                dep.changing = grabbed.changing()
+            }
+            if (!grabbed.transitive()) {
+                dep.transitive = grabbed.transitive()
+            }
+            if (!grabbed.force()) {
+                dep.force = grabbed.force()
+            }
+            if (grabbed.classifier()) {
+                dep.classifier = grabbed.classifier()
+            }
+            if (grabbed.ext()) {
+                dep.ext = grabbed.ext()
+            }
+            if (grabbed.type()) {
+                dep.type = grabbed.type()
+            }
+            dep
+        }
+        results as Map[]
+    }
+
+    @Override
+    void addResolver(@NamedParams([
+        @NamedParam(value = 'name', type = String, required = true),
+        @NamedParam(value = 'root', type = String, required = true),
+        @NamedParam(value = 'm2Compatible', type = Boolean, required = false)
+    ]) Map<String, Object> args) {
+        String root = (String) (args.root ?: args.value)
+        if (!root) {
+            throw new RuntimeException("GrabResolver requires a root: or value: argument")
+        }
+        String name = (String) (args.name ?: "resolver-${repos.size()}")
+        RemoteRepository resolver = new RemoteRepository.Builder(name, "default", root).build()
+        // Keep parity with Ivy: most recently added resolver should be checked first.
+        repos.add(0, resolver)
+    }
+
+    @Override
+    void setLoggingLevel(int level) {
+        // Maven Resolver uses SLF4J for logging, which is configured externally
+        // We could potentially adjust logging here if needed, but for now
+        // Maven Resolver logging is controlled via SLF4J configuration
+        // Level mapping: 0=quiet/errors, 1=warn, 2=info, 3=verbose, 4=debug
+    }
+
+    static File getGroovyRoot() {
+        String root = System.getProperty('groovy.root')
+        def groovyRoot
+        if (root == null) {
+            groovyRoot = new File(System.getProperty('user.home'), '.groovy')
+        } else {
+            groovyRoot = new File(root)
+        }
+        try {
+            groovyRoot = groovyRoot.getCanonicalFile()
+        } catch (IOException ignore) {
+            // skip canonicalization then, it may not exist yet
+        }
+        groovyRoot
+    }
+
+    static File getGrapeDir() {
+        String root = System.getProperty('grape.root')
+        if (root == null) {
+            return getGroovyRoot()
+        }
+        File grapeRoot = new File(root)
+        try {
+            grapeRoot = grapeRoot.getCanonicalFile()
+        } catch (IOException ignore) {
+            // skip canonicalization then, it may not exist yet
+        }
+        grapeRoot
+    }
+
+    static File getGrapeCacheDir() {
+        File cache = new File(getGrapeDir(), 'grapesM2')
+        if (!cache.exists()) {
+            cache.mkdirs()
+        } else if (!cache.isDirectory()) {
+            throw new RuntimeException("The grape cache dir $cache is not a directory")
+        }
+        cache
+    }
+
+    private ClassLoader chooseClassLoader(Map args) {
+        ClassLoader loader = (ClassLoader) args.classLoader
+        if (!isValidTargetClassLoader(loader)) {
+            Class caller = args.refObject?.getClass() ?:
+                ReflectionUtils.getCallingClass((int) args.calleeDepth ?: 1)
+            loader = caller?.getClassLoader()
+            while (loader && !isValidTargetClassLoader(loader)) {
+                loader = loader.getParent()
+            }
+            if (!isValidTargetClassLoader(loader)) {
+                throw new RuntimeException('No suitable ClassLoader found for grab')
+            }
+        }
+        loader
+    }
+
+    private boolean isValidTargetClassLoader(ClassLoader loader) {
+        isValidTargetClassLoaderClass(loader?.getClass())
+    }
+
+    private boolean isValidTargetClassLoaderClass(Class loaderClass) {
+        loaderClass != null && (loaderClass.getName() == 'groovy.lang.GroovyClassLoader'
+            || loaderClass.getName() == 'org.codehaus.groovy.tools.RootLoader'
+            || isValidTargetClassLoaderClass(loaderClass.getSuperclass()))
+    }
+
+    private static String detectOsName() {
+        String osName = System.getProperty('os.name').toLowerCase()
+        if (osName.contains('win')) {
+            return 'windows'
+        } else if (osName.contains('mac')) {
+            return 'osx'
+        } else if (osName.contains('linux')) {
+            return 'linux'
+        } else if (osName.contains('sunos')) {
+            return 'sunos'
+        } else if (osName.contains('freebsd')) {
+            return 'freebsd'
+        }
+        // Default fallback
+        return osName.replaceAll('[^a-zA-Z0-9]', '')
+    }
+
+    private static String detectOsArch() {
+        String osArch = System.getProperty('os.arch').toLowerCase()
+        if (osArch.matches('.*64.*')) {
+            if (osArch.contains('aarch64') || osArch.contains('arm64')) {
+                return 'aarch_64'
+            }
+            return 'x86_64'

Review Comment:
   Bug: The `detectOsArch()` method checks `osArch.matches('.*64.*')` first, which matches architectures like `ppc64le` and `s390x` (via potential '64' in the string). For `ppc64` and `ppc64le`, this first branch will match and return `x86_64` instead of falling through to the `ppc64` check on line 601. The `ppc64`, `s390`, and `sparc` checks further down are effectively unreachable for 64-bit variants of those architectures.
   
   The fix should be to check for `aarch64`/`arm64`/`ppc64`/`s390x` etc. before the generic `.*64.*` catch-all, or change the first check to be more specific (e.g., match `x86_64` or `amd64` explicitly).



##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/ConsoleIvyPlugin.groovy:
##########
@@ -36,29 +37,30 @@ class ConsoleIvyPlugin {
 
     def addListener(Console console) {
         savedConsole = console
-
-        ((GrapeIvy) Grape.instance).ivyInstance.eventManager.addIvyListener([progress: { ivyEvent ->
-            switch (ivyEvent) {
-                case StartResolveEvent:
-                    ivyEvent.moduleDescriptor.dependencies.each { it ->
-                        def name = it.toString()
-                        if (!resolvedDependencies.contains(name)) {
-                            resolvedDependencies << name
-                            savedConsole.showMessage "Resolving ${name} ..."
+        GrapeEngine engine = Grape.instance
+        if (engine instanceof GrapeIvy) {
+            ((GrapeIvy) Grape.instance).ivyInstance.eventManager.addIvyListener([progress: { ivyEvent ->

Review Comment:
   On line 40, the engine is already retrieved as `engine = Grape.instance`, but on line 42, `Grape.instance` is called again instead of using the local `engine` variable. This should use the already-retrieved `engine` variable for consistency and to avoid an unnecessary synchronized method call: `((GrapeIvy) engine).ivyInstance.eventManager...`



-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-11871: Support Maven Resolver based version of Grapes [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2140:
URL: https://github.com/apache/groovy/pull/2140#discussion_r2939635903


##########
src/main/groovy/groovy/grape/GrapeUtil.groovy:
##########
@@ -0,0 +1,175 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape
+
+import groovy.transform.CompileStatic
+import org.apache.groovy.plugin.GroovyRunner
+import org.apache.groovy.plugin.GroovyRunnerRegistry
+import org.codehaus.groovy.reflection.CachedClass
+import org.codehaus.groovy.reflection.ClassInfo
+import org.codehaus.groovy.runtime.m12n.ExtensionModuleScanner
+import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl
+
+import java.util.jar.JarFile
+import java.util.zip.ZipEntry
+import java.util.zip.ZipException
+import java.util.zip.ZipFile
+
+/**
+ * Utility methods shared between GrapeIvy and GrapeMaven implementations.
+ */
+@CompileStatic
+class GrapeUtil {
+
+    private static final String METAINF_PREFIX = 'META-INF/services/'
+    private static final String RUNNER_PROVIDER_CONFIG = GroovyRunner.name
+
+    /**
+     * Adds a URI to a classloader's classpath via reflection.
+     */
+    static void addURL(ClassLoader loader, URI uri) {
+        // Dynamic invocation needed as addURL is not part of ClassLoader interface
+        loader.metaClass.invokeMethod(loader, 'addURL', uri.toURL())
+    }
+
+    /**
+     * Processes and registers category methods (extension modules) from a JAR file.
+     *
+     * @param loader the classloader to register methods with
+     * @param file the JAR file to process
+     */
+    static void processCategoryMethods(ClassLoader loader, File file) {
+        // register extension methods if jar
+        if (file.getName().toLowerCase().endsWith('.jar')) {
+            def mcRegistry = GroovySystem.metaClassRegistry
+            if (mcRegistry instanceof MetaClassRegistryImpl) {
+                try (JarFile jar = new JarFile(file)) {
+                    ZipEntry entry = jar.getEntry(ExtensionModuleScanner.MODULE_META_INF_FILE)
+                    if (!entry) {
+                        entry = jar.getEntry(ExtensionModuleScanner.LEGACY_MODULE_META_INF_FILE)
+                    }
+                    if (entry) {
+                        Properties props = new Properties()
+
+                        try (InputStream is = jar.getInputStream(entry)) {
+                            props.load(is)
+                        }
+
+                        Map<CachedClass, List<MetaMethod>> metaMethods = [:]
+                        mcRegistry.registerExtensionModuleFromProperties(props, loader, metaMethods)
+                        // add old methods to the map
+                        metaMethods.each { CachedClass c, List<MetaMethod> methods ->
+                            // GROOVY-5543: if a module was loaded using grab, there are chances that subclasses
+                            // have their own ClassInfo, and we must change them as well!
+                            Set<CachedClass> classesToBeUpdated = [c].toSet()
+                            ClassInfo.onAllClassInfo { ClassInfo info ->
+                                if (c.getTheClass().isAssignableFrom(info.getCachedClass().getTheClass())) {
+                                    classesToBeUpdated << info.getCachedClass()
+                                }
+                            }
+                            classesToBeUpdated*.addNewMopMethods(methods)
+                        }
+                    }
+                } catch (ZipException e) {
+                    throw new RuntimeException("Grape could not load jar '$file'", e)
+                }
+            }
+        }
+    }
+
+    /**
+     * Searches the given File for known service provider configuration files to process.
+     *
+     * @param loader used to locate service provider files
+     * @param f ZipFile in which to search for services
+     * @return a collection of service provider files that were found
+     */
+    static Collection<String> processMetaInfServices(ClassLoader loader, File f) {
+        List<String> services = []
+        try (ZipFile zf = new ZipFile(f)) {
+            String providerConfig = 'org.codehaus.groovy.runtime.SerializedCategoryMethods'
+            ZipEntry serializedCategoryMethods = zf.getEntry(METAINF_PREFIX + providerConfig)
+            if (serializedCategoryMethods != null) {
+                services.add(providerConfig)
+
+                try (InputStream is = zf.getInputStream(serializedCategoryMethods)) {
+                    processSerializedCategoryMethods(is)
+                }
+            }
+            // TODO: remove in a future release (replaced by GroovyRunnerRegistry)
+            providerConfig = 'org.codehaus.groovy.plugins.Runners'
+            ZipEntry pluginRunners = zf.getEntry(METAINF_PREFIX + providerConfig)
+            if (pluginRunners != null) {
+                services.add(providerConfig)
+
+                try (InputStream is = zf.getInputStream(pluginRunners)) {
+                    processRunners(is, f.getName(), loader)
+                }
+            }
+            // GroovyRunners are loaded per ClassLoader using a ServiceLoader so here
+            // it only needs to be indicated that the service provider file was found
+            if (zf.getEntry(METAINF_PREFIX + RUNNER_PROVIDER_CONFIG) != null) {
+                services.add(RUNNER_PROVIDER_CONFIG)
+            }
+        } catch (ZipException ignore) {
+            // ignore files we can't process, e.g. non-jar/zip artifacts
+            // TODO: log a warning
+        }
+        services
+    }
+
+    /**
+     * Processes serialized category methods from an input stream.
+     *
+     * @param is the input stream containing category method definitions
+     */
+    static void processSerializedCategoryMethods(InputStream is) {
+        is.getText().readLines().each {
+            System.err.println(it.trim()) // TODO: implement this or delete it
+        }

Review Comment:
   `processSerializedCategoryMethods` currently prints each line to `System.err` with a TODO. Now that this code is shared by both Grape engines, it can introduce unexpected stderr noise whenever a grabbed JAR contains this service file. Consider either implementing the intended behavior or removing the stderr output (and/or gating it behind the existing `groovy.grape.debug` flag).



##########
subprojects/groovy-xml/src/main/groovy/groovy/xml/StaxBuilder.groovy:
##########
@@ -31,7 +31,10 @@ package groovy.xml
  *     elem2('hello2')
  *     elem3(x:7)
  * }
- * assert writer.toString() == '<?xml version="1.0" ?><root1 a="5" b="7"><elem1>hello1</elem1><elem2>hello2</elem2><elem3 x="7"></elem3></root1>'
+ * def pretty= writer.toString()
+ *     .replaceAll(/<\?xml[^>]*>/, '') // remove XML declaration
+ * assert pretty in ['<root1 a="5" b="7"><elem1>hello1</elem1><elem2>hello2</elem2><elem3 x="7"></elem3></root1>',
+ *                  '<root1 a="5" b="7"><elem1>hello1</elem1><elem2>hello2</elem2><elem3 x="7"/></root1>']

Review Comment:
   The documentation example removes the XML declaration but does not account for any whitespace left behind (e.g. a space/newline after `?>`). As written, `pretty` may start with whitespace and fail the `in [...]` assertion. Consider trimming `pretty` or removing trailing whitespace along with the declaration in the regex so the snippet is robust across StAX implementations.



##########
subprojects/groovy-xml/src/spec/test/StaxBuilderTest.groovy:
##########
@@ -32,7 +32,9 @@ class StaxBuilderTest extends GroovyTestCase {
             elem2('world')
         }
 
-        assert writer.toString() == '<?xml version="1.0" ?><root attribute="1"><elem1>hello</elem1><elem2>world</elem2></root>'
+        def pretty= writer.toString()
+            .replaceAll(/<\?xml[^>]*>/, '') // remove XML declaration
+        assert pretty == '<root attribute="1"><elem1>hello</elem1><elem2>world</elem2></root>'

Review Comment:
   Removing the XML declaration via `replaceAll(/<\?xml[^>]*>/, '')` can leave leading whitespace (e.g. the space after `?>` in the prior expected output), which will make the equality assertion fail. Consider also trimming the result or expanding the regex to remove surrounding whitespace with the declaration.



-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-11871: Support Maven Resolver based version of Grapes [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2140:
URL: https://github.com/apache/groovy/pull/2140#discussion_r2908783383


##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,501 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.util.artifact.JavaScopes
+import org.eclipse.aether.util.filter.DependencyFilterUtils
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    Set<String> downloadedArtifacts = []
+    Set<String> resolvedDependencies = []
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw e
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw e
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {
+            def localRepo = new LocalRepository(getGrapeCacheDir())
+            String checksumPolicy = args.disableChecksums ?
+                RepositoryPolicy.CHECKSUM_POLICY_IGNORE :
+                RepositoryPolicy.CHECKSUM_POLICY_WARN // Use WARN instead of FAIL for better compatibility
+
+            try (RepositorySystemSession.CloseableSession session = system
+                .createSessionBuilder()
+                .withLocalRepositories(localRepo)
+                .setChecksumPolicy(checksumPolicy)
+                .setSystemProperty('aether.artifactDescriptor.ignoreErrors', 'true')
+                .setConfigProperty('aether.artifactDescriptor.ignoreInvalidActivationExpressions', 'true')
+                .build()) {
+
+                List<URI> results = []
+
+                for (MavenGrabRecord grabRecord : grabRecords) {
+                    String coords = "${grabRecord.groupId()}:${grabRecord.module()}"
+                    if (grabRecord.classifier()) {
+                        coords += ":${grabRecord.classifier()}"
+                    }
+                    coords += ":${grabRecord.ext()}:${grabRecord.version()}"

Review Comment:
   The coordinate string is constructed incorrectly when a classifier is present. Maven Resolver's `DefaultArtifact` expects the 5-part format `groupId:artifactId:extension:classifier:version`, but this code produces `groupId:artifactId:classifier:extension:version` (classifier and extension are swapped).
   
   The classifier should be appended after the extension, not before it. The code should build the coords as `"${groupId}:${module}:${ext}:${classifier}:${version}"` when a classifier is present.



##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,501 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.util.artifact.JavaScopes
+import org.eclipse.aether.util.filter.DependencyFilterUtils
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    Set<String> downloadedArtifacts = []
+    Set<String> resolvedDependencies = []
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw e
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw e
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {
+            def localRepo = new LocalRepository(getGrapeCacheDir())
+            String checksumPolicy = args.disableChecksums ?
+                RepositoryPolicy.CHECKSUM_POLICY_IGNORE :
+                RepositoryPolicy.CHECKSUM_POLICY_WARN // Use WARN instead of FAIL for better compatibility
+
+            try (RepositorySystemSession.CloseableSession session = system
+                .createSessionBuilder()
+                .withLocalRepositories(localRepo)
+                .setChecksumPolicy(checksumPolicy)
+                .setSystemProperty('aether.artifactDescriptor.ignoreErrors', 'true')
+                .setConfigProperty('aether.artifactDescriptor.ignoreInvalidActivationExpressions', 'true')
+                .build()) {
+
+                List<URI> results = []
+
+                for (MavenGrabRecord grabRecord : grabRecords) {
+                    String coords = "${grabRecord.groupId()}:${grabRecord.module()}"
+                    if (grabRecord.classifier()) {
+                        coords += ":${grabRecord.classifier()}"
+                    }
+                    coords += ":${grabRecord.ext()}:${grabRecord.version()}"
+
+                    Artifact artifact = new DefaultArtifact(coords)
+
+                    String scope = grabRecord.conf()?.get(0) ?: JavaScopes.COMPILE
+                    if (scope == 'default') scope = JavaScopes.COMPILE
+
+                    List<ArtifactResult> artifactResults
+                    if (grabRecord.transitive()) {
+                        DependencyFilter classpathFilter = DependencyFilterUtils.classpathFilter(scope)
+                        CollectRequest collectRequest = new CollectRequest(
+                            root: new Dependency(artifact, scope),
+                            repositories: repos
+                        )
+                        DependencyRequest dependencyRequest = new DependencyRequest(collectRequest, classpathFilter)
+                        artifactResults = system.resolveDependencies(session, dependencyRequest).getArtifactResults()
+                    } else {
+                        ArtifactRequest artifactRequest = new ArtifactRequest(artifact, repos, null)
+                        artifactResults = [system.resolveArtifact(session, artifactRequest)]
+                    }
+
+                    for (ArtifactResult found : artifactResults) {
+                        if (found.artifact.file) {
+                            results << found.artifact.file.toURI()
+
+                            if (depsInfo != null) {
+                                depsInfo << [
+                                    'group': found.artifact.groupId,
+                                    'module': found.artifact.artifactId,
+                                    'version': found.artifact.version
+                                ]

Review Comment:
   The `depsInfo` map entries use the key `'version'` here, but `GrapeMain.Resolve` (line 299 of `GrapeMain.groovy`) accesses `dep.revision` for the Ivy-format output (`-i` flag). The existing `GrapeIvy` implementation uses `'revision'` as the key (see `GrapeIvy.groovy:551`). This should use `'revision'` instead of `'version'` to maintain compatibility with the existing `GrapeMain` consumer, or both keys could be included.



##########
src/main/java/groovy/grape/GrapeEngine.java:
##########
@@ -43,5 +43,11 @@ public interface GrapeEngine {
     Map[] listDependencies(ClassLoader classLoader);
 
     void addResolver(Map<String, Object> args);
+
+    /**
+     * Sets the logging level for the grape engine.
+     * @param level the logging level (0=quiet/errors only, 1=warn, 2=info, 3=verbose, 4=debug)
+     */
+    void setLoggingLevel(int level);

Review Comment:
   Adding `setLoggingLevel(int level)` as an abstract method to the `GrapeEngine` interface is a breaking change for any external implementations of this interface. Since this is a public API, consider providing a default (no-op) implementation to maintain backward compatibility:
   
   ```java
   default void setLoggingLevel(int level) { }
   ```
   
   This way, existing third-party `GrapeEngine` implementations won't be forced to update.



##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,501 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.util.artifact.JavaScopes
+import org.eclipse.aether.util.filter.DependencyFilterUtils
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    Set<String> downloadedArtifacts = []
+    Set<String> resolvedDependencies = []
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw e
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw e
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {
+            def localRepo = new LocalRepository(getGrapeCacheDir())
+            String checksumPolicy = args.disableChecksums ?
+                RepositoryPolicy.CHECKSUM_POLICY_IGNORE :
+                RepositoryPolicy.CHECKSUM_POLICY_WARN // Use WARN instead of FAIL for better compatibility
+
+            try (RepositorySystemSession.CloseableSession session = system
+                .createSessionBuilder()
+                .withLocalRepositories(localRepo)
+                .setChecksumPolicy(checksumPolicy)
+                .setSystemProperty('aether.artifactDescriptor.ignoreErrors', 'true')
+                .setConfigProperty('aether.artifactDescriptor.ignoreInvalidActivationExpressions', 'true')
+                .build()) {
+
+                List<URI> results = []
+
+                for (MavenGrabRecord grabRecord : grabRecords) {
+                    String coords = "${grabRecord.groupId()}:${grabRecord.module()}"
+                    if (grabRecord.classifier()) {
+                        coords += ":${grabRecord.classifier()}"
+                    }
+                    coords += ":${grabRecord.ext()}:${grabRecord.version()}"
+
+                    Artifact artifact = new DefaultArtifact(coords)
+
+                    String scope = grabRecord.conf()?.get(0) ?: JavaScopes.COMPILE
+                    if (scope == 'default') scope = JavaScopes.COMPILE
+
+                    List<ArtifactResult> artifactResults
+                    if (grabRecord.transitive()) {
+                        DependencyFilter classpathFilter = DependencyFilterUtils.classpathFilter(scope)
+                        CollectRequest collectRequest = new CollectRequest(
+                            root: new Dependency(artifact, scope),
+                            repositories: repos
+                        )
+                        DependencyRequest dependencyRequest = new DependencyRequest(collectRequest, classpathFilter)
+                        artifactResults = system.resolveDependencies(session, dependencyRequest).getArtifactResults()
+                    } else {
+                        ArtifactRequest artifactRequest = new ArtifactRequest(artifact, repos, null)
+                        artifactResults = [system.resolveArtifact(session, artifactRequest)]
+                    }
+
+                    for (ArtifactResult found : artifactResults) {
+                        if (found.artifact.file) {
+                            results << found.artifact.file.toURI()
+
+                            if (depsInfo != null) {
+                                depsInfo << [
+                                    'group': found.artifact.groupId,
+                                    'module': found.artifact.artifactId,
+                                    'version': found.artifact.version
+                                ]
+                            }
+                        }
+                    }
+                }
+
+                return results as URI[]
+            }
+        }
+    }
+
+    MavenGrabRecord createGrabRecord(Map dep) {
+        String module = dep.module ?: dep.artifactId ?: dep.artifact
+        if (!module) {
+            throw new RuntimeException('grab requires at least a module: or artifactId: or artifact: argument')
+        }
+
+        // check for malformed components of the coordinates
+        dep.each { k, v ->
+            if (v instanceof CharSequence) {
+                if (k.toString().contains('v')) { // revision, version, rev
+                    if (!(v ==~ '[^/:"<>|]*')) {
+                        throw new RuntimeException("Grab: invalid value of '$v' for $k: should not contain any of / \\ : \" < > |")

Review Comment:
   The version validation regex `'[^/:"<>|]*'` is missing the backslash character compared to the original `GrapeIvy` implementation which uses `'[^\\\\/:"<>|]*'` (at `GrapeIvy.groovy:201`). The error message at line 311 mentions `\\` but the regex doesn't actually reject it. The regex should be `'[^\\\\/:"<>|]*'` to match the GrapeIvy behavior.



##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,501 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.util.artifact.JavaScopes
+import org.eclipse.aether.util.filter.DependencyFilterUtils
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    Set<String> downloadedArtifacts = []
+    Set<String> resolvedDependencies = []
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw e
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw e
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {

Review Comment:
   A new `RepositorySystemSupplier().get()` (and hence a new `RepositorySystem`) is created on every call to `getDependencies`. The `RepositorySystemSupplier` bootstraps a significant amount of infrastructure (service locator, transport providers, etc.). Consider creating the `RepositorySystem` once (e.g., in the constructor or lazily) and reusing it across calls, similar to how `GrapeIvy` creates its `Ivy` instance once in its constructor.



-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-11871: Support Maven Resolver based version of Grapes [groovy]

Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert merged PR #2140:
URL: https://github.com/apache/groovy/pull/2140


-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] early draft [groovy]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2140:
URL: https://github.com/apache/groovy/pull/2140#issuecomment-4018649859

   ## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2140?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 `75.00000%` with `1 line` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 66.7494%. Comparing base ([`7d8349c`](https://app.codecov.io/gh/apache/groovy/commit/7d8349c4b70178b8a95c207ea4404f5d36a24e8e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0c1d84e`](https://app.codecov.io/gh/apache/groovy/commit/0c1d84eb26490ba0421f52ca4fa4811dc337ebdf?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2140?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [src/main/java/groovy/grape/Grape.java](https://app.codecov.io/gh/apache/groovy/pull/2140?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fgrape%2FGrape.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvZ3JhcGUvR3JhcGUuamF2YQ==) | 75.0000% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2140?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>
   
   
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/groovy/pull/2140/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2140        +/-   ##
   ==================================================
   - Coverage     66.7571%   66.7494%   -0.0077%     
   - Complexity      29856      29866        +10     
   ==================================================
     Files            1382       1382                
     Lines          116130     116109        -21     
     Branches        20471      20473         +2     
   ==================================================
   - Hits            77525      77502        -23     
   + Misses          32275      32274         -1     
   - Partials         6330       6333         +3     
   ```
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2140?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [src/main/java/groovy/grape/Grape.java](https://app.codecov.io/gh/apache/groovy/pull/2140?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fgrape%2FGrape.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvZ3JhcGUvR3JhcGUuamF2YQ==) | `29.7619% <75.0000%> (+1.3668%)` | :arrow_up: |
   
   ... and [12 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2140/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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-11871: Support Maven Resolver based version of Grapes [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2140:
URL: https://github.com/apache/groovy/pull/2140#discussion_r2917767202


##########
src/main/groovy/groovy/grape/GrapeUtil.groovy:
##########
@@ -0,0 +1,174 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape
+
+import groovy.transform.CompileStatic
+import org.apache.groovy.plugin.GroovyRunner
+import org.apache.groovy.plugin.GroovyRunnerRegistry
+import org.codehaus.groovy.reflection.CachedClass
+import org.codehaus.groovy.reflection.ClassInfo
+import org.codehaus.groovy.runtime.m12n.ExtensionModuleScanner
+import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl
+
+import java.util.jar.JarFile
+import java.util.zip.ZipEntry
+import java.util.zip.ZipException
+import java.util.zip.ZipFile
+
+/**
+ * Utility methods shared between GrapeIvy and GrapeMaven implementations.
+ */
+@CompileStatic
+class GrapeUtil {
+
+    private static final String METAINF_PREFIX = 'META-INF/services/'
+    private static final String RUNNER_PROVIDER_CONFIG = GroovyRunner.name
+
+    /**
+     * Adds a URI to a classloader's classpath via reflection.
+     */
+    static void addURL(ClassLoader loader, URI uri) {
+        // Dynamic invocation needed as addURL is not part of ClassLoader interface
+        loader.metaClass.invokeMethod(loader, 'addURL', uri.toURL())
+    }
+
+    /**
+     * Processes and registers category methods (extension modules) from a JAR file.
+     *
+     * @param loader the classloader to register methods with
+     * @param file the JAR file to process
+     */
+    static void processCategoryMethods(ClassLoader loader, File file) {
+        // register extension methods if jar
+        if (file.getName().toLowerCase().endsWith('.jar')) {
+            def mcRegistry = GroovySystem.metaClassRegistry
+            if (mcRegistry instanceof MetaClassRegistryImpl) {
+                try (JarFile jar = new JarFile(file)) {
+                    ZipEntry entry = jar.getEntry(ExtensionModuleScanner.MODULE_META_INF_FILE)
+                    if (!entry) {
+                        entry = jar.getEntry(ExtensionModuleScanner.LEGACY_MODULE_META_INF_FILE)
+                    }
+                    if (entry) {
+                        Properties props = new Properties()
+
+                        try (InputStream is = jar.getInputStream(entry)) {
+                            props.load(is)
+                        }
+
+                        Map<CachedClass, List<MetaMethod>> metaMethods = [:]
+                        mcRegistry.registerExtensionModuleFromProperties(props, loader, metaMethods)
+                        // add old methods to the map
+                        metaMethods.each { CachedClass c, List<MetaMethod> methods ->
+                            // GROOVY-5543: if a module was loaded using grab, there are chances that subclasses
+                            // have their own ClassInfo, and we must change them as well!
+                            Set<CachedClass> classesToBeUpdated = [c].toSet()
+                            ClassInfo.onAllClassInfo { ClassInfo info ->
+                                if (c.getTheClass().isAssignableFrom(info.getCachedClass().getTheClass())) {
+                                    classesToBeUpdated << info.getCachedClass()
+                                }
+                            }
+                            classesToBeUpdated*.addNewMopMethods(methods)
+                        }
+                    }
+                } catch (ZipException e) {
+                    throw new RuntimeException("Grape could not load jar '$file'", e)
+                }
+            }
+        }
+    }
+
+    /**
+     * Searches the given File for known service provider configuration files to process.
+     *
+     * @param loader used to locate service provider files
+     * @param f ZipFile in which to search for services
+     * @return a collection of service provider files that were found
+     */
+    static Collection<String> processMetaInfServices(ClassLoader loader, File f) {
+        List<String> services = []
+        try (ZipFile zf = new ZipFile(f)) {
+            String providerConfig = 'org.codehaus.groovy.runtime.SerializedCategoryMethods'
+            ZipEntry serializedCategoryMethods = zf.getEntry(METAINF_PREFIX + providerConfig)
+            if (serializedCategoryMethods != null) {
+                services.add(providerConfig)
+
+                try (InputStream is = zf.getInputStream(serializedCategoryMethods)) {
+                    processSerializedCategoryMethods(is)
+                }
+            }
+            // TODO: remove in a future release (replaced by GroovyRunnerRegistry)
+            providerConfig = 'org.codehaus.groovy.plugins.Runners'
+            ZipEntry pluginRunners = zf.getEntry(METAINF_PREFIX + providerConfig)
+            if (pluginRunners != null) {
+                services.add(providerConfig)
+
+                try (InputStream is = zf.getInputStream(pluginRunners)) {
+                    processRunners(is, f.getName(), loader)
+                }
+            }
+            // GroovyRunners are loaded per ClassLoader using a ServiceLoader so here
+            // it only needs to be indicated that the service provider file was found
+            if (zf.getEntry(METAINF_PREFIX + RUNNER_PROVIDER_CONFIG) != null) {
+                services.add(RUNNER_PROVIDER_CONFIG)
+            }
+        } catch (ZipException ignore) {
+            // ignore files we can't process, e.g. non-jar/zip artifacts
+            // TODO: log a warning
+        }
+        services
+    }
+
+    /**
+     * Processes serialized category methods from an input stream.
+     *
+     * @param is the input stream containing category method definitions
+     */
+    static void processSerializedCategoryMethods(InputStream is) {
+        is.getText().readLines().each {
+            System.err.println(it.trim()) // TODO: implement this or delete it
+        }
+    }
+
+    /**
+     * Processes and registers Groovy runner implementations from a service provider file.
+     *
+     * @param is the input stream containing runner class names
+     * @param name the name to register the runners under
+     * @param loader the classloader to load runner classes from
+     */
+    static void processRunners(InputStream is, String name, ClassLoader loader) {
+        GroovyRunnerRegistry registry = GroovyRunnerRegistry.instance
+        is.getText().readLines()*.trim().each { String line ->
+            if (!line.isEmpty() && line[0] != '#')
+            try {
+                registry[name] = (GroovyRunner) loader.loadClass(line).getDeclaredConstructor().newInstance()
+            } catch (Exception e) {
+                throw new IllegalStateException("Error registering runner class '$line'", e)

Review Comment:
   `processRunners` has an `if (!line.isEmpty() && line[0] != '#')` with no braces/body, so the subsequent `try { ... }` runs for *every* line, including empty/comment lines. This will attempt to load invalid class names and can break runner discovery. Wrap the `try` block inside the `if` (or add `continue` when the condition fails).
   ```suggestion
               if (!line.isEmpty() && line[0] != '#') {
                   try {
                       registry[name] = (GroovyRunner) loader.loadClass(line).getDeclaredConstructor().newInstance()
                   } catch (Exception e) {
                       throw new IllegalStateException("Error registering runner class '$line'", e)
                   }
   ```



##########
subprojects/groovy-xml/src/main/groovy/groovy/xml/StaxBuilder.groovy:
##########
@@ -31,7 +31,10 @@ package groovy.xml
  *     elem2('hello2')
  *     elem3(x:7)
  * }
- * assert writer.toString() == '<?xml version="1.0" ?><root1 a="5" b="7"><elem1>hello1</elem1><elem2>hello2</elem2><elem3 x="7"></elem3></root1>'
+ * def pretty= writer.toString()
+ *     .replaceAll(/<\?xml[^>]*>/, '') // remove XML declaration

Review Comment:
   The doc example removes the XML declaration using `.replaceAll(/<\?xml[^>]*>/, '')` but (as shown in the old asserted output) implementations may emit a trailing space after the declaration. That leaves a leading space before `<root1...>` and makes the sample assertion brittle. Consider stripping the declaration plus any following whitespace (or trimming) in the example as well.
   ```suggestion
    *     .replaceAll(/<\?xml[^>]*>\s*/, '') // remove XML declaration and any following whitespace
   ```



##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/Console.groovy:
##########
@@ -1242,12 +1249,7 @@ class Console implements CaretListener, HyperlinkListener, ComponentListener, Fo
 
     // actually run the script
     void runScript(EventObject evt = null, SourceType st = new GroovySourceType()) {

Review Comment:
   `runScript` no longer calls `saveInputAreaContentHash()`. `runSelectedScript` still does, so this looks like an unintended regression from the previous implementation, and may break dirty-state tracking or other logic relying on the hash being updated whenever a script is run. Keep the ternary if desired, but preserve the `saveInputAreaContentHash()` call at the start of `runScript`.
   ```suggestion
       void runScript(EventObject evt = null, SourceType st = new GroovySourceType()) {
           saveInputAreaContentHash()
   ```



##########
subprojects/groovy-xml/src/spec/test/StaxBuilderTest.groovy:
##########
@@ -32,7 +32,9 @@ class StaxBuilderTest extends GroovyTestCase {
             elem2('world')
         }
 
-        assert writer.toString() == '<?xml version="1.0" ?><root attribute="1"><elem1>hello</elem1><elem2>world</elem2></root>'
+        def pretty= writer.toString()
+            .replaceAll(/<\?xml[^>]*>/, '') // remove XML declaration

Review Comment:
   `writer.toString()` previously included a space after the XML declaration (`<?xml ... ?> <root...>`). After `.replaceAll(/<\?xml[^>]*>/, '')`, that leading space will remain, so `pretty` is likely `' <root...` and this assertion will fail depending on the StAX writer implementation. Consider removing the declaration *and* any adjacent whitespace (e.g., include `\s*` in the regex or trim the result) before comparing.
   ```suggestion
               .replaceAll(/<\?xml[^>]*>\s*/, '') // remove XML declaration and any following whitespace
   ```



##########
subprojects/groovy-grape-maven/src/test/groovy/groovy/grape/maven/GrapeMavenTest.groovy:
##########
@@ -0,0 +1,165 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.Grape
+import org.junit.jupiter.api.AfterAll
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.Test
+
+import java.nio.file.Files
+import java.util.jar.JarOutputStream
+
+final class GrapeMavenTest {
+    private static File writeEmptyJar(File jarFile) {
+        jarFile.parentFile.mkdirs()
+        jarFile.withOutputStream { os ->
+            new JarOutputStream(os).close()
+        }
+        jarFile
+    }
+
+    private static void deleteCachedGroup(String groupId) {
+        File cachedGroupDir = new File(GrapeMaven.grapeCacheDir, groupId.replace('.' as char, File.separatorChar))
+        if (cachedGroupDir.exists()) {
+            cachedGroupDir.deleteDir()
+        }
+    }
+
+    private static File writePom(File pomFile, String groupId, String artifactId, String version, List<Map<String, Object>> deps = []) {
+        pomFile.parentFile.mkdirs()
+        String depsXml = deps.collect { Map<String, Object> dep ->
+            String optionalXml = dep.optional ? '\n      <optional>true</optional>' : ''
+            """\
+    <dependency>
+      <groupId>${dep.groupId}</groupId>
+      <artifactId>${dep.artifactId}</artifactId>
+      <version>${dep.version}</version>
+      <scope>compile</scope>${optionalXml}
+    </dependency>""".stripIndent()
+        }.join('\n')
+
+        pomFile.text = (
+"""<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>${groupId}</groupId>
+  <artifactId>${artifactId}</artifactId>
+  <version>${version}</version>
+  <packaging>jar</packaging>
+  <dependencies>
+${depsXml}
+  </dependencies>
+</project>
+"""
+        )
+        pomFile
+    }
+
+    private static void publishArtifact(File repoDir, String groupId, String artifactId, String version, List<Map<String, Object>> deps = []) {
+        String relPath = groupId.replace('.', '/') + "/${artifactId}/${version}"
+        File artifactDir = new File(repoDir, relPath)
+        writePom(new File(artifactDir, "${artifactId}-${version}.pom"), groupId, artifactId, version, deps)
+        writeEmptyJar(new File(artifactDir, "${artifactId}-${version}.jar"))
+    }
+
+    private static Set<String> jarNames(GroovyClassLoader loader) {
+        loader.URLs.collect { url -> url.path.split('/')[-1] } as Set
+    }
+
+    @BeforeAll
+    static void setUpClass() {
+        System.setProperty('groovy.grape.impl', 'groovy.grape.maven.GrapeMaven')
+        Grape.reset()
+    }
+
+    @AfterAll
+    static void cleanup() {
+        Grape.reset()
+    }

Review Comment:
   This test sets the global `groovy.grape.impl` system property in `@BeforeAll` but `@AfterAll` only calls `Grape.reset()` and never restores/clears the property. That can leak into subsequent tests and change which engine they run against. Capture the previous property value in `setUpClass()` and restore it (or clear it) in `cleanup()`.



##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeMaven.groovy:
##########
@@ -0,0 +1,747 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape.maven
+
+import groovy.grape.GrapeEngine
+import groovy.grape.GrapeUtil
+import groovy.transform.AutoFinal
+import groovy.transform.CompileDynamic
+import groovy.transform.CompileStatic
+import groovy.transform.NamedParam
+import groovy.transform.NamedParams
+import org.codehaus.groovy.reflection.ReflectionUtils
+import org.eclipse.aether.AbstractRepositoryListener
+import org.eclipse.aether.RepositoryEvent
+import org.eclipse.aether.RepositorySystem
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.artifact.Artifact
+import org.eclipse.aether.artifact.DefaultArtifact
+import org.eclipse.aether.collection.CollectRequest
+import org.eclipse.aether.graph.Dependency
+import org.eclipse.aether.graph.DependencyFilter
+import org.eclipse.aether.graph.DependencyNode
+import org.eclipse.aether.internal.impl.scope.OptionalDependencySelector
+import org.eclipse.aether.internal.impl.scope.ScopeDependencySelector
+import org.eclipse.aether.repository.LocalRepository
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.resolution.ArtifactRequest
+import org.eclipse.aether.resolution.ArtifactResult
+import org.eclipse.aether.resolution.DependencyRequest
+import org.eclipse.aether.resolution.DependencyResolutionException
+import org.eclipse.aether.supplier.RepositorySystemSupplier
+import org.eclipse.aether.transfer.AbstractTransferListener
+import org.eclipse.aether.transfer.TransferEvent
+import org.eclipse.aether.util.graph.selector.AndDependencySelector
+import org.eclipse.aether.util.repository.SimpleArtifactDescriptorPolicy
+
+import java.util.concurrent.CopyOnWriteArrayList
+
+/**
+ * Implementation supporting {@code @Grape} and {@code @Grab} annotations based on Maven.
+ */
+@AutoFinal
+@CompileStatic
+class GrapeMaven implements GrapeEngine {
+    private static final List<String> DEFAULT_CONF = Collections.singletonList('default')
+    private static final Map<String, Set<String>> MUTUALLY_EXCLUSIVE_KEYS = processGrabArgs([
+            ['group', 'groupId', 'organisation', 'organization', 'org'],
+            ['module', 'artifactId', 'artifact'],
+            ['version', 'revision', 'rev'],
+            ['conf', 'scope', 'configuration'],
+    ])
+    private static final boolean DEBUG_GRAB = Boolean.getBoolean('groovy.grape.debug')
+
+    @CompileDynamic // maps a->[b,c], b->[a,c] and c->[a,b] given [a,b,c]
+    private static Map<String, Set<String>> processGrabArgs(List<List<String>> grabArgs) {
+        grabArgs.inject([:]) { Map m, List g -> g.each { a -> m[a] = (g - a) as Set }; m }
+    }
+
+    // weak hash map so we don't leak loaders directly
+    final Map<ClassLoader, Set<MavenGrabRecord>> loadedDeps = [] as WeakHashMap
+    /** Stores the MavenGrabRecord(s) for all dependencies in each grab() call. */
+    final Set<MavenGrabRecord> grabRecordsForCurrDependencies = [] as LinkedHashSet
+    boolean enableGrapes = true
+    final List<Closure> progressListeners = new CopyOnWriteArrayList<>()
+    final List<RemoteRepository> repos = [
+        new RemoteRepository.Builder("central", "default", "https://repo.maven.apache.org/maven2/").build()
+    ]
+
+    @CompileDynamic
+    void addProgressListener(Closure listener) {
+        if (listener != null) {
+            progressListeners.add(listener)
+        }
+    }
+
+    void removeProgressListener(Closure listener) {
+        progressListeners.remove(listener)
+    }
+
+    private boolean hasProgressListeners() {
+        !progressListeners.isEmpty()
+    }
+
+    @CompileDynamic
+    private void fireProgressEvent(String type, String name) {
+        if (!name) return
+        Map<String, String> event = [type: type, name: name]
+        progressListeners.each { Closure listener ->
+            listener.call(event)
+        }
+    }
+
+    private AbstractRepositoryListener createRepositoryListener() {
+        new AbstractRepositoryListener() {
+            @Override
+            void artifactResolving(RepositoryEvent event) {
+                Artifact artifact = event?.artifact
+                if (artifact != null && artifact.extension != 'pom') {
+                    fireProgressEvent('resolving', artifact.toString())
+                }
+            }
+        }
+    }
+
+    private AbstractTransferListener createTransferListener() {
+        new AbstractTransferListener() {
+            @Override
+            void transferInitiated(TransferEvent event) {
+                String resourceName = event?.resource?.resourceName
+                String displayName = displayDownloadName(resourceName)
+                if (displayName != null) {
+                    fireProgressEvent('downloading', displayName)
+                }
+            }
+        }
+    }
+
+    private static String displayDownloadName(String resourceName) {
+        if (!resourceName) return null
+        String fileName = resourceName.tokenize('/').last()
+        if (!fileName
+            || fileName.startsWith('maven-metadata')
+            || fileName.endsWith('.pom')
+            || fileName.endsWith('.sha1')
+            || fileName.endsWith('.md5')
+            || fileName.endsWith('.asc')) {
+            return null
+        }
+        fileName
+    }
+
+    /**
+     * Grab the endorsed module for the current Groovy version.
+     */
+    @Override
+    grab(String endorsedModule) {
+        grab(group: 'groovy.endorsed', module: endorsedModule, version: GroovySystem.getVersion())
+    }
+
+    @Override
+    grab(Map args) {
+        args.calleeDepth = args.calleeDepth ?: DEFAULT_CALLEE_DEPTH + 1
+        grab(args, args)
+    }
+
+    @Override
+    grab(Map args, Map... dependencies) {
+        ClassLoader loader = null
+        grabRecordsForCurrDependencies.clear()
+
+        try {
+            // identify the target classloader early, so we fail before checking repositories
+            loader = chooseClassLoader(
+                    refObject: args.remove('refObject'),
+                    classLoader: args.remove('classLoader'),
+                    calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+            )
+
+            // check for non-fail null
+            // if we were in fail mode we would have already thrown an exception
+            if (!loader) return
+
+            URI[] uris = resolve(loader, args, dependencies)
+            for (URI uri : uris) {
+                GrapeUtil.addURL(loader, uri)
+            }
+            boolean runnerServicesFound = false
+            for (URI uri : uris) {
+                // TODO: check artifact type, jar vs library, etc.
+                File file = new File(uri)
+                GrapeUtil.processCategoryMethods(loader, file)
+                Collection<String> services = GrapeUtil.processMetaInfServices(loader, file)
+                if (!runnerServicesFound) {
+                    runnerServicesFound = GrapeUtil.checkForRunner(services)
+                }
+            }
+            if (runnerServicesFound) {
+                GrapeUtil.registryLoad(loader)
+            }
+        } catch (Exception e) {
+            // clean-up the state first
+            Set<MavenGrabRecord> grabRecordsForCurrLoader = getLoadedDepsForLoader(loader)
+            grabRecordsForCurrLoader.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+
+            if (args.noExceptions) {
+                return e
+            }
+            throw asRuntimeGrabError(e)
+        }
+        null
+    }
+
+
+    @Override
+    @CompileDynamic
+    Map<String, Map<String, List<String>>> enumerateGrapes() {
+        Map<String, Map<String, List<String>>> bunches = [:]
+        File cacheRoot = grapeCacheDir.canonicalFile
+
+        cacheRoot.eachFileRecurse { File f ->
+            if (!f.isFile()) return
+            String name = f.name
+            if (name.endsWith('.sha1') || name.endsWith('.md5') || name.endsWith('.asc')) return
+
+            File versionDir = f.parentFile
+            File moduleDir = versionDir?.parentFile
+            File groupDir = moduleDir?.parentFile
+            if (!versionDir || !moduleDir || !groupDir) return
+
+            String version = versionDir.name
+            String module = moduleDir.name
+            String expectedPrefix = module + '-' + version
+            if (!name.startsWith(expectedPrefix)) return
+
+            String groupPath = cacheRoot.toPath().relativize(groupDir.toPath()).toString()
+            if (!groupPath) return
+            String groupKey = groupPath.replace(File.separatorChar, '.' as char)
+
+            Map<String, List<String>> modules = bunches.computeIfAbsent(groupKey) { [:] }
+            List<String> versions = modules.computeIfAbsent(module) { [] }
+            if (!versions.contains(version)) {
+                versions << version
+            }
+        }
+
+        bunches.values()*.values()*.each { List<String> versions -> versions.sort() }
+        bunches
+    }
+
+    void uninstallArtifact(String group, String module, String rev) {
+        String groupPath = group.replace('.' as char, File.separatorChar)
+        def artifactDir = new File(grapeCacheDir, groupPath + File.separator + module + File.separator + rev)
+        if (artifactDir.exists()) {
+            artifactDir.deleteDir()
+        }
+    }
+
+    @Override
+    URI[] resolve(Map args, Map... dependencies) {
+        resolve(args, null, dependencies)
+    }
+
+    @Override
+    URI[] resolve(Map args, List depsInfo, Map... dependencies) {
+        // identify the target classloader early, so we fail before checking repositories
+        ClassLoader loader = chooseClassLoader(
+            refObject: args.remove('refObject'),
+            classLoader: args.remove('classLoader'),
+            calleeDepth: args.calleeDepth ?: DEFAULT_CALLEE_DEPTH,
+        )
+
+        // check for non-fail null
+        // if we were in fail mode we would have already thrown an exception
+        if (!loader) {
+            return new URI[0]
+        }
+
+        resolve(loader, args, depsInfo, dependencies)
+    }
+
+    private Set<MavenGrabRecord> getLoadedDepsForLoader(ClassLoader loader) {
+        // use a LinkedHashSet to preserve the initial insertion order
+        loadedDeps.computeIfAbsent(loader, k -> [] as LinkedHashSet)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, Map... dependencies) {
+        resolve(loader, args, null, dependencies)
+    }
+
+    URI[] resolve(ClassLoader loader, Map args, List depsInfo, Map... dependencies) {
+        if (!enableGrapes) {
+            return new URI[0]
+        }
+
+        boolean populateDepsInfo = (depsInfo != null)
+        Set<MavenGrabRecord> localDeps = getLoadedDepsForLoader(loader)
+        List<MavenGrabRecord> grabRecords = []
+        for (Map dep : dependencies) {
+             MavenGrabRecord mgr = createGrabRecord(dep)
+             grabRecordsForCurrDependencies.add(mgr)
+             localDeps.add(mgr)
+             grabRecords.add(mgr)
+        }
+
+        try {
+            URI[] results = getDependencies(args, populateDepsInfo ? depsInfo : null, grabRecords as MavenGrabRecord[])
+            return results
+        } catch (Exception e) {
+            localDeps.removeAll(grabRecordsForCurrDependencies)
+            grabRecordsForCurrDependencies.clear()
+            throw asRuntimeGrabError(e)
+        }
+    }
+
+    URI[] getDependencies(Map args, List depsInfo, MavenGrabRecord... grabRecords) {
+        try (RepositorySystem system = new RepositorySystemSupplier().get()) {
+            def localRepo = new LocalRepository(grapeCacheDir.toPath())
+            String checksumPolicy = args.disableChecksums ?

Review Comment:
   `try (RepositorySystem system = new RepositorySystemSupplier().get())` uses try-with-resources on `org.eclipse.aether.RepositorySystem`, which (in Maven Resolver/Aether) is not an `AutoCloseable` resource. Under `@CompileStatic` this is very likely to fail compilation. Use a plain local variable for the repository system (and keep the session as the closable resource), or otherwise manage the actual closeable object returned by the supplier if one exists.



-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GROOVY-11871: Support Maven Resolver based version of Grapes [groovy]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2140:
URL: https://github.com/apache/groovy/pull/2140#discussion_r2921432290


##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/Console.groovy:
##########
@@ -378,6 +378,13 @@ class Console implements CaretListener, HyperlinkListener, ComponentListener, Fo
         } catch (ClassNotFoundException ignore) {
         }
 
+        // listen for Maven resolver events if the Maven console plugin is on the classpath
+        try {
+            def mavenPluginClass = Class.forName('groovy.console.ui.ConsoleMavenPlugin')
+            mavenPluginClass.getConstructor().newInstance().addListener(this)
+        } catch (ClassNotFoundException ignore) {

Review Comment:
   The Maven console plugin is loaded reflectively but the code only catches `ClassNotFoundException`. If the class exists but fails to instantiate (e.g. constructor/initializer throws, missing dependencies, `ReflectiveOperationException`), Console startup will fail. Consider catching broader reflection exceptions here (similar to how optional integrations are typically guarded) and either ignoring or reporting a concise message.
   ```suggestion
           } catch (ReflectiveOperationException ignore) {
   ```



##########
versions.properties:
##########
@@ -48,6 +48,8 @@ junit5Platform=1.14.3
 log4j=1.2.17
 log4j2=2.25.3
 logback=1.5.27
+mavenResolverProvider=4.0.0-rc-5
+mavenResolverSupplier=2.0.16

Review Comment:
   The PR description says "Just a placeholder at the moment", but this PR introduces a new Maven-based Grape engine subproject plus CLI/console/test changes. Please update the PR description to reflect the implemented scope and any remaining TODOs so reviewers know what's intended for this change set.



##########
src/main/java/groovy/grape/Grape.java:
##########
@@ -119,9 +121,12 @@ public static void setDisableChecksums(boolean disableChecksums) {
     public static synchronized GrapeEngine getInstance() {
         if (instance == null) {
             try {
-                // by default use GrapeIvy
+                String grapeClass = System.getProperty("groovy.grape.impl");
+                if (grapeClass == null) {
+                    grapeClass = "groovy.grape.GrapeIvy"; // by default use GrapeIvy
+                }
                 // TODO: META-INF/services resolver?
-                instance = (GrapeEngine) Class.forName("groovy.grape.GrapeIvy").getDeclaredConstructor().newInstance();
+                instance = (GrapeEngine) Class.forName(grapeClass).getDeclaredConstructor().newInstance();
             } catch (ReflectiveOperationException ignore) {
             }

Review Comment:
   `getInstance()` now instantiates the engine from the `groovy.grape.impl` system property, but any `ReflectiveOperationException` is silently swallowed, leaving `instance` as `null` and effectively disabling Grapes with no diagnostics. Consider falling back to the default engine (GrapeIvy) and/or throwing an `IllegalStateException` (or at least logging to stderr) so misconfiguration is discoverable.



##########
src/main/groovy/groovy/grape/GrapeUtil.groovy:
##########
@@ -0,0 +1,175 @@
+/*
+ *  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
+ *
+ *    http://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 groovy.grape
+
+import groovy.transform.CompileStatic
+import org.apache.groovy.plugin.GroovyRunner
+import org.apache.groovy.plugin.GroovyRunnerRegistry
+import org.codehaus.groovy.reflection.CachedClass
+import org.codehaus.groovy.reflection.ClassInfo
+import org.codehaus.groovy.runtime.m12n.ExtensionModuleScanner
+import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl
+
+import java.util.jar.JarFile
+import java.util.zip.ZipEntry
+import java.util.zip.ZipException
+import java.util.zip.ZipFile
+
+/**
+ * Utility methods shared between GrapeIvy and GrapeMaven implementations.
+ */
+@CompileStatic
+class GrapeUtil {
+
+    private static final String METAINF_PREFIX = 'META-INF/services/'
+    private static final String RUNNER_PROVIDER_CONFIG = GroovyRunner.name
+
+    /**
+     * Adds a URI to a classloader's classpath via reflection.
+     */
+    static void addURL(ClassLoader loader, URI uri) {
+        // Dynamic invocation needed as addURL is not part of ClassLoader interface
+        loader.metaClass.invokeMethod(loader, 'addURL', uri.toURL())
+    }

Review Comment:
   `GrapeUtil` is `@CompileStatic` but `addURL` uses `loader.metaClass.invokeMethod(...)`, which relies on Groovy's dynamic metaClass property and is likely to fail static compilation. Consider marking `addURL` as `@CompileDynamic` (as the old Ivy implementation did) or using a statically-compilable reflective call (e.g., via `InvokerHelper`/reflection) to invoke `addURL`.



##########
src/test/groovy/org/codehaus/groovy/runtime/m12n/ExtensionModuleHelperForTests.groovy:
##########
@@ -53,7 +53,8 @@ final class ExtensionModuleHelperForTests {
         def ant = new AntBuilder()
         def allowed = [
             'Picked up JAVA_TOOL_OPTIONS: .*',
-            'Picked up _JAVA_OPTIONS: .*'
+            'Picked up _JAVA_OPTIONS: .*',
+            '(?s)SLF4J\\(W\\): Class path contains multiple SLF4J providers\\..*SLF4J\\(I\\): Actual provider is of type \\[[^\\n]+\\]'
         ]

Review Comment:
   This test now whitelists the SLF4J "multiple providers" warning rather than preventing the duplicate providers from appearing on the forked classpath. If possible, prefer fixing the underlying classpath/provider selection (so warnings don't leak into user output) instead of accepting the warning as normal.



##########
subprojects/groovy-grape-maven/build.gradle:
##########
@@ -0,0 +1,28 @@
+/*
+ *  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
+ *
+ *    http://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.groovy-library'
+}
+
+dependencies {
+    api rootProject
+    implementation "org.apache.maven:maven-resolver-provider:${versions.mavenResolverProvider}"
+    implementation "org.apache.maven.resolver:maven-resolver-supplier-mvn4:${versions.mavenResolverSupplier}"
+    runtimeOnly "org.slf4j:slf4j-simple:${versions.slf4j}"

Review Comment:
   Declaring `slf4j-simple` as `runtimeOnly` makes this library bring its own SLF4J provider, which can cause "multiple SLF4J providers" warnings and override an application's chosen logging backend. Consider making the backend a test-only dependency (e.g. `testRuntimeOnly`) or leaving provider selection to the consuming application.
   ```suggestion
       testRuntimeOnly "org.slf4j:slf4j-simple:${versions.slf4j}"
   ```



-- 
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@groovy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org