Skip to content

Commit

Permalink
Merge pull request #23246 Ensure all version catalog dependency prope…
Browse files Browse the repository at this point in the history
…rties are copied

Follow up for #23096 with a more comprehensive test.
Ensures `targetConfiguration` is copied when version catalog dependencies are copied. This property was copied pre `7.6` but regressed in `7.6`.

While fixing this bug, I discovered `endorsing` and `versionConstraint.branch` were also not copied. We are leaving those fixes out of this PR but will fix in `8.1`. See: #23286

Co-authored-by: Justin Van Dort <jvandort@gradle.com>
  • Loading branch information
bot-gradle and jvandort committed Dec 23, 2022
2 parents ea3081d + 10086f7 commit 289e454
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 67 deletions.
Expand Up @@ -506,53 +506,4 @@ 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]""")
}

}
Expand Up @@ -2306,4 +2306,114 @@ Second: 1.1"""
expect:
succeeds ':help'
}
@Issue("https://github.com/gradle/gradle/issues/23096")
def 'all properties of version catalog dependencies are copied when the dependency is copied'() {
given:
buildFile << """
configurations {
implementation
destination1
destination2
}

dependencies {
implementation(libs.test1) {
because("reason1")

exclude(group: "test-group", module: "test-module")
artifact {
name = "test-name"
classifier = "test-classifier"
extension = "test-ext"
type = "test-type"
url = "test-url"
}
transitive = true
endorseStrictVersions()

version {
branch = "branch"
strictly("123")
prefer("789")
reject("aaa")
}

changing = true
}
implementation(libs.test2) {
transitive = false
targetConfiguration = "abc"
doNotEndorseStrictVersions()

version {
require("456")
}

changing = false
}
implementation(libs.test3) {
attributes {
attribute(Attribute.of('foo', String), 'bar')
}
capabilities {
requireCapability("org:test-cap:1.1")
}
}
}

def verifyDep(original, copied) {
// Dependency
assert original.group == copied.group
assert original.name == copied.name
assert original.version == copied.version
assert original.reason == copied.reason

// ModuleDependency
assert original.excludeRules == copied.excludeRules
assert original.artifacts == copied.artifacts
assert original.transitive == copied.transitive
assert original.targetConfiguration == copied.targetConfiguration
assert original.attributes == copied.attributes
assert original.requestedCapabilities == copied.requestedCapabilities

// ExternalDependency + ExternalModuleDependency
assert original.changing == copied.changing
}

def getOriginal(dep) {
configurations.implementation.dependencies.find { it.name == dep.name }
}

task copyAndVerifyDependencies {
configurations.implementation.dependencies.each {
project.dependencies.add("destination1", it)
configurations.destination2.dependencies.add(it)
}

doLast {
configurations.destination1.dependencies.each {
verifyDep(getOriginal(it), it)
}

configurations.destination2.dependencies.each {
verifyDep(getOriginal(it), it)
}

configurations.implementation.copy().dependencies.each {
verifyDep(getOriginal(it), it)
}
}
}
"""
file("gradle/libs.versions.toml") << """[libraries]
test1 = { module = 'org:test1', version = '1.0' }
test2 = { module = 'org:test2', version = '1.0' }
test3 = { module = 'org:test3', version = '1.0' }
"""
expect:
succeeds "copyAndVerifyDependencies"
}
}
Expand Up @@ -45,8 +45,8 @@ public AbstractExternalModuleDependency(ModuleIdentifier module, String version,
this.versionConstraint = new DefaultMutableVersionConstraint(version);
}

public AbstractExternalModuleDependency(ModuleIdentifier module, MutableVersionConstraint version) {
super(null);
public AbstractExternalModuleDependency(ModuleIdentifier module, MutableVersionConstraint version, @Nullable String configuration) {
super(configuration);
if (module == null) {
throw new InvalidUserDataException("Module must not be null!");
}
Expand Down
Expand Up @@ -21,18 +21,20 @@
import org.gradle.api.artifacts.ModuleIdentifier;
import org.gradle.api.artifacts.MutableVersionConstraint;

import javax.annotation.Nullable;

public class DefaultExternalModuleDependency extends AbstractExternalModuleDependency implements ExternalModuleDependency {

public DefaultExternalModuleDependency(String group, String name, String version) {
this(group, name, version, null);
}

public DefaultExternalModuleDependency(String group, String name, String version, String configuration) {
public DefaultExternalModuleDependency(String group, String name, String version, @Nullable String configuration) {
super(assertModuleId(group, name), version, configuration);
}

public DefaultExternalModuleDependency(ModuleIdentifier id, MutableVersionConstraint versionConstraint) {
super(id, versionConstraint);
public DefaultExternalModuleDependency(ModuleIdentifier id, MutableVersionConstraint versionConstraint, @Nullable String configuration) {
super(id, versionConstraint, configuration);
}

@Override
Expand Down
Expand Up @@ -22,7 +22,7 @@

public class DefaultMinimalDependency extends DefaultExternalModuleDependency implements MinimalExternalModuleDependencyInternal, Serializable {
public DefaultMinimalDependency(ModuleIdentifier module, MutableVersionConstraint versionConstraint) {
super(module, versionConstraint);
super(module, versionConstraint, null);
}

@Override
Expand All @@ -48,7 +48,7 @@ public void copyTo(AbstractExternalModuleDependency target) {
// Intentionally changes to the mutable version.
@Override
public DefaultMutableMinimalDependency copy() {
DefaultMutableMinimalDependency dependency = new DefaultMutableMinimalDependency(getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()));
DefaultMutableMinimalDependency dependency = new DefaultMutableMinimalDependency(getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()), getTargetConfiguration());
copyTo(dependency);
return dependency;
}
Expand Down
Expand Up @@ -39,7 +39,7 @@ public DefaultMinimalDependencyVariant(MinimalExternalModuleDependency delegate,
@Nullable String classifier,
@Nullable String artifactType
) {
super(delegate.getModule(), new DefaultMutableVersionConstraint(delegate.getVersionConstraint()));
super(delegate.getModule(), new DefaultMutableVersionConstraint(delegate.getVersionConstraint()), delegate.getTargetConfiguration());

attributesMutator = GUtil.elvis(attributesMutator, Actions.doNothing());
capabilitiesMutator = GUtil.elvis(capabilitiesMutator, Actions.doNothing());
Expand All @@ -64,12 +64,13 @@ public DefaultMinimalDependencyVariant(MinimalExternalModuleDependency delegate,
private DefaultMinimalDependencyVariant(
ModuleIdentifier id,
MutableVersionConstraint versionConstraint,
@Nullable String configuration,
Action<? super AttributeContainer> attributesMutator,
Action<? super ModuleDependencyCapabilitiesHandler> capabilitiesMutator,
@Nullable String classifier,
@Nullable String artifactType
) {
super(id, versionConstraint);
super(id, versionConstraint, configuration);
this.attributesMutator = attributesMutator;
this.capabilitiesMutator = capabilitiesMutator;
this.classifier = classifier;
Expand Down Expand Up @@ -97,7 +98,7 @@ public void copyTo(AbstractExternalModuleDependency target) {
@Override
public MinimalExternalModuleDependency copy() {
DefaultMinimalDependencyVariant dependency = new DefaultMinimalDependencyVariant(
getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()),
getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()), getTargetConfiguration(),
attributesMutator, capabilitiesMutator, classifier, artifactType
);
copyTo(dependency);
Expand Down
Expand Up @@ -18,16 +18,17 @@
import org.gradle.api.artifacts.ModuleIdentifier;
import org.gradle.api.artifacts.MutableVersionConstraint;

import javax.annotation.Nullable;
import java.io.Serializable;

public class DefaultMutableMinimalDependency extends DefaultExternalModuleDependency implements MinimalExternalModuleDependencyInternal, Serializable {
public DefaultMutableMinimalDependency(ModuleIdentifier module, MutableVersionConstraint versionConstraint) {
super(module, versionConstraint);
public DefaultMutableMinimalDependency(ModuleIdentifier module, MutableVersionConstraint versionConstraint, @Nullable String configuration) {
super(module, versionConstraint, configuration);
}

@Override
public DefaultMutableMinimalDependency copy() {
DefaultMutableMinimalDependency dependency = new DefaultMutableMinimalDependency(getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()));
DefaultMutableMinimalDependency dependency = new DefaultMutableMinimalDependency(getModule(), new DefaultMutableVersionConstraint(getVersionConstraint()), getTargetConfiguration());
copyTo(dependency);
return dependency;
}
Expand Down
Expand Up @@ -34,18 +34,18 @@ public class DefaultMutableVersionConstraint extends AbstractVersionConstraint i
private final List<String> rejectedVersions = Lists.newArrayListWithExpectedSize(1);

public DefaultMutableVersionConstraint(VersionConstraint versionConstraint) {
this(versionConstraint.getPreferredVersion(), versionConstraint.getRequiredVersion(), versionConstraint.getStrictVersion(), versionConstraint.getRejectedVersions());
this(versionConstraint.getPreferredVersion(), versionConstraint.getRequiredVersion(), versionConstraint.getStrictVersion(), versionConstraint.getRejectedVersions(), versionConstraint.getBranch());
}

public DefaultMutableVersionConstraint(String version) {
this(null, version, null);
}

private DefaultMutableVersionConstraint(@Nullable String preferredVersion, String requiredVersion, @Nullable String strictVersion) {
this(preferredVersion, requiredVersion, strictVersion, Collections.emptyList());
this(preferredVersion, requiredVersion, strictVersion, Collections.emptyList(), null);
}

private DefaultMutableVersionConstraint(@Nullable String preferredVersion, String requiredVersion, @Nullable String strictVersion, List<String> rejects) {
private DefaultMutableVersionConstraint(@Nullable String preferredVersion, String requiredVersion, @Nullable String strictVersion, List<String> rejects, @Nullable String branch) {
updateVersions(preferredVersion, requiredVersion, strictVersion);
for (String reject : rejects) {
this.rejectedVersions.add(nullToEmpty(reject));
Expand Down
Expand Up @@ -140,7 +140,7 @@ public MinimalExternalDependencyNotationConverter(Instantiator instantiator) {

@Override
public void convert(MinimalExternalModuleDependency notation, NotationConvertResult<? super MinimalExternalModuleDependency> result) throws TypeConversionException {
DefaultMutableMinimalDependency moduleDependency = instantiator.newInstance(DefaultMutableMinimalDependency.class, notation.getModule(), notation.getVersionConstraint());
DefaultMutableMinimalDependency moduleDependency = instantiator.newInstance(DefaultMutableMinimalDependency.class, notation.getModule(), notation.getVersionConstraint(), notation.getTargetConfiguration());
MinimalExternalModuleDependencyInternal internal = (MinimalExternalModuleDependencyInternal) notation;
internal.copyTo(moduleDependency);
result.converted(moduleDependency);
Expand Down
Expand Up @@ -144,7 +144,7 @@ class DefaultImmutableVersionConstraintTest extends Specification {

def "can convert mutable version constraint to immutable version constraint"() {
given:
def v = new DefaultMutableVersionConstraint('1.0', '2.0', '3.0', ['1.1', '2.0'])
def v = new DefaultMutableVersionConstraint('1.0', '2.0', '3.0', ['1.1', '2.0'], null)

when:
def c = DefaultImmutableVersionConstraint.of(v)
Expand Down

0 comments on commit 289e454

Please sign in to comment.