Merge pull request #24280 Run dependency actions on entire hierarchy

Make sure we run the dependency actions on the entire hierarchy before traversing that hierarchy for dependencies and excludes. This is because the dependency actions may themselves modify the hierarchy, so before traversing we need to make sure the hierarchy isn't going to change out from under us.

This should be fine since configurations' dependency actions are idempotent. The actions automatically de-register themselves, so once they're called, they're not triggered for subsequent calls.

Fixes: #24234

Co-authored-by: Justin Van Dort <jvandort@gradle.com>

subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/api/ExtendingConfigurationsIntegrationTest.groovy

@@ -135,4 +135,46 @@ task checkResolveParentThenChild {
         then:
         output.contains("[foo-1.0.jar]\n[foo-1.0.jar]")
     }
+
+    @Issue("https://github.com/gradle/gradle/issues/24234")
+    def "configuration extensions can be changed in withDependencies during resolution"() {
+        given:
+        mavenRepo.module("org", "foo").publish()
+        buildFile << """
+            def attr = Attribute.of('org.example.attr', String)
+
+            repositories {
+                maven { url "${mavenRepo.uri}" }
+            }
+
+            configurations {
+                parentConf
+                depConf {
+                    attributes {
+                        attribute(attr, 'pick-me')
+                    }
+                    withDependencies {
+                        depConf.extendsFrom(parentConf)
+                    }
+                }
+                conf
+            }
+
+            dependencies {
+                conf(project(':')) {
+                    attributes {
+                        attribute(attr, 'pick-me')
+                    }
+                }
+                parentConf 'org:foo:1.0'
+            }
+
+            task resolve {
+                assert configurations.conf.files.collect { it.name } == ["foo-1.0.jar"]
+            }
+        """
+
+        expect:
+        succeeds("resolve")
+    }
 }

subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/moduleconverter/dependencies/DefaultLocalConfigurationMetadataBuilder.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
+import org.gradle.api.artifacts.Configuration;
 import org.gradle.api.artifacts.Dependency;
 import org.gradle.api.artifacts.DependencyConstraint;
 import org.gradle.api.artifacts.ExcludeRule;
@@ -50,7 +51,11 @@
 import org.gradle.internal.model.ModelContainer;
 
 import javax.annotation.Nullable;
+import java.util.ArrayDeque;
 import java.util.Collection;
+import java.util.HashSet;
+import java.util.Queue;
+import java.util.Set;
 
 /**
  * Encapsulates all logic required to build a {@link LocalConfigurationMetadata} from a
@@ -101,6 +106,9 @@ public void visitChildVariant(String name, DisplayName displayName, ImmutableAtt
             }
         });
 
+        // We must call this before collecting dependency state, since dependency actions may modify the hierarchy.
+        runDependencyActionsInHierarchy(configuration);
+
         // Collect all dependencies and excludes in hierarchy.
         ImmutableAttributes attributes = configuration.getAttributes().asImmutable();
         ImmutableSet<String> hierarchy = Configurations.getNames(configuration.getHierarchy());
@@ -129,6 +137,30 @@ public void visitChildVariant(String name, DisplayName displayName, ImmutableAtt
         );
     }
 
+    /**
+     * Runs the dependency actions for all configurations in {@code conf}'s hierarchy.
+     *
+     * <p>Specifically handles the case where {@link Configuration#extendsFrom} is called during the
+     * dependency action execution.</p>
+     */
+    private static void runDependencyActionsInHierarchy(ConfigurationInternal conf) {
+        Set<Configuration> seen = new HashSet<>();
+        Queue<Configuration> remaining = new ArrayDeque<>();
+        remaining.add(conf);
+        seen.add(conf);
+
+        while (!remaining.isEmpty()) {
+            Configuration current = remaining.remove();
+            ((ConfigurationInternal) current).runDependencyActions();
+
+            for (Configuration parent : current.getExtendsFrom()) {
+                if (seen.add(parent)) {
+                    remaining.add(parent);
+                }
+            }
+        }
+    }
+
     /**
      * Collect all dependencies and excludes of all configurations in the provided {@code hierarchy}.
      */
@@ -166,8 +198,6 @@ private DependencyState getDefinedState(ConfigurationInternal configuration, Com
      * DSL representation to the internal representation.
      */
     private DependencyState doGetDefinedState(ConfigurationInternal configuration, ComponentIdentifier componentId) {
-        // Run any actions to add/modify dependencies
-        configuration.runDependencyActions();
 
         AttributeContainer attributes = configuration.getAttributes();