Skip to content

Commit

Permalink
Rework exclude rule merging
Browse files Browse the repository at this point in the history
As a follow-up to #9197, this commit properly fixes the
exclude rule merging algorithm, by completely rewriting
it. The new merging algorithm works by implementing the
minimal set of algebra operations that make sense to
minimize computation durations. In order to do this,
this commit introduces a number of exclude specs
(found in their own package) and factories to create
actual implementation of those specs.

Specs represent the different kind of excludes we can
find:

- excluding a group
- excluding a module (no group defined)
- excluding a group+module
- excluding an artifact of a group+module
- pattern-matching excludes
- unions of excludes
- intersections of excludes

With all those minimal bricks, factories are responsible
of generating consistent specs. The dumbest factory
will just generate new instances for everything. This
is the default factory.

Minimally, this factory has to be backed by an optimizing
factory, which will take care of handling special cases:

- union or intersection of a single spec
- union or intersection of 2 specs
   - when one of them is null
   - when both are equal

Then we have a factory which performs the minimal algebra
to minimize specs:

- unions of unions
- intersections of intersections
- union of a union and individual specs
- insection of an intersection and individual spec
- ...

This factory can be as smart as it can, but one must be
careful that it's worth it: some previously implemented
optimizations (like (A+B).A = A turned out to be costly
to detect, and didn't make it the final cut.

Yet another factory is there to reduce the memory footprint
and, as a side effect, make things faster by interning
the specs: equivalent specs are interned and indexed, which
allows us to optimize unions and intersections of specs.

Last but not least, a caching factory is there to avoid
recomputing the same intersections and unions of specs
when we have already done the job. This is efficient if
the underlying (delegate) specs are easily compared,
which is the case thanks to the interning factory.

All in all, the delegation chain allows us to make
the algorithm fast and hopefully reliable, while
making it easier to debug.
  • Loading branch information
eskatos authored and melix committed Apr 27, 2019
1 parent 4056e83 commit 9b3d1c7
Show file tree
Hide file tree
Showing 89 changed files with 3,189 additions and 2,403 deletions.
Expand Up @@ -38,6 +38,7 @@ open class MinifyPlugin : Plugin<Project> {
val keepPatterns = mapOf(
"fastutil" to setOf(
"it.unimi.dsi.fastutil.ints.IntOpenHashSet",
"it.unimi.dsi.fastutil.ints.IntRBTreeSet",
"it.unimi.dsi.fastutil.ints.IntSets"
)
)
Expand Down
Expand Up @@ -16,9 +16,11 @@

package org.gradle.integtests.resolve


import org.gradle.integtests.fixtures.executer.GradleContextualExecuter
import spock.lang.IgnoreIf
import spock.lang.Issue
import spock.lang.Unroll

/**
* Demonstrates the resolution of dependency excludes in published module metadata.
Expand Down Expand Up @@ -109,7 +111,8 @@ task check(type: Sync) {
*
* Exclude is applied to dependency a->b
*/
def "dependency exclude for group or module applies to child module of dependency"() {
@Unroll
def "dependency exclude for group or module applies to child module of dependency (#excluded)"() {
given:
def expectResolved = ['a', 'b', 'c', 'd', 'e'] - expectExcluded
repository {
Expand Down Expand Up @@ -202,7 +205,8 @@ task check(type: Sync) {
*
* Selective exclusions are applied to dependency a->b
*/
def "can exclude transitive dependencies"() {
@Unroll
def "can exclude transitive dependencies (#condition)"() {
repository {
'a:a:1.0' {
dependsOn group: 'b', artifact: 'b', version: '1.0', exclusions: excludes
Expand Down
Expand Up @@ -169,8 +169,8 @@ BuildCommencedTimeProvider createBuildTimeProvider() {
return new BuildCommencedTimeProvider();
}

ModuleExclusions createModuleExclusions(ImmutableModuleIdentifierFactory moduleIdentifierFactory) {
return new ModuleExclusions(moduleIdentifierFactory);
ModuleExclusions createModuleExclusions() {
return new ModuleExclusions();
}

MavenMutableModuleMetadataFactory createMutableMavenMetadataFactory(ImmutableModuleIdentifierFactory moduleIdentifierFactory,
Expand Down
Expand Up @@ -19,7 +19,7 @@
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelector;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ArtifactSet;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.ModuleExclusion;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.specs.ExcludeSpec;
import org.gradle.api.internal.artifacts.type.ArtifactTypeRegistry;
import org.gradle.api.internal.attributes.ImmutableAttributes;
import org.gradle.api.internal.component.ArtifactType;
Expand Down Expand Up @@ -82,7 +82,7 @@ public boolean isFetchingMetadataCheap(ComponentIdentifier identifier) {

@Nullable
@Override
public ArtifactSet resolveArtifacts(ComponentResolveMetadata component, ConfigurationMetadata configuration, ArtifactTypeRegistry artifactTypeRegistry, ModuleExclusion exclusions, ImmutableAttributes overriddenAttributes) {
public ArtifactSet resolveArtifacts(ComponentResolveMetadata component, ConfigurationMetadata configuration, ArtifactTypeRegistry artifactTypeRegistry, ExcludeSpec exclusions, ImmutableAttributes overriddenAttributes) {
throw new UnsupportedOperationException();
}

Expand Down
Expand Up @@ -17,7 +17,7 @@

import org.gradle.api.attributes.Category;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ArtifactSet;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.ModuleExclusion;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.specs.ExcludeSpec;
import org.gradle.api.internal.artifacts.repositories.metadata.MavenImmutableAttributesFactory;
import org.gradle.api.internal.artifacts.type.ArtifactTypeRegistry;
import org.gradle.api.internal.attributes.AttributeValue;
Expand Down Expand Up @@ -60,7 +60,7 @@ public void resolveArtifactsWithType(ComponentResolveMetadata component, Artifac

@Nullable
@Override
public ArtifactSet resolveArtifacts(ComponentResolveMetadata component, ConfigurationMetadata configuration, ArtifactTypeRegistry artifactTypeRegistry, ModuleExclusion exclusions, ImmutableAttributes overriddenAttributes) {
public ArtifactSet resolveArtifacts(ComponentResolveMetadata component, ConfigurationMetadata configuration, ArtifactTypeRegistry artifactTypeRegistry, ExcludeSpec exclusions, ImmutableAttributes overriddenAttributes) {
if (component.getSource() == null) {
// virtual components have no source
return NO_ARTIFACTS;
Expand Down
Expand Up @@ -26,7 +26,7 @@
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ArtifactSet;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.DefaultArtifactSet;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ResolvableArtifact;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.ModuleExclusion;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.specs.ExcludeSpec;
import org.gradle.api.internal.artifacts.type.ArtifactTypeRegistry;
import org.gradle.api.internal.attributes.ImmutableAttributes;
import org.gradle.api.internal.component.ArtifactType;
Expand Down Expand Up @@ -131,7 +131,7 @@ public void resolveArtifactsWithType(ComponentResolveMetadata component, Artifac

@Nullable
@Override
public ArtifactSet resolveArtifacts(final ComponentResolveMetadata component, final ConfigurationMetadata configuration, final ArtifactTypeRegistry artifactTypeRegistry, final ModuleExclusion exclusions, final ImmutableAttributes overriddenAttributes) {
public ArtifactSet resolveArtifacts(final ComponentResolveMetadata component, final ConfigurationMetadata configuration, final ArtifactTypeRegistry artifactTypeRegistry, final ExcludeSpec exclusions, final ImmutableAttributes overriddenAttributes) {
if (isProjectModule(component.getId())) {
return DefaultArtifactSet.multipleVariants(component.getId(), component.getModuleVersionId(), component.getSource(), exclusions, configuration.getVariants(), component.getAttributesSchema(), self, allProjectArtifacts, artifactTypeRegistry, overriddenAttributes);
} else {
Expand Down
Expand Up @@ -23,7 +23,7 @@
import org.gradle.api.artifacts.component.ComponentArtifactIdentifier;
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.internal.artifacts.DefaultResolvedArtifact;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.ModuleExclusion;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.specs.ExcludeSpec;
import org.gradle.api.internal.artifacts.transform.VariantSelector;
import org.gradle.api.internal.artifacts.type.ArtifactTypeRegistry;
import org.gradle.api.internal.attributes.AttributesSchemaInternal;
Expand Down Expand Up @@ -71,7 +71,7 @@ public ImmutableAttributes getOverriddenAttributes() {
return selectionAttributes;
}

public static ArtifactSet multipleVariants(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier ownerId, ModuleSource moduleSource, ModuleExclusion exclusions, Set<? extends VariantResolveMetadata> variants, AttributesSchemaInternal schema, ArtifactResolver artifactResolver, Map<ComponentArtifactIdentifier, ResolvableArtifact> allResolvedArtifacts, ArtifactTypeRegistry artifactTypeRegistry, ImmutableAttributes selectionAttributes) {
public static ArtifactSet multipleVariants(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier ownerId, ModuleSource moduleSource, ExcludeSpec exclusions, Set<? extends VariantResolveMetadata> variants, AttributesSchemaInternal schema, ArtifactResolver artifactResolver, Map<ComponentArtifactIdentifier, ResolvableArtifact> allResolvedArtifacts, ArtifactTypeRegistry artifactTypeRegistry, ImmutableAttributes selectionAttributes) {
if (variants.size() == 1) {
VariantResolveMetadata variantMetadata = variants.iterator().next();
ResolvedVariant resolvedVariant = toResolvedVariant(variantMetadata, ownerId, moduleSource, exclusions, artifactResolver, allResolvedArtifacts, artifactTypeRegistry);
Expand All @@ -85,13 +85,13 @@ public static ArtifactSet multipleVariants(ComponentIdentifier componentIdentifi
return new MultipleVariantArtifactSet(componentIdentifier, schema, result.build(), selectionAttributes);
}

public static ArtifactSet singleVariant(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier ownerId, DisplayName displayName, Collection<? extends ComponentArtifactMetadata> artifacts, ModuleSource moduleSource, ModuleExclusion exclusions, AttributesSchemaInternal schema, ArtifactResolver artifactResolver, Map<ComponentArtifactIdentifier, ResolvableArtifact> allResolvedArtifacts, ArtifactTypeRegistry artifactTypeRegistry, ImmutableAttributes selectionAttributes) {
public static ArtifactSet singleVariant(ComponentIdentifier componentIdentifier, ModuleVersionIdentifier ownerId, DisplayName displayName, Collection<? extends ComponentArtifactMetadata> artifacts, ModuleSource moduleSource, ExcludeSpec exclusions, AttributesSchemaInternal schema, ArtifactResolver artifactResolver, Map<ComponentArtifactIdentifier, ResolvableArtifact> allResolvedArtifacts, ArtifactTypeRegistry artifactTypeRegistry, ImmutableAttributes selectionAttributes) {
VariantResolveMetadata variantMetadata = new DefaultVariantMetadata(displayName, ImmutableAttributes.EMPTY, ImmutableList.copyOf(artifacts), ImmutableCapabilities.EMPTY);
ResolvedVariant resolvedVariant = toResolvedVariant(variantMetadata, ownerId, moduleSource, exclusions, artifactResolver, allResolvedArtifacts, artifactTypeRegistry);
return new SingleVariantArtifactSet(componentIdentifier, schema, resolvedVariant, selectionAttributes);
}

private static ResolvedVariant toResolvedVariant(VariantResolveMetadata variant, ModuleVersionIdentifier ownerId, ModuleSource moduleSource, ModuleExclusion exclusions, ArtifactResolver artifactResolver, Map<ComponentArtifactIdentifier, ResolvableArtifact> allResolvedArtifacts, ArtifactTypeRegistry artifactTypeRegistry) {
private static ResolvedVariant toResolvedVariant(VariantResolveMetadata variant, ModuleVersionIdentifier ownerId, ModuleSource moduleSource, ExcludeSpec exclusions, ArtifactResolver artifactResolver, Map<ComponentArtifactIdentifier, ResolvableArtifact> allResolvedArtifacts, ArtifactTypeRegistry artifactTypeRegistry) {
List<? extends ComponentArtifactMetadata> artifacts = variant.getArtifacts();
ImmutableSet.Builder<ResolvableArtifact> resolvedArtifacts = ImmutableSet.builder();

Expand All @@ -100,7 +100,7 @@ private static ResolvedVariant toResolvedVariant(VariantResolveMetadata variant,

for (ComponentArtifactMetadata artifact : artifacts) {
IvyArtifactName artifactName = artifact.getName();
if (exclusions.excludeArtifact(ownerId.getModule(), artifactName)) {
if (exclusions.excludesArtifact(ownerId.getModule(), artifactName)) {
continue;
}

Expand Down
Expand Up @@ -17,7 +17,7 @@
package org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact;

import com.google.common.collect.Maps;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.ModuleExclusion;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.specs.ExcludeSpec;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.DependencyGraphEdge;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.DependencyGraphNode;
import org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.DependencyGraphSelector;
Expand Down Expand Up @@ -96,7 +96,7 @@ private ArtifactsForNode getArtifacts(DependencyGraphEdge dependency, Dependency

ArtifactsForNode configurationArtifactSet = artifactsByNodeId.get(toConfiguration.getNodeId());
if (configurationArtifactSet == null) {
ModuleExclusion exclusions = dependency.getExclusions();
ExcludeSpec exclusions = dependency.getExclusions();
ArtifactSet nodeArtifacts = artifactSelector.resolveArtifacts(component, targetConfiguration, exclusions, overriddenAttributes);
int id = nextId++;
configurationArtifactSet = new ArtifactsForNode(id, nodeArtifacts);
Expand Down

This file was deleted.

This file was deleted.

0 comments on commit 9b3d1c7

Please sign in to comment.