Skip to content

Commit

Permalink
Improve tracking of unattached dependencies
Browse files Browse the repository at this point in the history
When restarting an edge, it is possible that the new state fails to
compute the target node. In some cases, it is important to make sure the
detached edge is effectively tracked as an unattached edge.

Fixes #13251
Fixes #13316
  • Loading branch information
ljacomet committed Jun 29, 2020
1 parent 933a99e commit 7794fc3
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 9 deletions.
Expand Up @@ -640,6 +640,84 @@ class JavaPlatformResolveIntegrationTest extends AbstractHttpDependencyResolutio
platform << ["project(':platform')", "'org:other-platform:1.0'", "'org:bom-platform:1.0'"]
}
def 'platform deselection / reselection does not cause orphan edges'() {
given:
def depExcluded = mavenHttpRepo.module('org.test', 'excluded', '1.0').publish()
def depA = mavenHttpRepo.module('org.test', 'depA', '1.0').publish()
def platform = mavenHttpRepo.module('org.test', 'platform', '1.0').withModuleMetadata().withoutDefaultVariants()
.withVariant('apiElements') {
useDefaultArtifacts = false
attribute(Usage.USAGE_ATTRIBUTE.name, Usage.JAVA_API)
attribute(Category.CATEGORY_ATTRIBUTE.name, Category.REGULAR_PLATFORM)
constraint('org.test', 'depA', '1.0')
constraint('org.test', 'excluded', '1.0')
}
.withVariant('runtimeElements') {
useDefaultArtifacts = false
attribute(Usage.USAGE_ATTRIBUTE.name, Usage.JAVA_RUNTIME)
attribute(Category.CATEGORY_ATTRIBUTE.name, Category.REGULAR_PLATFORM)
constraint('org.test', 'depA', '1.0')
constraint('org.test', 'excluded', '1.0')
}.publish()
def depC = mavenHttpRepo.module('org.test', 'depC', '1.0').withModuleMetadata()
.withVariant('runtime') {
dependsOn('org.test', 'platform', '1.0', null, [(Category.CATEGORY_ATTRIBUTE.name): Category.REGULAR_PLATFORM])
}.publish()
def depB = mavenHttpRepo.module('org.test', 'depB', '1.0').dependsOn([exclusions: [[module: 'excluded']]], depC).publish()
def depF = mavenHttpRepo.module('org.test', 'depF', '1.0').dependsOn(depC).publish()
def depE = mavenHttpRepo.module('org.test', 'depE', '1.0').dependsOn(depF).publish()
def depD = mavenHttpRepo.module('org.test', 'depD', '1.0').dependsOn(depE).publish()
depExcluded.allowAll()
depA.allowAll()
depB.allowAll()
depC.allowAll()
depD.allowAll()
depE.allowAll()
depF.allowAll()
platform.allowAll()
buildFile << """
configurations {
conf.dependencies.clear()
}
dependencies {
conf 'org.test:depA'
conf 'org.test:depB:1.0'
conf 'org.test:depD:1.0'
}
"""
checkConfiguration("conf")
resolve.expectDefaultConfiguration("runtime")
when:
succeeds 'checkDeps'
then:
resolve.expectGraph {
root(":", "org.test:test:1.9") {
edge("org.test:depA", "org.test:depA:1.0")
module("org.test:depB:1.0") {
module("org.test:depC:1.0") {
module('org.test:platform:1.0') {
noArtifacts()
constraint('org.test:depA:1.0')
}
}
}
module('org.test:depD:1.0') {
module('org.test:depE:1.0') {
module('org.test:depF:1.0') {
module('org.test:depC:1.0')
}
}
}
}
}
}
private void checkConfiguration(String configuration) {
resolve = new ResolveTestFixture(buildFile, configuration)
resolve.expectDefaultConfiguration("compile")
Expand Down
Expand Up @@ -66,6 +66,7 @@ class EdgeState implements DependencyGraphEdge {
private ExcludeSpec cachedExclusions;

private ResolvedVariantResult resolvedVariant;
private boolean unattached;

EdgeState(NodeState from, DependencyState dependencyState, ExcludeSpec transitiveExclusions, ResolveState resolveState) {
this.from = from;
Expand Down Expand Up @@ -181,9 +182,13 @@ void failWith(Throwable err) {
targetNodeSelectionFailure = new ModuleVersionResolveException(dependencyState.getRequested(), err);
}

public void restart() {
public void restart(boolean checkUnattached) {
if (from.isSelected()) {
removeFromTargetConfigurations();
// We now have corner cases that can lead to this restart not succeeding
if (checkUnattached && !isUnattached()) {
selector.getTargetModule().addUnattachedDependency(this);
}
attachToTargetConfigurations();
}
}
Expand Down Expand Up @@ -430,4 +435,16 @@ public void updateTransitiveExcludes(ExcludeSpec newResolutionFilter) {
targetNode.updateTransitiveExcludes();
}
}

public void markUnattached() {
this.unattached = true;
}

public void markAttached() {
this.unattached = false;
}

public boolean isUnattached() {
return unattached;
}
}
Expand Up @@ -262,20 +262,23 @@ private void doRestart(ComponentState selected) {
private void restartUnattachedDependencies() {
if (unattachedDependencies.size() == 1) {
EdgeState singleDependency = unattachedDependencies.get(0);
singleDependency.restart();
singleDependency.restart(false);
} else {
for (EdgeState dependency : new ArrayList<>(unattachedDependencies)) {
dependency.restart();
dependency.restart(false);
}
}
}

public void addUnattachedDependency(EdgeState edge) {
unattachedDependencies.add(edge);
edge.markUnattached();
}

public void removeUnattachedDependency(EdgeState edge) {
unattachedDependencies.remove(edge);
if (unattachedDependencies.remove(edge)) {
edge.markAttached();
}
}

public ComponentState getVersion(ModuleVersionIdentifier id, ComponentIdentifier componentIdentifier) {
Expand Down
Expand Up @@ -610,7 +610,7 @@ public boolean isSelected() {

public void evict() {
evicted = true;
restartIncomingEdges();
restartIncomingEdges(false);
}

boolean shouldIncludedInGraphResult() {
Expand Down Expand Up @@ -971,18 +971,18 @@ public void restart(ComponentState selected) {
}
} else {
if (!incomingEdges.isEmpty()) {
restartIncomingEdges();
restartIncomingEdges(true);
}
}
}

private void restartIncomingEdges() {
private void restartIncomingEdges(boolean checkUnattached) {
if (incomingEdges.size() == 1) {
EdgeState singleEdge = incomingEdges.get(0);
singleEdge.restart();
singleEdge.restart(checkUnattached);
} else {
for (EdgeState dependency : new ArrayList<>(incomingEdges)) {
dependency.restart();
dependency.restart(checkUnattached);
}
}
clearIncomingEdges();
Expand Down

0 comments on commit 7794fc3

Please sign in to comment.