From bce12182b4de2484ca827b6634e95641ca4f9606 Mon Sep 17 00:00:00 2001 From: Artem Chubaryan Date: Mon, 6 Jan 2020 17:38:36 -0600 Subject: [PATCH 1/6] Optimizations to relocation checks. Measured to give almost 3x performance boost on large project (5:48 -> 2:02 min total duration). - Changed signature of methods to accept String as path instead of RelocatePathContext/RelocateClassContext to prevent unnecessary allocations for builder objects - Added extra string length checks to avoid string operations where possible - Reordered checks in canRelocateClass so that expensive string replacement is the last operation - (Main Optimization) Avoided string concatenation (which led to creating StringBuilder instances and expensive array copy operations) by adding check for first character being '/': if so - perform a startsWith check with an offset. - Removed an incorrect length check - Moved context creation outside the loop --- .../shadow/impl/RelocatorRemapper.groovy | 12 +- .../shadow/relocation/Relocator.groovy | 4 +- .../shadow/relocation/SimpleRelocator.groovy | 24 ++-- .../Log4j2PluginsCacheFileTransformer.groovy | 3 +- .../ServiceFileTransformer.groovy | 6 +- .../relocation/SimpleRelocatorTest.groovy | 117 ++++++++++-------- 6 files changed, 94 insertions(+), 72 deletions(-) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/impl/RelocatorRemapper.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/impl/RelocatorRemapper.groovy index 953cacbe6..b994463d1 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/impl/RelocatorRemapper.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/impl/RelocatorRemapper.groovy @@ -65,13 +65,13 @@ class RelocatorRemapper extends Remapper { name = m.group(2) } - RelocateClassContext classContext = RelocateClassContext.builder().className(name).stats(stats).build() - RelocatePathContext pathContext = RelocatePathContext.builder().path(name).stats(stats).build() for (Relocator r : relocators) { - if (r.canRelocateClass(classContext)) { + if (r.canRelocateClass(name)) { + RelocateClassContext classContext = RelocateClassContext.builder().className(name).stats(stats).build() value = prefix + r.relocateClass(classContext) + suffix break - } else if (r.canRelocatePath(pathContext)) { + } else if (r.canRelocatePath(name)) { + RelocatePathContext pathContext = RelocatePathContext.builder().path(name).stats(stats).build() value = prefix + r.relocatePath(pathContext) + suffix break } @@ -96,9 +96,9 @@ class RelocatorRemapper extends Remapper { name = m.group(2) } - RelocatePathContext pathContext = RelocatePathContext.builder().path(name).stats(stats).build() for (Relocator r : relocators) { - if (r.canRelocatePath(pathContext)) { + if (r.canRelocatePath(name)) { + RelocatePathContext pathContext = RelocatePathContext.builder().path(name).stats(stats).build() value = prefix + r.relocatePath(pathContext) + suffix break } diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.groovy index 32fcc3a0f..d9cb1d48f 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.groovy @@ -28,11 +28,11 @@ package com.github.jengelman.gradle.plugins.shadow.relocation interface Relocator { String ROLE = Relocator.class.getName() - boolean canRelocatePath(RelocatePathContext context) + boolean canRelocatePath(String path) String relocatePath(RelocatePathContext context) - boolean canRelocateClass(RelocateClassContext context) + boolean canRelocateClass(String className) String relocateClass(RelocateClassContext context) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy index 36b22a128..9e5d8cb08 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy @@ -143,27 +143,37 @@ class SimpleRelocator implements Relocator { return false } - boolean canRelocatePath(RelocatePathContext context) { - String path = context.path + boolean canRelocatePath(String path) { if (rawString) { return Pattern.compile(pathPattern).matcher(path).find() } + // If string is too short - no need to perform expensive string operations + if (path.length() < pathPattern.length()) { + return false + } + if (path.endsWith(".class")) { + // Safeguard against strings containing only ".class" + if (path.length() == 6) { + return false + } path = path.substring(0, path.length() - 6) } // Allow for annoying option of an extra / on the front of a path. See MSHADE-119 comes from getClass().getResource("/a/b/c.properties"). - if (path.startsWith(pathPattern) || path.startsWith("/" + pathPattern)) { + boolean pathStartsWithPattern = + path.charAt(0) == '/' ? path.startsWith(pathPattern, 1) : path.startsWith(pathPattern) + if (pathStartsWithPattern) { return isIncluded(path) && !isExcluded(path) } return false } - boolean canRelocateClass(RelocateClassContext context) { - String clazz = context.className - RelocatePathContext pathContext = RelocatePathContext.builder().path(clazz.replace('.', '/')).stats(context.stats).build() - return !rawString && clazz.indexOf('/') < 0 && canRelocatePath(pathContext) + boolean canRelocateClass(String className) { + return !rawString && + className.indexOf('/') < 0 && + canRelocatePath(className.replace('.', '/')) } String relocatePath(RelocatePathContext context) { diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy index 7414d4d84..b82e210e1 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy @@ -115,8 +115,7 @@ class Log4j2PluginsCacheFileTransformer implements Transformer { RelocateClassContext relocateClassContext = new RelocateClassContext(className, stats) for (Relocator currentRelocator : relocators) { // If we have a relocator that can relocate our current entry... - boolean canRelocateClass = currentRelocator.canRelocateClass(relocateClassContext) - if (canRelocateClass) { + if (currentRelocator.canRelocateClass(className)) { // Then we perform that relocation and update the plugin entry to reflect the new value. String relocatedClassName = currentRelocator.relocateClass(relocateClassContext) currentPluginEntry.setClassName(relocatedClassName) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/ServiceFileTransformer.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/ServiceFileTransformer.groovy index 418360c5a..acbe21e84 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/ServiceFileTransformer.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/ServiceFileTransformer.groovy @@ -71,12 +71,12 @@ class ServiceFileTransformer implements Transformer, PatternFilterable { def lines = context.is.readLines() def targetPath = context.path context.relocators.each {rel -> - if(rel.canRelocateClass(RelocateClassContext.builder().className(new File(targetPath).name).stats(context.stats).build())) { + if (rel.canRelocateClass(new File(targetPath).name)) { targetPath = rel.relocateClass(RelocateClassContext.builder().className(targetPath).stats(context.stats).build()) } lines.eachWithIndex { String line, int i -> - def lineContext = RelocateClassContext.builder().className(line).stats(context.stats).build() - if(rel.canRelocateClass(lineContext)) { + if (rel.canRelocateClass(line)) { + def lineContext = RelocateClassContext.builder().className(line).stats(context.stats).build() lines[i] = rel.relocateClass(lineContext) } } diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy index 5009de155..52ee9a89f 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy @@ -46,78 +46,91 @@ class SimpleRelocatorTest extends TestCase { SimpleRelocator relocator relocator = new SimpleRelocator("org.foo", null, null, null) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/Class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/Class.class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/bar/Class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/bar/Class.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("com/foo/bar/Class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("com/foo/bar/Class.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/Foo/Class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/Foo/Class.class"))) + assertEquals(true, relocator.canRelocatePath("org/foo/Class")) + assertEquals(true, relocator.canRelocatePath("org/foo/Class.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/bar/Class")) + assertEquals(true, relocator.canRelocatePath("org/foo/bar/Class.class")) + assertEquals(false, relocator.canRelocatePath("com/foo/bar/Class")) + assertEquals(false, relocator.canRelocatePath("com/foo/bar/Class.class")) + assertEquals(false, relocator.canRelocatePath("org/Foo/Class")) + assertEquals(false, relocator.canRelocatePath("org/Foo/Class.class")) + + // Verify paths starting with '/' + assertEquals(false, relocator.canRelocatePath("/org/Foo/Class")) + assertEquals(false, relocator.canRelocatePath("/org/Foo/Class.class")) relocator = new SimpleRelocator("org.foo", null, null, Arrays.asList( [ "org.foo.Excluded", "org.foo.public.*", "org.foo.recurse.**", "org.foo.Public*Stuff" ] as String[])) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/Class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/Class.class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/excluded"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/Excluded"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/Excluded.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/public"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/public/Class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/public/Class.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/public/sub"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/public/sub/Class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/publicRELOC/Class"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/PrivateStuff"))) - assertEquals(true, relocator.canRelocatePath(pathContext("org/foo/PrivateStuff.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/PublicStuff"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/PublicStuff.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/PublicUtilStuff"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/PublicUtilStuff.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/recurse"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/recurse/Class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/recurse/Class.class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/recurse/sub"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/recurse/sub/Class"))) - assertEquals(false, relocator.canRelocatePath(pathContext("org/foo/recurse/sub/Class.class"))) + assertEquals(true, relocator.canRelocatePath("org/foo/Class")) + assertEquals(true, relocator.canRelocatePath("org/foo/Class.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/excluded")) + assertEquals(false, relocator.canRelocatePath("org/foo/Excluded")) + assertEquals(false, relocator.canRelocatePath("org/foo/Excluded.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/public")) + assertEquals(false, relocator.canRelocatePath("org/foo/public/Class")) + assertEquals(false, relocator.canRelocatePath("org/foo/public/Class.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/public/sub")) + assertEquals(true, relocator.canRelocatePath("org/foo/public/sub/Class")) + assertEquals(true, relocator.canRelocatePath("org/foo/publicRELOC/Class")) + assertEquals(true, relocator.canRelocatePath("org/foo/PrivateStuff")) + assertEquals(true, relocator.canRelocatePath("org/foo/PrivateStuff.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/PublicStuff")) + assertEquals(false, relocator.canRelocatePath("org/foo/PublicStuff.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/PublicUtilStuff")) + assertEquals(false, relocator.canRelocatePath("org/foo/PublicUtilStuff.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/recurse")) + assertEquals(false, relocator.canRelocatePath("org/foo/recurse/Class")) + assertEquals(false, relocator.canRelocatePath("org/foo/recurse/Class.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/recurse/sub")) + assertEquals(false, relocator.canRelocatePath("org/foo/recurse/sub/Class")) + assertEquals(false, relocator.canRelocatePath("org/foo/recurse/sub/Class.class")) + + // Verify edge cases + relocator = new SimpleRelocator("org.f", null, null, null) + assertEquals(false, relocator.canRelocatePath("")) // Empty path + assertEquals(false, relocator.canRelocatePath(".class")) // only .class + assertEquals(false, relocator.canRelocatePath("te")) // shorter than path pattern + assertEquals(false, relocator.canRelocatePath("test")) // shorter than path pattern with / + assertEquals(true, relocator.canRelocatePath("org/f")) // equal to path pattern + assertEquals(true, relocator.canRelocatePath("/org/f")) // equal to path pattern with / } void testCanRelocateClass() { SimpleRelocator relocator relocator = new SimpleRelocator("org.foo", null, null, null) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.Class"))) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.bar.Class"))) - assertEquals(false, relocator.canRelocateClass(classContext("com.foo.bar.Class"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.Foo.Class"))) + assertEquals(true, relocator.canRelocateClass("org.foo.Class")) + assertEquals(true, relocator.canRelocateClass("org.foo.bar.Class")) + assertEquals(false, relocator.canRelocateClass("com.foo.bar.Class")) + assertEquals(false, relocator.canRelocateClass("org.Foo.Class")) relocator = new SimpleRelocator("org.foo", null, null, Arrays.asList( [ "org.foo.Excluded", "org.foo.public.*", "org.foo.recurse.**", "org.foo.Public*Stuff" ] as String[])) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.Class"))) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.excluded"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.Excluded"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.public"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.public.Class"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.public.sub"))) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.public.sub.Class"))) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.publicRELOC.Class"))) - assertEquals(true, relocator.canRelocateClass(classContext("org.foo.PrivateStuff"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.PublicStuff"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.PublicUtilStuff"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.recurse"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.recurse.Class"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.recurse.sub"))) - assertEquals(false, relocator.canRelocateClass(classContext("org.foo.recurse.sub.Class"))) + assertEquals(true, relocator.canRelocateClass("org.foo.Class")) + assertEquals(true, relocator.canRelocateClass("org.foo.excluded")) + assertEquals(false, relocator.canRelocateClass("org.foo.Excluded")) + assertEquals(false, relocator.canRelocateClass("org.foo.public")) + assertEquals(false, relocator.canRelocateClass("org.foo.public.Class")) + assertEquals(false, relocator.canRelocateClass("org.foo.public.sub")) + assertEquals(true, relocator.canRelocateClass("org.foo.public.sub.Class")) + assertEquals(true, relocator.canRelocateClass("org.foo.publicRELOC.Class")) + assertEquals(true, relocator.canRelocateClass("org.foo.PrivateStuff")) + assertEquals(false, relocator.canRelocateClass("org.foo.PublicStuff")) + assertEquals(false, relocator.canRelocateClass("org.foo.PublicUtilStuff")) + assertEquals(false, relocator.canRelocateClass("org.foo.recurse")) + assertEquals(false, relocator.canRelocateClass("org.foo.recurse.Class")) + assertEquals(false, relocator.canRelocateClass("org.foo.recurse.sub")) + assertEquals(false, relocator.canRelocateClass("org.foo.recurse.sub.Class")) } void testCanRelocateRawString() { SimpleRelocator relocator relocator = new SimpleRelocator("org/foo", null, null, null, true) - assertEquals(true, relocator.canRelocatePath(pathContext("(I)org/foo/bar/Class"))) + assertEquals(true, relocator.canRelocatePath("(I)org/foo/bar/Class")) relocator = new SimpleRelocator("^META-INF/org.foo.xml\$", null, null, null, true) - assertEquals(true, relocator.canRelocatePath(pathContext("META-INF/org.foo.xml"))) + assertEquals(true, relocator.canRelocatePath("META-INF/org.foo.xml")) } //MSHADE-119, make sure that the easy part of this works. From e57d5755545bde21c0669580b8b3b041d48b4ab4 Mon Sep 17 00:00:00 2001 From: Artem Chubaryan Date: Mon, 6 Jan 2020 17:38:36 -0600 Subject: [PATCH 2/6] Optimizations to relocation checks. Measured to give almost 3x performance boost on large project (5:48 -> 2:02 min total duration). - Changed signature of methods to accept String as path instead of RelocatePathContext/RelocateClassContext to prevent unnecessary allocations for builder objects - Added extra string length checks to avoid string operations where possible - Reordered checks in canRelocateClass so that expensive string replacement is the last operation - (Main Optimization) Avoided string concatenation (which led to creating StringBuilder instances and expensive array copy operations) by adding check for first character being '/': if so - perform a startsWith check with an offset. --- .../transformers/Log4j2PluginsCacheFileTransformer.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy index b82e210e1..093cd365f 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy @@ -112,11 +112,11 @@ class Log4j2PluginsCacheFileTransformer implements Transformer { pluginEntryLoop: for (PluginEntry currentPluginEntry : currentMap.values()) { String className = currentPluginEntry.getClassName() - RelocateClassContext relocateClassContext = new RelocateClassContext(className, stats) for (Relocator currentRelocator : relocators) { // If we have a relocator that can relocate our current entry... if (currentRelocator.canRelocateClass(className)) { // Then we perform that relocation and update the plugin entry to reflect the new value. + RelocateClassContext relocateClassContext = new RelocateClassContext(className, stats) String relocatedClassName = currentRelocator.relocateClass(relocateClassContext) currentPluginEntry.setClassName(relocatedClassName) continue pluginEntryLoop From af4524b00aade9d95bfa22c14a4a45fc7626b132 Mon Sep 17 00:00:00 2001 From: Artem Chubaryan Date: Tue, 7 Jan 2020 14:01:57 -0600 Subject: [PATCH 3/6] Removed an incorrect length check Moved context creation outside the loop (cherry picked from commit 8476da7f43193b366ba13c1eaeb6e13b55d2f25e) --- .../transformers/Log4j2PluginsCacheFileTransformer.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy index 093cd365f..b82e210e1 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.groovy @@ -112,11 +112,11 @@ class Log4j2PluginsCacheFileTransformer implements Transformer { pluginEntryLoop: for (PluginEntry currentPluginEntry : currentMap.values()) { String className = currentPluginEntry.getClassName() + RelocateClassContext relocateClassContext = new RelocateClassContext(className, stats) for (Relocator currentRelocator : relocators) { // If we have a relocator that can relocate our current entry... if (currentRelocator.canRelocateClass(className)) { // Then we perform that relocation and update the plugin entry to reflect the new value. - RelocateClassContext relocateClassContext = new RelocateClassContext(className, stats) String relocatedClassName = currentRelocator.relocateClass(relocateClassContext) currentPluginEntry.setClassName(relocatedClassName) continue pluginEntryLoop From 2791ec6ef95de0efe830db1fa64367a638c4e6fc Mon Sep 17 00:00:00 2001 From: Artem Chubaryan Date: Wed, 8 Jan 2020 12:43:11 -0600 Subject: [PATCH 4/6] Added ability to include/exclude paths/classes via real Regex (cherry picked from commit 076df6211d01a1a9904a741566d645e9ab36c118) --- .../shadow/relocation/SimpleRelocator.groovy | 10 ++++++++ .../relocation/SimpleRelocatorTest.groovy | 24 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy index 9e5d8cb08..6e4641dd4 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy @@ -98,6 +98,16 @@ class SimpleRelocator implements Relocator { return this } + SimpleRelocator includeRegex(String pattern) { + this.includes.add("%regex[$pattern]") + return this + } + + SimpleRelocator excludeRegex(String pattern) { + this.excludes.add("%regex[$pattern]") + return this + } + private static Set normalizePatterns(Collection patterns) { Set normalized = null diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy index 52ee9a89f..2111909c1 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy @@ -95,6 +95,30 @@ class SimpleRelocatorTest extends TestCase { assertEquals(true, relocator.canRelocatePath("/org/f")) // equal to path pattern with / } + void testCanRelocatePathWithRegex() { + SimpleRelocator relocator + + relocator = new SimpleRelocator("org.foo", null, null, null) + relocator.includeRegex("org/foo/R(\\\$.*)?\$") + assertEquals(true, relocator.canRelocatePath("org/foo/R.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/R\$string.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/R\$layout.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/Recording/R.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/Recording.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/bar/R\$string.class")) + assertEquals(false, relocator.canRelocatePath("org/R.class")) + assertEquals(false, relocator.canRelocatePath("org/R\$string.class")) + + relocator = new SimpleRelocator("org.foo", null, null, null) + relocator.excludeRegex("org/foo/.*Factory[0-9].*") + assertEquals(true, relocator.canRelocatePath("org/foo/Factory.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/FooFactoryMain.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/BarFactory.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/Factory0.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/FooFactory1Main.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/BarFactory2.class")) + } + void testCanRelocateClass() { SimpleRelocator relocator From 7a94fca626272a63c78e9440aedfe68a232fece2 Mon Sep 17 00:00:00 2001 From: Artem Chubaryan Date: Mon, 3 Feb 2020 09:57:45 -0600 Subject: [PATCH 5/6] Removed includeRegex/excludeRegex and added ignoring real regexs in the `normalizePatterns` methods. --- .../shadow/relocation/SimpleRelocator.groovy | 15 +++++---------- .../shadow/relocation/SimpleRelocatorTest.groovy | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy index 6e4641dd4..acc3cb13d 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.groovy @@ -98,16 +98,6 @@ class SimpleRelocator implements Relocator { return this } - SimpleRelocator includeRegex(String pattern) { - this.includes.add("%regex[$pattern]") - return this - } - - SimpleRelocator excludeRegex(String pattern) { - this.excludes.add("%regex[$pattern]") - return this - } - private static Set normalizePatterns(Collection patterns) { Set normalized = null @@ -115,6 +105,11 @@ class SimpleRelocator implements Relocator { normalized = new LinkedHashSet() for (String pattern : patterns) { + // Regex patterns don't need to be normalized and stay as is + if (pattern.startsWith(SelectorUtils.REGEX_HANDLER_PREFIX)) { + normalized.add(pattern) + continue + } String classPattern = pattern.replace('.', '/') diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy index 2111909c1..fe9436943 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorTest.groovy @@ -98,8 +98,8 @@ class SimpleRelocatorTest extends TestCase { void testCanRelocatePathWithRegex() { SimpleRelocator relocator - relocator = new SimpleRelocator("org.foo", null, null, null) - relocator.includeRegex("org/foo/R(\\\$.*)?\$") + // Include with Regex + relocator = new SimpleRelocator("org.foo", null, Collections.singletonList("%regex[org/foo/R(\\\$.*)?\$]"), null) assertEquals(true, relocator.canRelocatePath("org/foo/R.class")) assertEquals(true, relocator.canRelocatePath("org/foo/R\$string.class")) assertEquals(true, relocator.canRelocatePath("org/foo/R\$layout.class")) @@ -109,14 +109,23 @@ class SimpleRelocatorTest extends TestCase { assertEquals(false, relocator.canRelocatePath("org/R.class")) assertEquals(false, relocator.canRelocatePath("org/R\$string.class")) + // Exclude with Regex relocator = new SimpleRelocator("org.foo", null, null, null) - relocator.excludeRegex("org/foo/.*Factory[0-9].*") + relocator.exclude("%regex[org/foo/.*Factory[0-9].*]") assertEquals(true, relocator.canRelocatePath("org/foo/Factory.class")) assertEquals(true, relocator.canRelocatePath("org/foo/FooFactoryMain.class")) assertEquals(true, relocator.canRelocatePath("org/foo/BarFactory.class")) assertEquals(false, relocator.canRelocatePath("org/foo/Factory0.class")) assertEquals(false, relocator.canRelocatePath("org/foo/FooFactory1Main.class")) assertEquals(false, relocator.canRelocatePath("org/foo/BarFactory2.class")) + + // Include with Regex and normal pattern + relocator = new SimpleRelocator("org.foo", null, + Arrays.asList("%regex[org/foo/.*Factory[0-9].*]", "org.foo.public.*"), null) + assertEquals(true, relocator.canRelocatePath("org/foo/Factory1.class")) + assertEquals(true, relocator.canRelocatePath("org/foo/public/Bar.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/Factory.class")) + assertEquals(false, relocator.canRelocatePath("org/foo/R.class")) } void testCanRelocateClass() { From f45967848892855f0eeecbdf6722d41ff9dee99a Mon Sep 17 00:00:00 2001 From: Artem Chubaryan Date: Mon, 3 Feb 2020 10:17:30 -0600 Subject: [PATCH 6/6] Added documentation for Regex usage --- src/docs/configuration/relocation/README.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/docs/configuration/relocation/README.md b/src/docs/configuration/relocation/README.md index 7b2407dc8..0bce9ae22 100644 --- a/src/docs/configuration/relocation/README.md +++ b/src/docs/configuration/relocation/README.md @@ -32,7 +32,9 @@ Be specific as possible when configuring relocation as to avoid unintended reloc ## Filtering Relocation -Specific classes or files can be `included`/`excluded` from the relocation operation if necessary. +Specific classes or files can be `included`/`excluded` from the relocation operation if necessary. Use +[Ant Path Matcher](https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/util/AntPathMatcher.html) +syntax to specify matching path for your files and directories. ```groovy // Configuring Filtering for Relocation @@ -46,6 +48,18 @@ shadowJar { } ``` +For a more advanced path matching you might want to use [Regular Expressions](https://regexr.com/) instead. Wrap the expresion in `%regex[]` before +passing it to `include`/`exclude`. + +```groovy +// Configuring Filtering for Relocation with Regex +shadowJar { + relocate('org.foo', 'a') { + include '%regex[org/foo/.*Factory[0-9].*]' + } +} +``` + ## Automatically Relocating Dependencies Shadow is shipped with a task that can be used to automatically configure all packages from all dependencies to be relocated.