From 942bbcd91242d91d88ff4d616eb1e2b689a2c04f Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Tue, 4 Oct 2022 14:53:16 +0200 Subject: [PATCH] Add some more optimizations for exclude simplification Changes are based on real world use cases where keeping an allOf(group(s),module(s)) instead of making it a moduleId(Set) had pretty bad downstream consequences. Fixes #22358 --- .../excludes/factories/Intersections.java | 4 +++ .../NormalizingExcludeFactoryTest.groovy | 28 +++++++++++++++++++ .../factories/ExcludeJsonLogToCode.groovy | 18 +++++++++--- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/Intersections.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/Intersections.java index 259da3e4b25a..5a499e713a26 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/Intersections.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/Intersections.java @@ -295,6 +295,8 @@ private ExcludeSpec intersectGroupSet(GroupSetExclude left, ExcludeSpec right) { .filter(id -> groups.contains(id.getGroup())) .collect(toSet()); return moduleIdSet(filtered); + } else if (right instanceof ModuleSetExclude) { + return factory.moduleIdSet(groups.stream().flatMap(group -> ((ModuleSetExclude) right).getModules().stream().map(module -> DefaultModuleIdentifier.newId(group, module))).collect(toSet())); } return null; } @@ -332,6 +334,8 @@ private ExcludeSpec intersectModule(ModuleExclude left, ExcludeSpec right) { } else if (right instanceof ModuleIdSetExclude) { Set common = ((ModuleIdSetExclude) right).getModuleIds().stream().filter(id -> id.getName().equals(module)).collect(toSet()); return moduleIdSet(common); + } else if (right instanceof GroupSetExclude) { + return factory.moduleIdSet(((GroupSetExclude) right).getGroups().stream().map(group -> DefaultModuleIdentifier.newId(group, module)).collect(toSet())); } return null; } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/NormalizingExcludeFactoryTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/NormalizingExcludeFactoryTest.groovy index 2cabf6a87cc9..8d42be9bfe52 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/NormalizingExcludeFactoryTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/NormalizingExcludeFactoryTest.groovy @@ -16,7 +16,10 @@ package org.gradle.api.internal.artifacts.ivyservice.resolveengine.excludes.factories +import groovy.json.JsonSlurper +import org.codehaus.groovy.control.CompilerConfiguration import org.gradle.internal.component.model.DefaultIvyArtifactName +import spock.lang.Ignore import spock.lang.Shared import spock.lang.Specification import spock.lang.Unroll @@ -112,6 +115,9 @@ class NormalizingExcludeFactoryTest extends Specification implements ExcludeTest everything() | group("foo") | group("foo") nothing() | group("foo") | nothing() group("foo") | group("foo") | group("foo") + group("foo") | module("bar") | moduleId("foo", "bar") + groupSet("foo", "foz") | module("bar") | moduleIdSet("foo:bar", "foz:bar") + groupSet("foo", "foz") | moduleSet("bar", "baz") | moduleIdSet("foo:bar", "foz:bar", "foo:baz", "foz:baz") allOf(group("foo"), group("foo2")) | module("bar") | nothing() allOf(group("foo"), module("bar")) | module("bar") | moduleId("foo", "bar") moduleSet("m1", "m2", "m3") | moduleSet("m1", "m3") | moduleSet("m1", "m3") @@ -270,6 +276,28 @@ class NormalizingExcludeFactoryTest extends Specification implements ExcludeTest expect: operation + } + + @Ignore("to be used adhoc when analyzing specific issue") + def 'replay content from shared json formatted log'() { + def input = new File('/path/to/log.json') + def converter = new ExcludeJsonLogToCode() + + def slurper = new JsonSlurper() + + def compilerConf = new CompilerConfiguration() + compilerConf.scriptBaseClass = DelegatingScript.class.name + def shell = new GroovyShell(this.class.classLoader, new Binding(), compilerConf) + + expect: + input.readLines().eachWithIndex { line, index -> + def parsedLine = slurper.parse(line.trim().getBytes("utf-8")) + def script = shell.parse(converter.toCodeFromSlurpedJson(parsedLine)) + script.setDelegate(this) + def result = script.run() + + assert "$index" + slurper.parse(result.toString().getBytes("utf-8")).toString() == "$index" + parsedLine.operation.result.toString() + } } } diff --git a/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/ExcludeJsonLogToCode.groovy b/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/ExcludeJsonLogToCode.groovy index 102e578ad2a9..f0ae06f0cc38 100644 --- a/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/ExcludeJsonLogToCode.groovy +++ b/subprojects/dependency-management/src/testFixtures/groovy/org/gradle/api/internal/artifacts/ivyservice/resolveengine/excludes/factories/ExcludeJsonLogToCode.groovy @@ -100,8 +100,12 @@ class ExcludeJsonLogToCode { */ String toCode(String json) { def slurper = new JsonSlurper().parse(json.trim().getBytes("utf-8")) - def operation = slurper.operation.name - def operands = slurper.operation.operands.collect { e -> + toCodeFromSlurpedJson(slurper) + } + + String toCodeFromSlurpedJson(Object parsed) { + def operation = parsed.operation.name + def operands = parsed.operation.operands.collect { e -> "${toExclude(e)}" } """ @@ -113,6 +117,9 @@ def operation = factory.$operation([ } private String toExclude(it) { + if (it == 'excludes everything') { + return 'everything()' + } assert it instanceof Map assert it.keySet().size() == 1 String key = it.keySet()[0] @@ -148,16 +155,19 @@ def operation = factory.$operation([ case "exclude module id": def (group, name) = value.split(':') return "moduleId('${anonymize(group)}', '${anonymize(name)}')" - case "module set": - return "moduleSet('${anonymize(value)}')" + case "module names": + return "moduleSet(${value.collect { "'${anonymize(it)}'" }.join(', ')})" case "module ids": return "moduleIdSet(${value.collect { "'${anonymize(it)}'" }.join(', ')})" + case "groups": + return "groupSet(${value.collect { "'${anonymize(it)}'" }.join(', ')})" default: throw new UnsupportedOperationException("Not yet implemented for $opType") } } private String anonymize(String value) { + value value.split(":").collect { mappingCache[it] }.join(':')