Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue: 17964: Run exclude before and after dependencySubstitution to exclude rewritten dependencies #18131

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -542,4 +542,73 @@ task check(type: Sync) {
}
}
}


/**
* In the project, dependency `c` will be rewritten to dependency `b`.
* If we exclude dependency b, both the direct request dependency `b`
* and the dependency rewritten from `c` will be excluded
* with their transitive dependencies.
*
* Dependency graph:
* a -> b, c, f, g
* b -> d
* c -> e
*
* Exclude is applied to configuration conf
*/
def "ensure renamed dependencies are exclude correctly"() {
given:
buildFile << """
configurations {
conf {
exclude group: 'b', module: 'b'
resolutionStrategy {
dependencySubstitution {
all {
if (it.requested instanceof ModuleComponentSelector) {
if (it.requested.group == 'c' && it.requested.module == 'c') {
it.useTarget group: 'b', name: 'b', version: '1.0'
}
}
}
}
}
}
}
"""

def expectResolved = ['a', 'f', 'g']
repository {
'a:a:1.0' {
dependsOn 'b:b:1.0'
dependsOn 'c:c:1.0'
dependsOn 'f:f:1.0'
dependsOn 'g:g:1.0'
}
'b:b:1.0' {
dependsOn 'd:d:1.0'
}
'c:c:1.0' {
dependsOn 'e:e:1.0'
}
'd:d:1.0'()
'e:e:1.0'()
'f:f:1.0'()
'g:g:1.0'()
}

repositoryInteractions {
expectResolved.each {
"${it}:${it}:1.0" { expectResolve() }
}
}

when:
succeedsDependencyResolution()

then:
def resolvedJars = expectResolved.collect { it + '-1.0.jar'}
assertResolvedFiles(resolvedJars)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ private void visitDependencies(ExcludeSpec resolutionFilter, Collection<EdgeStat
try {
collectAncestorsStrictVersions(incomingEdges);
for (DependencyState dependencyState : dependencies(resolutionFilter)) {
dependencyState = maybeSubstitute(dependencyState, resolveState.getDependencySubstitutionApplicator());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there side-effects now that the below methods (pending edge resolution, constraint activation) are operating on the un-substituted module instead of the substituted one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dependencies(resolutionFilter) now performs the substitution. So the below method still apply to the substituted dependency.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh, for some reason I had it in my mind that cacheFilteredDependencyStates was only called later on. Thanks for clarification 😄

PendingDependenciesVisitor.PendingState pendingState = pendingDepsVisitor.maybeAddAsPendingDependency(this, dependencyState);
if (dependencyState.getDependency().isConstraint()) {
registerActivatingConstraint(dependencyState);
Expand Down Expand Up @@ -507,6 +506,11 @@ private List<DependencyState> cacheFilteredDependencyStates(ExcludeSpec spec, Li
}
List<DependencyState> tmp = Lists.newArrayListWithCapacity(from.size());
for (DependencyState dependencyState : from) {
if (isExcluded(spec, dependencyState)) {
continue;
}
dependencyState = maybeSubstitute(dependencyState, resolveState.getDependencySubstitutionApplicator());

if (!isExcluded(spec, dependencyState)) {
tmp.add(dependencyState);
}
Expand Down