Skip to content

Commit

Permalink
Merge pull request #25675 Cherry pick: Associate each component in th…
Browse files Browse the repository at this point in the history
…e dependency graph with the correct repository in build operation results

Fixes #25674

Co-authored-by: Rodrigo B. de Oliveira <rodrigo@gradle.com>
  • Loading branch information
bot-gradle and bamboo committed Jul 7, 2023
2 parents 95006a7 + 81de96d commit 3ee9dc4
Show file tree
Hide file tree
Showing 28 changed files with 245 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,11 @@ class ResolveConfigurationDependenciesBuildOperationIntegrationTest extends Abst

then:
def op = operations.first(ResolveConfigurationDependenciesBuildOperationType)
def repo1Id = repoId('withCreds', op.details)
op.details.repositories.size() == 2
op.details.repositories*.id.unique(false).size() == 2
op.details.repositories[0].name == 'withoutCreds'
op.details.repositories[1].name == 'withCreds'
def repo1Id = op.details.repositories[1].id
def resolvedComponents = op.result.components
resolvedComponents.size() == 2
resolvedComponents.'org.foo:good:1.0'.repoId == repo1Id
Expand All @@ -632,7 +636,11 @@ class ResolveConfigurationDependenciesBuildOperationIntegrationTest extends Abst
then:
// This demonstrates a bug in Gradle, where we ignore the requirement for credentials when retrieving from the cache
def op2 = operations.first(ResolveConfigurationDependenciesBuildOperationType)
def repo2Id = repoId('withoutCreds', op.details)
op2.details.repositories.size() == 2
op2.details.repositories*.id.unique(false).size() == 2
op2.details.repositories[0].name == 'withoutCreds'
op2.details.repositories[1].name == 'withCreds'
def repo2Id = op2.details.repositories[0].id
def resolvedComponents2 = op2.result.components
resolvedComponents2.size() == 2
resolvedComponents2.'org.foo:good:1.0'.repoId == repo2Id
Expand Down Expand Up @@ -696,12 +704,14 @@ class ResolveConfigurationDependenciesBuildOperationIntegrationTest extends Abst
def ops = operations.all(ResolveConfigurationDependenciesBuildOperationType)
def op = ops[0]
op.details.configurationName == 'compileClasspath'
op.details.repositories.size() == 1
def repo1Id = repoId('one', op.details)
def resolvedComponents = op.result.components
resolvedComponents.size() == 2
resolvedComponents.'org.foo:good:1.0'.repoId == repo1Id
def op2 = ops[1]
op2.details.configurationName == 'testCompileClasspath'
op2.details.repositories.size() == 2
def repo2Id = repoId('two', op2.details)
def resolvedComponents2 = op2.result.components
resolvedComponents2.size() == 4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private RepositoryImpl(RepositoryDescriptor descriptor) {

@Override
public String getId() {
return descriptor.getId();
return descriptor.getName();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private void found(BuildableComponentIdResolveResult result, List<RepositoryReso
for (RepositoryResolveState resolveState : resolveStates) {
resolveState.registerAttempts(result);
}
result.resolved(latestResolved.component);
result.resolved(latestResolved.component, new ModuleComponentGraphSpecificResolveState(latestResolved.repository.getName()));
}

private void notFound(BuildableComponentIdResolveResult result, ModuleComponentSelector requested, List<RepositoryResolveState> resolveStates) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed 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 org.gradle.api.internal.artifacts.ivyservice.ivyresolve;

import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;

class ModuleComponentGraphSpecificResolveState implements ComponentGraphSpecificResolveState {
private final String repositoryName;

public ModuleComponentGraphSpecificResolveState(String repositoryName) {
this.repositoryName = repositoryName;
}

@Override
public String getRepositoryName() {
return repositoryName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ private void resolveModule(ModuleComponentIdentifier identifier, ComponentOverri
LOGGER.debug("Discarding resolve failure.", error);
}

result.resolved(latestResolved.component);
String repositoryName = latestResolved.repository.getName();
result.resolved(latestResolved.component, new ModuleComponentGraphSpecificResolveState(repositoryName));
return;
}
if (!errors.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.gradle.internal.component.model.ComponentArtifactMetadata;
import org.gradle.internal.component.model.ComponentArtifactResolveMetadata;
import org.gradle.internal.component.model.ComponentArtifactResolveVariantState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.component.model.ComponentOverrideMetadata;
import org.gradle.internal.component.model.DependencyMetadata;
import org.gradle.internal.resolve.ModuleVersionResolveException;
Expand Down Expand Up @@ -89,7 +90,7 @@ public void resolve(DependencyMetadata dependency, VersionSelector acceptor, @Nu
if (rejector != null && rejector.accept(component.getModuleVersionId().getVersion())) {
result.rejected(projectId, component.getModuleVersionId());
} else {
result.resolved(component);
result.resolved(component, ComponentGraphSpecificResolveState.EMPTY_STATE);
}
}
}
Expand All @@ -103,7 +104,7 @@ public void resolve(ComponentIdentifier identifier, ComponentOverrideMetadata co
if (component == null) {
result.failed(new ModuleVersionResolveException(DefaultProjectComponentSelector.newSelector(projectId), () -> projectId + " not found."));
} else {
result.resolved(component);
result.resolved(component, ComponentGraphSpecificResolveState.EMPTY_STATE);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.gradle.api.artifacts.result.ComponentSelectionReason;
import org.gradle.api.artifacts.result.ResolvedVariantResult;
import org.gradle.api.capabilities.Capability;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.RepositoryChainModuleSource;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.ComponentResolutionState;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.DependencyGraphComponent;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.conflicts.VersionConflictResolutionDetails;
Expand All @@ -40,6 +39,7 @@
import org.gradle.internal.component.external.model.DefaultImmutableCapability;
import org.gradle.internal.component.model.ComponentGraphResolveMetadata;
import org.gradle.internal.component.model.ComponentGraphResolveState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.component.model.ComponentOverrideMetadata;
import org.gradle.internal.component.model.DefaultComponentOverrideMetadata;
import org.gradle.internal.component.model.VariantGraphResolveMetadata;
Expand Down Expand Up @@ -71,6 +71,7 @@ public class ComponentState implements ComponentResolutionState, DependencyGraph
private final int hashCode;

private volatile ComponentGraphResolveState resolveState;
private volatile ComponentGraphSpecificResolveState graphResolveState;

private ComponentSelectionState state = ComponentSelectionState.Selectable;
private ModuleVersionResolveException metadataResolveFailure;
Expand Down Expand Up @@ -114,9 +115,7 @@ public ModuleVersionIdentifier getId() {

@Override
public String getRepositoryId() {
return resolveState.getSources().withSource(RepositoryChainModuleSource.class, source -> source
.map(RepositoryChainModuleSource::getRepositoryId)
.orElse(null));
return graphResolveState.getRepositoryName();
}

@Override
Expand Down Expand Up @@ -234,6 +233,7 @@ public void resolve() {
return;
}
resolveState = result.getState();
graphResolveState = result.getGraphState();
}

private boolean tryResolveVirtualPlatform() {
Expand All @@ -244,7 +244,7 @@ private boolean tryResolveVirtualPlatform() {
if (versionState != null) {
ComponentGraphResolveState lenient = versionState.maybeAsLenientPlatform((ModuleComponentIdentifier) componentIdentifier, id);
if (lenient != null) {
setState(lenient);
setState(lenient, ComponentGraphSpecificResolveState.EMPTY_STATE);
return true;
}
}
Expand All @@ -254,8 +254,9 @@ private boolean tryResolveVirtualPlatform() {
return false;
}

public void setState(ComponentGraphResolveState state) {
public void setState(ComponentGraphResolveState state, ComponentGraphSpecificResolveState graphState) {
this.resolveState = state;
this.graphResolveState = graphState;
this.metadataResolveFailure = null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.gradle.api.internal.attributes.AttributeMergingException;
import org.gradle.api.internal.attributes.ImmutableAttributes;
import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.component.model.DependencyMetadata;
import org.gradle.internal.component.model.ForcingDependencyMetadata;
import org.gradle.internal.id.IdGenerator;
Expand Down Expand Up @@ -465,7 +466,7 @@ void maybeCreateVirtualMetadata(ResolveState resolveState) {
for (ComponentState componentState : versions.values()) {
if (componentState.getMetadataOrNull() == null) {
// TODO LJA Using the root as the NodeState here is a bit of a cheat, investigate if we can track the proper NodeState
componentState.setState(LenientPlatformGraphResolveState.of((ModuleComponentIdentifier) componentState.getComponentId(), componentState.getId(), platformState, resolveState.getRoot(), resolveState));
componentState.setState(LenientPlatformGraphResolveState.of((ModuleComponentIdentifier) componentState.getComponentId(), componentState.getId(), platformState, resolveState.getRoot(), resolveState), ComponentGraphSpecificResolveState.EMPTY_STATE);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.gradle.internal.component.local.model.LocalConfigurationGraphResolveMetadata;
import org.gradle.internal.component.local.model.LocalFileDependencyMetadata;
import org.gradle.internal.component.model.ComponentGraphResolveState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.component.model.DelegatingDependencyMetadata;
import org.gradle.internal.component.model.DependencyMetadata;
import org.gradle.internal.component.model.IvyArtifactName;
Expand Down Expand Up @@ -603,7 +604,7 @@ private void addPlatformEdges(Collection<EdgeState> discoveredEdges, ModuleCompo
if (state == null) {
// the platform doesn't exist, so we're building a lenient one
state = LenientPlatformGraphResolveState.of(platformComponentIdentifier, potentialEdge.toModuleVersionId, virtualPlatformState, this, resolveState);
potentialEdge.component.setState(state);
potentialEdge.component.setState(state, ComponentGraphSpecificResolveState.EMPTY_STATE);
// And now let's make sure we do not have another version of that virtual platform missing its metadata
potentialEdge.component.getModule().maybeCreateVirtualMetadata(resolveState);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.gradle.internal.component.local.model.DefaultLocalComponentGraphResolveState;
import org.gradle.internal.component.local.model.LocalComponentMetadata;
import org.gradle.internal.component.model.ComponentGraphResolveState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.component.model.DependencyMetadata;
import org.gradle.internal.component.model.VariantGraphResolveMetadata;
import org.gradle.internal.id.IdGenerator;
Expand Down Expand Up @@ -138,7 +139,7 @@ public ResolveState(
// Create root component and module
ModuleResolveState rootModule = getModule(moduleVersionId.getModule(), true);
ComponentState rootComponent = rootModule.getVersion(moduleVersionId, rootComponentMetadata.getId());
rootComponent.setState(new DefaultLocalComponentGraphResolveState(rootComponentMetadata));
rootComponent.setState(new DefaultLocalComponentGraphResolveState(rootComponentMetadata), ComponentGraphSpecificResolveState.EMPTY_STATE);
rootModule.select(rootComponent);

this.selectorStateResolver = new SelectorStateResolver<>(conflictResolver, this, rootComponent, resolveOptimizations, versionComparator, versionParser);
Expand Down Expand Up @@ -176,10 +177,10 @@ private ModuleResolveState getModule(ModuleIdentifier id, boolean rootModule) {
}

@Override
public ComponentState getRevision(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier id, ComponentGraphResolveState state) {
public ComponentState getRevision(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier id, ComponentGraphResolveState state, ComponentGraphSpecificResolveState graphState) {
ComponentState componentState = getModule(id.getModule()).getVersion(id, componentIdentifier);
if (!componentState.alreadyResolved()) {
componentState.setState(state);
componentState.setState(state, graphState);
}
return componentState;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.ComponentResolutionState;
import org.gradle.internal.component.model.ComponentGraphResolveState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;

import javax.annotation.Nullable;

public interface ComponentStateFactory<T extends ComponentResolutionState> {
T getRevision(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier id, ComponentGraphResolveState state);
T getRevision(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier id, @Nullable ComponentGraphResolveState state, @Nullable ComponentGraphSpecificResolveState graphState);
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private boolean hasSoftForce() {
}

public static <T extends ComponentResolutionState> T componentForIdResolveResult(ComponentStateFactory<T> componentFactory, ComponentIdResolveResult idResolveResult, ResolvableSelectorState selector) {
T component = componentFactory.getRevision(idResolveResult.getId(), idResolveResult.getModuleVersionId(), idResolveResult.getState());
T component = componentFactory.getRevision(idResolveResult.getId(), idResolveResult.getModuleVersionId(), idResolveResult.getState(), idResolveResult.getGraphState());
if (idResolveResult.isRejected()) {
component.reject();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@
import javax.annotation.Nullable;

/**
* State for a component instance (eg version of a component) that is used to perform dependency graph resolution.
* State for a component instance (e.g. version of a component) that is used to perform dependency graph resolution and that is independent of the particular
* graph being resolved.
*
* <p>Resolution happens in multiple steps. The first step is to calculate the dependency graph, which involves selecting component instances and one or more variants of each instance.
* This type exposes only the information and operations required to do this. In particular, it does not expose any information about artifacts unless this is actually required for graph resolution,
* which only happens in certain specific cases (and something we should deprecate).</p>
*
* <p>The subsequent resolution steps, to select artifacts, are performed using the instance returned by {@link #prepareForArtifactResolution()}.</p>
*
* <p>Instances of this type are obtained via the methods of {@link org.gradle.internal.resolve.resolver.DependencyToComponentIdResolver} or {@link org.gradle.internal.resolve.resolver.ComponentMetaDataResolver}.</p>
*
* <p>This interface says nothing about thread safety, however some subtypes may be required to be thread safe.</p>
*
* @see ComponentGraphSpecificResolveState for dependency graph specific state for the component.
*/
public interface ComponentGraphResolveState {
ComponentIdentifier getId();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed 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 org.gradle.internal.component.model;

import javax.annotation.Nullable;

/**
* State for a component that is specific to a particular dependency graph resolution.
*
* @see ComponentGraphResolveState for graph independent state for the component.
*/
public interface ComponentGraphSpecificResolveState {
ComponentGraphSpecificResolveState EMPTY_STATE = new ComponentGraphSpecificResolveState() {
@Nullable
@Override
public String getRepositoryName() {
return null;
}
};

@Nullable
String getRepositoryName();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.gradle.api.artifacts.ModuleVersionIdentifier;
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.internal.component.model.ComponentGraphResolveState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.resolve.ModuleVersionResolveException;
import org.gradle.internal.resolve.RejectedVersion;

Expand All @@ -38,7 +39,7 @@ public interface BuildableComponentIdResolveResult extends ComponentIdResolveRes
/**
* Marks the component selector as resolved, with the provided state. The id is taken from the metadata.
*/
void resolved(ComponentGraphResolveState state);
void resolved(ComponentGraphResolveState state, ComponentGraphSpecificResolveState graphState);

/**
* Marks the component selection as failed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@

import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.internal.component.model.ComponentGraphResolveState;
import org.gradle.internal.component.model.ComponentGraphSpecificResolveState;
import org.gradle.internal.resolve.ModuleVersionResolveException;

public interface BuildableComponentResolveResult extends ComponentResolveResult, ResourceAwareResolveResult {
/**
* Marks the component as resolved, with the given graph resolution state.
*/
void resolved(ComponentGraphResolveState state);
void resolved(ComponentGraphResolveState state, ComponentGraphSpecificResolveState graphState);

/**
* Marks the resolve as failed with the given exception.
Expand Down

0 comments on commit 3ee9dc4

Please sign in to comment.