Skip to content

Commit

Permalink
Merge pull request #23232 Inject services into MinimalExternalModuleD…
Browse files Browse the repository at this point in the history 
…ependency instances

<!--- The issue this PR addresses -->
Fixes #23096

All dependencies which extend `AbstractModuleDependency` should have their `attributesFactory` and `capabilityNotationParser` initialized. This is usually done in the `DefaultDependencyFactory`, though now that `MinimalExternalModuleDependency` extends `ExternalModuleDependency` and its implements extend `AbstractModuleDependency`, we must be sure that we initialize its services when we instantiate them. This PR does that.

Further, we ensure we properly copy all fields in instances of `MinimalExternalModuleDependency` when copying them. This ensures we do not leave behind dependency artifacts or other data when adding dependencies to configurations via the project `DependencyHandler`.

Co-authored-by: Justin Van Dort <jvandort@gradle.com>
  • Loading branch information
@bot-gradle @jvandort
bot-gradle and jvandort committed Dec 20, 2022
2 parents d1a0b6e + 4fed83e commit 399facb
Show file tree
Hide file tree
Showing 19 changed files with 355 additions and 104 deletions.
6 changes: 6 additions & 0 deletions ...ects/core-api/src/main/java/org/gradle/api/artifacts/MinimalExternalModuleDependency.java
View file
Expand Up @@ -26,4 +26,10 @@
public interface MinimalExternalModuleDependency extends ExternalModuleDependency {
ModuleIdentifier getModule();
VersionConstraint getVersionConstraint();

/**
* {@inheritDoc}
*/
@Override
MinimalExternalModuleDependency copy();
}
6 changes: 5 additions & 1 deletion ...rc/main/java/org/gradle/api/internal/artifacts/dependencies/AbstractModuleDependency.java
View file
Expand Up @@ -275,10 +275,14 @@ public void setCapabilityNotationParser(NotationParser<Object, Capability> capab
this.capabilityNotationParser = capabilityNotationParser;
}

protected ImmutableAttributesFactory getAttributesFactory() {
public ImmutableAttributesFactory getAttributesFactory() {
return attributesFactory;
}

public NotationParser<Object, Capability> getCapabilityNotationParser() {
return capabilityNotationParser;
}

private void setAttributes(AttributeContainerInternal attributes) {
this.attributes = attributes;
}
Expand Down
61 changes: 55 additions & 6 deletions ...egTest/groovy/org/gradle/integtests/resolve/api/ArtifactDeclarationIntegrationTest.groovy
View file
Expand Up @@ -209,7 +209,7 @@ task checkArtifacts {
}
task jar {}
}
project(':b') {
dependencies {
compile project(':a')
Expand All @@ -218,7 +218,7 @@ task checkArtifacts {
task checkArtifacts {
inputs.files configurations.compile
doLast {
configurations.compile.resolvedConfiguration.resolvedArtifacts.forEach { println it }
configurations.compile.resolvedConfiguration.resolvedArtifacts.forEach { println it }
}
}
}
Expand Down Expand Up @@ -247,7 +247,7 @@ task checkArtifacts {
}
task jar {}
}
project(':b') {
dependencies {
compile project(':a')
Expand All @@ -256,7 +256,7 @@ task checkArtifacts {
task checkArtifacts {
inputs.files configurations.compile
doLast {
assert configurations.compile.resolvedConfiguration.resolvedArtifacts.each { println it }
assert configurations.compile.resolvedConfiguration.resolvedArtifacts.each { println it }
}
}
}
Expand Down Expand Up @@ -289,7 +289,7 @@ task checkArtifacts {
}
task classes {}
}
project(':b') {
dependencies {
compile project(':a')
Expand All @@ -298,7 +298,7 @@ task checkArtifacts {
task checkArtifacts {
inputs.files configurations.compile
doLast {
assert configurations.compile.resolvedConfiguration.resolvedArtifacts.each { println it }
assert configurations.compile.resolvedConfiguration.resolvedArtifacts.each { println it }
}
}
}
Expand Down Expand Up @@ -506,4 +506,53 @@ task checkArtifacts {
result.assertTasksExecutedInOrder(":a:checkArtifacts", ":a:jar", ":b:checkArtifacts")
}

def 'artifacts are copied when declaring dependency on existing version catalog dependency with artifact'() {
given:
buildFile << """
configurations {
implementation
destination1
destination2
}
dependencies {
implementation(libs.test) {
artifact {
classifier = "alternative"
}
}
}
task copyAndPrintDependencies {
configurations.implementation.dependencies.each {
project.dependencies.add("destination1", it)
configurations.destination2.dependencies.add(it)
}
doLast {
configurations.implementation.dependencies.each {
println("implementation " + it + " " + it.artifacts*.classifier)
}
configurations.destination1.dependencies.each {
println("destination1 " + it + " " + it.artifacts*.classifier)
}
configurations.destination2.dependencies.each {
println("destination2 " + it + " " + it.artifacts*.classifier)
}
}
}
"""
file("gradle/libs.versions.toml") << """[libraries]
test = { module = 'org:test', version = '1.0' }
"""

when:
succeeds "copyAndPrintDependencies"

then:
outputContains("""implementation org:test:1.0 [alternative]
destination1 org:test:1.0 [alternative]
destination2 org:test:1.0 [alternative]""")
}

}
23 changes: 14 additions & 9 deletions ...c/main/java/org/gradle/api/internal/artifacts/DependencyManagementBuildScopeServices.java
View file
Expand Up @@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Sets;
import org.gradle.StartParameter;
import org.gradle.api.capabilities.Capability;
import org.gradle.api.internal.ClassPathRegistry;
import org.gradle.api.internal.CollectionCallbackActionDecorator;
import org.gradle.api.internal.DocumentationRegistry;
Expand All @@ -30,6 +29,7 @@
import org.gradle.api.internal.artifacts.component.ComponentIdentifierFactory;
import org.gradle.api.internal.artifacts.component.DefaultComponentIdentifierFactory;
import org.gradle.api.internal.artifacts.configurations.DependencyMetaDataProvider;
import org.gradle.api.internal.artifacts.dsl.CapabilityNotationParser;
import org.gradle.api.internal.artifacts.dsl.CapabilityNotationParserFactory;
import org.gradle.api.internal.artifacts.dsl.dependencies.DependencyFactoryInternal;
import org.gradle.api.internal.artifacts.ivyservice.ArtifactCacheLockingManager;
Expand Down Expand Up @@ -215,7 +215,6 @@
import org.gradle.internal.service.ServiceRegistry;
import org.gradle.internal.snapshot.ValueSnapshot;
import org.gradle.internal.snapshot.ValueSnapshotter;
import org.gradle.internal.typeconversion.NotationParser;
import org.gradle.internal.vfs.VirtualFileSystem;
import org.gradle.util.internal.BuildCommencedTimeProvider;
import org.gradle.util.internal.SimpleMapInterner;
Expand Down Expand Up @@ -274,11 +273,15 @@ VersionComparator createVersionComparator() {
return new DefaultVersionComparator();
}

CapabilityNotationParser createCapabilityNotationParser() {
return new CapabilityNotationParserFactory(false).create();
}

DefaultProjectDependencyFactory createProjectDependencyFactory(
Instantiator instantiator,
StartParameter startParameter,
ImmutableAttributesFactory attributesFactory) {
NotationParser<Object, Capability> capabilityNotationParser = new CapabilityNotationParserFactory(false).create();
ImmutableAttributesFactory attributesFactory,
CapabilityNotationParser capabilityNotationParser) {
return new DefaultProjectDependencyFactory(instantiator, startParameter.isBuildProjectDependencies(), capabilityNotationParser, attributesFactory);
}

Expand All @@ -290,13 +293,13 @@ DependencyFactoryInternal createDependencyFactory(
FileCollectionFactory fileCollectionFactory,
RuntimeShadedJarFactory runtimeShadedJarFactory,
ImmutableAttributesFactory attributesFactory,
SimpleMapInterner stringInterner) {
NotationParser<Object, Capability> capabilityNotationParser = new CapabilityNotationParserFactory(false).create();
SimpleMapInterner stringInterner,
CapabilityNotationParser capabilityNotationParser) {
ProjectDependencyFactory projectDependencyFactory = new ProjectDependencyFactory(factory);

return new DefaultDependencyFactory(
instantiator,
DependencyNotationParser.create(instantiator, factory, classPathRegistry, fileCollectionFactory, runtimeShadedJarFactory, currentGradleInstallation, stringInterner, attributesFactory, capabilityNotationParser),
DependencyNotationParser.create(instantiator, factory, classPathRegistry, fileCollectionFactory, runtimeShadedJarFactory, currentGradleInstallation, stringInterner),
DependencyConstraintNotationParser.parser(instantiator, factory, stringInterner, attributesFactory),
new ClientModuleNotationParserFactory(instantiator, stringInterner).create(),
capabilityNotationParser, projectDependencyFactory,
Expand Down Expand Up @@ -677,8 +680,10 @@ DependenciesAccessors createDependenciesAccessorGenerator(ClassPathRegistry regi
ExecutionEngine executionEngine,
FeaturePreviews featurePreviews,
FileCollectionFactory fileCollectionFactory,
InputFingerprinter inputFingerprinter) {
return new DefaultDependenciesAccessors(registry, workspace, factory, featurePreviews, executionEngine, fileCollectionFactory, inputFingerprinter);
InputFingerprinter inputFingerprinter,
ImmutableAttributesFactory attributesFactory,
CapabilityNotationParser capabilityNotationParser) {
return new DefaultDependenciesAccessors(registry, workspace, factory, featurePreviews, executionEngine, fileCollectionFactory, inputFingerprinter, attributesFactory, capabilityNotationParser);
}


Expand Down
2 changes: 1 addition & 1 deletion .../java/org/gradle/api/internal/artifacts/dependencies/DefaultExternalModuleDependency.java
View file
Expand Up @@ -36,7 +36,7 @@ public DefaultExternalModuleDependency(ModuleIdentifier id, MutableVersionConstr
}

@Override
public DefaultExternalModuleDependency copy() {
public ExternalModuleDependency copy() {
DefaultExternalModuleDependency copiedModuleDependency = new DefaultExternalModuleDependency(getGroup(), getName(), getVersion(), getTargetConfiguration());
copyTo(copiedModuleDependency);
return copiedModuleDependency;
Expand Down
8 changes: 6 additions & 2 deletions ...rc/main/java/org/gradle/api/internal/artifacts/dependencies/DefaultMinimalDependency.java
View file
Expand Up @@ -15,13 +15,12 @@
*/
package org.gradle.api.internal.artifacts.dependencies;

import org.gradle.api.artifacts.MinimalExternalModuleDependency;
import org.gradle.api.artifacts.ModuleIdentifier;
import org.gradle.api.artifacts.MutableVersionConstraint;

import java.io.Serializable;

public class DefaultMinimalDependency extends DefaultExternalModuleDependency implements MinimalExternalModuleDependency, Serializable {
public class DefaultMinimalDependency extends DefaultExternalModuleDependency implements MinimalExternalModuleDependencyInternal, Serializable {
public DefaultMinimalDependency(ModuleIdentifier module, MutableVersionConstraint versionConstraint) {
super(module, versionConstraint);
}
Expand All @@ -41,6 +40,11 @@ protected void validateMutation(Object currentValue, Object newValue) {
validateMutation();
}

@Override
public void copyTo(AbstractExternalModuleDependency target) {
super.copyTo(target);
}

// Intentionally changes to the mutable version.
@Override
public DefaultMutableMinimalDependency copy() {
Expand Down
101 changes: 78 additions & 23 deletions .../java/org/gradle/api/internal/artifacts/dependencies/DefaultMinimalDependencyVariant.java
View file
Expand Up @@ -18,38 +18,92 @@
import org.gradle.api.Action;
import org.gradle.api.artifacts.MinimalExternalModuleDependency;
import org.gradle.api.artifacts.ModuleDependencyCapabilitiesHandler;
import org.gradle.api.artifacts.ModuleIdentifier;
import org.gradle.api.artifacts.MutableVersionConstraint;
import org.gradle.api.attributes.AttributeContainer;
import org.gradle.api.internal.artifacts.dsl.dependencies.ModuleFactoryHelper;
import org.gradle.internal.Actions;
import org.gradle.util.internal.GUtil;

import javax.annotation.Nullable;

public class DefaultMinimalDependencyVariant extends DefaultExternalModuleDependency implements MinimalExternalModuleDependency, DependencyVariant {
private final MinimalExternalModuleDependency delegate;
private final Action<? super AttributeContainer> attributesMutator;
private final Action<? super ModuleDependencyCapabilitiesHandler> capabilitiesMutator;
private final String classifier;
private final String artifactType;
public class DefaultMinimalDependencyVariant extends DefaultExternalModuleDependency implements MinimalExternalModuleDependencyInternal, DependencyVariant {
private Action<? super AttributeContainer> attributesMutator;
private Action<? super ModuleDependencyCapabilitiesHandler> capabilitiesMutator;
private String classifier;
private String artifactType;

public DefaultMinimalDependencyVariant(MinimalExternalModuleDependency delegate,
@Nullable Action<? super AttributeContainer> attributesMutator,
@Nullable Action<? super ModuleDependencyCapabilitiesHandler> capabilitiesMutator,
@Nullable String classifier,
@Nullable String artifactType) {
@Nullable String artifactType
) {
super(delegate.getModule(), new DefaultMutableVersionConstraint(delegate.getVersionConstraint()));
this.delegate = delegate;
boolean delegateIsVariant = delegate instanceof DefaultMinimalDependencyVariant;
this.attributesMutator = delegateIsVariant ? Actions.composite(((DefaultMinimalDependencyVariant) delegate).attributesMutator, attributesMutator == null ? Actions.doNothing() : attributesMutator) : attributesMutator;
this.capabilitiesMutator = delegateIsVariant ? Actions.composite(((DefaultMinimalDependencyVariant) delegate).capabilitiesMutator, capabilitiesMutator == null ? Actions.doNothing() : capabilitiesMutator) : capabilitiesMutator;
if (classifier == null && delegateIsVariant) {
classifier = ((DefaultMinimalDependencyVariant) delegate).getClassifier();
}
if (artifactType == null && delegateIsVariant) {
artifactType = ((DefaultMinimalDependencyVariant) delegate).getArtifactType();

attributesMutator = GUtil.elvis(attributesMutator, Actions.doNothing());
capabilitiesMutator = GUtil.elvis(capabilitiesMutator, Actions.doNothing());

if (delegate instanceof DefaultMinimalDependencyVariant) {
this.attributesMutator = Actions.composite(((DefaultMinimalDependencyVariant) delegate).attributesMutator, attributesMutator);
this.capabilitiesMutator = Actions.composite(((DefaultMinimalDependencyVariant) delegate).capabilitiesMutator, capabilitiesMutator);
this.classifier = GUtil.elvis(classifier, ((DefaultMinimalDependencyVariant) delegate).getClassifier());
this.artifactType = GUtil.elvis(classifier, ((DefaultMinimalDependencyVariant) delegate).getArtifactType());
} else {
this.attributesMutator = attributesMutator;
this.capabilitiesMutator = capabilitiesMutator;
this.classifier = classifier;
this.artifactType = artifactType;
}

MinimalExternalModuleDependencyInternal internal = (MinimalExternalModuleDependencyInternal) delegate;
setAttributesFactory(internal.getAttributesFactory());
setCapabilityNotationParser(internal.getCapabilityNotationParser());
}

private DefaultMinimalDependencyVariant(
ModuleIdentifier id,
MutableVersionConstraint versionConstraint,
Action<? super AttributeContainer> attributesMutator,
Action<? super ModuleDependencyCapabilitiesHandler> capabilitiesMutator,
@Nullable String classifier,
@Nullable String artifactType
) {
super(id, versionConstraint);
this.attributesMutator = attributesMutator;
this.capabilitiesMutator = capabilitiesMutator;
this.classifier = classifier;
this.artifactType = artifactType;
}

@Override
public void copyTo(AbstractExternalModuleDependency target) {
super.copyTo(target);
if (target instanceof DefaultMinimalDependencyVariant) {
DefaultMinimalDependencyVariant depVariant = (DefaultMinimalDependencyVariant) target;
depVariant.attributesMutator = attributesMutator;
depVariant.capabilitiesMutator = capabilitiesMutator;
depVariant.classifier = classifier;
depVariant.artifactType = artifactType;
} else {
target.attributes(attributesMutator);
target.capabilities(capabilitiesMutator);
if (classifier != null || artifactType != null) {
ModuleFactoryHelper.addExplicitArtifactsIfDefined(target, artifactType, classifier);
}
}
}

@Override
public MinimalExternalModuleDependency copy() {
DefaultMinimalDependencyVariant dependency = new DefaultMinimalDependencyVariant(
getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()),
attributesMutator, capabilitiesMutator, classifier, artifactType
);
copyTo(dependency);
return dependency;
}

@Override
public void because(String reason) {
validateMutation();
Expand All @@ -67,16 +121,12 @@ protected void validateMutation(Object currentValue, Object newValue) {

@Override
public void mutateAttributes(AttributeContainer attributes) {
if (attributesMutator!= null) {
attributesMutator.execute(attributes);
}
attributesMutator.execute(attributes);
}

@Override
public void mutateCapabilities(ModuleDependencyCapabilitiesHandler capabilitiesHandler) {
if (capabilitiesMutator != null) {
capabilitiesMutator.execute(capabilitiesHandler);
}
capabilitiesMutator.execute(capabilitiesHandler);
}

@Nullable
Expand All @@ -93,6 +143,11 @@ public String getArtifactType() {

@Override
public String toString() {
return delegate.toString();
return "DefaultMinimalDependencyVariant{" +
", attributesMutator=" + attributesMutator +
", capabilitiesMutator=" + capabilitiesMutator +
", classifier='" + classifier + '\'' +
", artifactType='" + artifactType + '\'' +
"} " + super.toString();
}
}

0 comments on commit 399facb

Please sign in to comment.