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.

Sign up for GitHub

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

Jump to bottom

Fix inconsistent classpath ordering when dependencies have lots of excludes #9197

Merged
merged 2 commits into from Apr 24, 2019
Merged

Fix inconsistent classpath ordering when dependencies have lots of excludes #9197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88dbb29
Use consistent ordering for module exclusions
melix Apr 23, 2019
f2093f2
Clarifying renames
melix Apr 24, 2019
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
4 changes: 2 additions & 2 deletions ...dle/api/internal/artifacts/ivyservice/resolveengine/excludes/AbstractModuleExclusion.java
View file
Open in desktop
Expand Up @@ -81,14 +81,14 @@ protected boolean excludesNoModules() {
* Possibly unpack a composite spec into it's constituent parts, if those parts are applied as an intersection.
* @param specs
*/
protected void unpackIntersection(Collection<AbstractModuleExclusion> specs) {
protected void unpackEither(Collection<AbstractModuleExclusion> specs) {
specs.add(this);
}

/**
* Possibly unpack a composite spec into it's constituent parts, if those parts are applied as a union.
*/
protected void unpackUnion(Collection<AbstractModuleExclusion> specs) {
protected void unpackAll(Collection<AbstractModuleExclusion> specs) {
specs.add(this);
}

Expand Down
6 changes: 3 additions & 3 deletions ...esolveengine/excludes/UnionExclusion.java → .../resolveengine/excludes/AllExclusion.java
View file
Open in desktop
Expand Up @@ -26,10 +26,10 @@
* A filter that only excludes artifacts and modules that are excluded by _all_ of the supplied exclude rules.
* As such, this is a union of the separate exclude rule filters.
*/
class UnionExclusion extends AbstractCompositeExclusion {
class AllExclusion extends AbstractCompositeExclusion {
private final List<AbstractModuleExclusion> filters;

public UnionExclusion(List<AbstractModuleExclusion> filters) {
public AllExclusion(List<AbstractModuleExclusion> filters) {
this.filters = filters;
}

Expand All @@ -41,7 +41,7 @@ Collection<AbstractModuleExclusion> getFilters() {
* Can unpack into constituents when creating a larger union.
*/
@Override
protected void unpackUnion(Collection<AbstractModuleExclusion> specs) {
protected void unpackAll(Collection<AbstractModuleExclusion> specs) {
specs.addAll(this.filters);
}

Expand Down
6 changes: 3 additions & 3 deletions ...ngine/excludes/IntersectionExclusion.java → ...solveengine/excludes/EitherExclusion.java
View file
Open in desktop
Expand Up @@ -25,13 +25,13 @@
* A spec that excludes modules or artifacts that are excluded by _any_ of the supplied exclusions.
* As such, this is an intersection of the separate exclude rule filters.
*/
class IntersectionExclusion extends AbstractCompositeExclusion {
class EitherExclusion extends AbstractCompositeExclusion {
private final ImmutableModuleExclusionSet excludeSpecs;
private final boolean mergeable;

private Boolean excludesNoModules;

public IntersectionExclusion(ImmutableModuleExclusionSet specs) {
public EitherExclusion(ImmutableModuleExclusionSet specs) {
this.excludeSpecs = specs;
boolean canMerge = true;
for (AbstractModuleExclusion spec : specs) {
Expand Down Expand Up @@ -92,7 +92,7 @@ public boolean mayExcludeArtifacts() {
* @param specs
*/
@Override
protected void unpackIntersection(Collection<AbstractModuleExclusion> specs) {
protected void unpackEither(Collection<AbstractModuleExclusion> specs) {
specs.addAll(excludeSpecs);
}

Expand Down
48 changes: 24 additions & 24 deletions ...org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/ModuleExclusions.java
View file
Open in desktop
Expand Up @@ -29,7 +29,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -59,7 +59,7 @@ public class ModuleExclusions {

private final Map<MergeOperation, AbstractModuleExclusion> mergeCache = Maps.newConcurrentMap();
private final Map<ImmutableList<ExcludeMetadata>, AbstractModuleExclusion> excludeAnyCache = Maps.newConcurrentMap();
private final Map<ImmutableSet<AbstractModuleExclusion>, IntersectionExclusion> intersectionCache = Maps.newConcurrentMap();
private final Map<ImmutableSet<AbstractModuleExclusion>, EitherExclusion> eitherCache = Maps.newConcurrentMap();
private final Map<AbstractModuleExclusion[], Map<AbstractModuleExclusion[], MergeOperation>> mergeOperationCache = Maps.newIdentityHashMap();
private final Map<ModuleIdentifier, ModuleIdExcludeSpec> moduleIdSpecs = Maps.newConcurrentMap();
private final Map<String, ModuleNameExcludeSpec> moduleNameSpecs = Maps.newConcurrentMap();
Expand Down Expand Up @@ -103,7 +103,7 @@ public ModuleExclusion excludeAny(ImmutableList<ExcludeMetadata> excludes) {
for (ExcludeMetadata exclude : excludes) {
exclusions.add(forExclude(exclude));
}
exclusion = asIntersection(exclusions.build());
exclusion = asEither(exclusions.build());
excludeAnyCache.put(excludes, exclusion);
return exclusion;
}
Expand Down Expand Up @@ -165,7 +165,7 @@ private GroupNameExcludeSpec groupNameExcludeSpec(String id) {
/**
* Returns a spec that excludes those modules and artifacts that are excluded by _either_ of the given exclude rules.
*/
public ModuleExclusion intersect(ModuleExclusion one, ModuleExclusion two) {
public ModuleExclusion either(ModuleExclusion one, ModuleExclusion two) {
if (one == two) {
return one;
}
Expand All @@ -179,9 +179,9 @@ public ModuleExclusion intersect(ModuleExclusion one, ModuleExclusion two) {
return one;
}

if (one instanceof IntersectionExclusion && ((IntersectionExclusion) one).getFilters().contains(two)) {
if (one instanceof EitherExclusion && ((EitherExclusion) one).getFilters().contains(two)) {
return one;
} else if (two instanceof IntersectionExclusion && ((IntersectionExclusion) two).getFilters().contains(one)) {
} else if (two instanceof EitherExclusion && ((EitherExclusion) two).getFilters().contains(one)) {
return two;
}

Expand All @@ -190,10 +190,10 @@ public ModuleExclusion intersect(ModuleExclusion one, ModuleExclusion two) {

List<AbstractModuleExclusion> builder = Lists.newArrayListWithExpectedSize(estimateSize(aOne) + estimateSize(aTwo));

aOne.unpackIntersection(builder);
aTwo.unpackIntersection(builder);
aOne.unpackEither(builder);
aTwo.unpackEither(builder);

return asIntersection(ImmutableSet.copyOf(builder));
return asEither(ImmutableSet.copyOf(builder));
}

private static int estimateSize(AbstractModuleExclusion ex) {
Expand All @@ -206,7 +206,7 @@ private static int estimateSize(AbstractModuleExclusion ex) {
/**
* Returns a spec that excludes only those modules and artifacts that are excluded by _both_ of the supplied exclude rules.
*/
public ModuleExclusion union(ModuleExclusion one, ModuleExclusion two) {
public ModuleExclusion both(ModuleExclusion one, ModuleExclusion two) {
if (one == two) {
return one;
}
Expand All @@ -218,15 +218,15 @@ public ModuleExclusion union(ModuleExclusion one, ModuleExclusion two) {
}

List<AbstractModuleExclusion> specs = new ArrayList<AbstractModuleExclusion>();
((AbstractModuleExclusion) one).unpackUnion(specs);
((AbstractModuleExclusion) two).unpackUnion(specs);
((AbstractModuleExclusion) one).unpackAll(specs);
((AbstractModuleExclusion) two).unpackAll(specs);
for (int i = 0; i < specs.size();) {
AbstractModuleExclusion spec = specs.get(i);
AbstractModuleExclusion merged = null;
// See if we can merge any of the following specs into one
for (int j = i + 1; j < specs.size(); j++) {
AbstractModuleExclusion other = specs.get(j);
merged = maybeMergeIntoUnion(spec, other);
merged = maybeMergeIntoAll(spec, other);
if (merged != null) {
specs.remove(j);
break;
Expand All @@ -241,24 +241,24 @@ public ModuleExclusion union(ModuleExclusion one, ModuleExclusion two) {
if (specs.size() == 1) {
return specs.get(0);
}
return new UnionExclusion(specs);
return new AllExclusion(specs);
}

/**
* Attempt to merge 2 exclusions into a single filter that is the union of both.
* Currently this is only implemented when both exclusions are `IntersectionExclusion`s.
*/
private AbstractModuleExclusion maybeMergeIntoUnion(AbstractModuleExclusion one, AbstractModuleExclusion two) {
private AbstractModuleExclusion maybeMergeIntoAll(AbstractModuleExclusion one, AbstractModuleExclusion two) {
if (one.equals(two)) {
return one;
}
if (one instanceof IntersectionExclusion && two instanceof IntersectionExclusion) {
return maybeMergeIntoUnion((IntersectionExclusion) one, (IntersectionExclusion) two);
if (one instanceof EitherExclusion && two instanceof EitherExclusion) {
return maybeMergeIntoAll((EitherExclusion) one, (EitherExclusion) two);
}
return null;
}

private AbstractModuleExclusion maybeMergeIntoUnion(IntersectionExclusion one, IntersectionExclusion other) {
private AbstractModuleExclusion maybeMergeIntoAll(EitherExclusion one, EitherExclusion other) {
if (one.equals(other)) {
return one;
}
Expand Down Expand Up @@ -315,17 +315,17 @@ private AbstractModuleExclusion mergeAndCacheResult(MergeOperation merge, Abstra
if (merged.isEmpty()) {
exclusion = ModuleExclusions.EXCLUDE_NONE;
} else {
exclusion = asIntersection(ImmutableSet.copyOf(merged));
exclusion = asEither(ImmutableSet.copyOf(merged));
}
mergeCache.put(merge, exclusion);
return exclusion;
}

private IntersectionExclusion asIntersection(ImmutableSet<AbstractModuleExclusion> excludes) {
IntersectionExclusion cached = intersectionCache.get(excludes);
private EitherExclusion asEither(ImmutableSet<AbstractModuleExclusion> excludes) {
EitherExclusion cached = eitherCache.get(excludes);
if (cached == null) {
cached = new IntersectionExclusion(new ImmutableModuleExclusionSet(excludes));
intersectionCache.put(excludes, cached);
cached = new EitherExclusion(new ImmutableModuleExclusionSet(excludes));
eitherCache.put(excludes, cached);
}
return cached;
}
Expand Down Expand Up @@ -444,7 +444,7 @@ public int hashCode() {
}
}

private static final class MergeSet extends HashSet<AbstractModuleExclusion> {
private static final class MergeSet extends LinkedHashSet<AbstractModuleExclusion> {
private final BitSet remaining;
private int idx;
private AbstractModuleExclusion current;
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/EdgeState.java
View file
Open in desktop
Expand Up @@ -232,7 +232,7 @@ public ModuleExclusion getExclusions() {
return transitiveExclusions;
}
ModuleExclusion edgeExclusions = resolveState.getModuleExclusions().excludeAny(ImmutableList.copyOf(excludes));
return resolveState.getModuleExclusions().intersect(edgeExclusions, transitiveExclusions);
return resolveState.getModuleExclusions().either(edgeExclusions, transitiveExclusions);
}

public ModuleExclusion getEdgeExclusions() {
Expand Down
4 changes: 2 additions & 2 deletions ...a/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/NodeState.java
View file
Open in desktop
Expand Up @@ -469,7 +469,7 @@ private ModuleExclusion excludedByBoth(ModuleExclusion one, ModuleExclusion two)
if (two == null) {
return one;
}
return resolveState.getModuleExclusions().union(one, two);
return resolveState.getModuleExclusions().both(one, two);
}

private ModuleExclusion excludedByEither(ModuleExclusion one, ModuleExclusion two) {
Expand All @@ -479,7 +479,7 @@ private ModuleExclusion excludedByEither(ModuleExclusion one, ModuleExclusion tw
if (two == null) {
return one;
}
return resolveState.getModuleExclusions().intersect(one, two);
return resolveState.getModuleExclusions().either(one, two);
}

private void removeOutgoingEdges() {
Expand Down