Skip to content

Commit

Permalink
Clarifying renames
Browse files Browse the repository at this point in the history 
The exclude rule merging algorithms are pretty hard to understand
due to the mental shift required by weird definitions of "union"
and "intersection" which are the exact opposite of what they should
be.
  • Loading branch information
@melix
melix committed Apr 24, 2019
1 parent f747fc2 commit 88972af
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 116 deletions.
4 changes: 2 additions & 2 deletions ...dle/api/internal/artifacts/ivyservice/resolveengine/excludes/AbstractModuleExclusion.java
View file
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
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
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
44 changes: 22 additions & 22 deletions ...org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/ModuleExclusions.java
View file
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
2 changes: 1 addition & 1 deletion ...a/org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/EdgeState.java
View file
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
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

0 comments on commit 88972af

Please sign in to comment.