From 1b58258a8b42a6dd5326b51bdf2dd2634259f3bb Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 10:46:05 +0800 Subject: [PATCH 1/7] Revert "Migrate SimpleRelocator to using lazy properties (#1047)" This reverts commit 59676b7b --- api/shadow.api | 24 ++-- .../shadow/relocation/SimpleRelocator.kt | 111 +++++++++--------- .../gradle/plugins/shadow/tasks/ShadowJar.kt | 4 +- .../SimpleRelocatorParameterTest.groovy | 4 +- .../relocation/SimpleRelocatorTest.groovy | 36 +++--- ...g4j2PluginsCacheFileTransformerSpec.groovy | 6 +- 6 files changed, 90 insertions(+), 95 deletions(-) diff --git a/api/shadow.api b/api/shadow.api index 2992b7941..8b962c1e4 100644 --- a/api/shadow.api +++ b/api/shadow.api @@ -181,23 +181,23 @@ public final class com/github/jengelman/gradle/plugins/shadow/relocation/Relocat } public class com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator : com/github/jengelman/gradle/plugins/shadow/relocation/Relocator { - public fun (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;)V - public fun (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V - public fun (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V - public fun (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Z)V - public synthetic fun (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/lang/String;Ljava/lang/String;)V + public fun (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V + public fun (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V + public fun (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Z)V + public synthetic fun (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun applyToSourceContent (Ljava/lang/String;)Ljava/lang/String; public fun canRelocateClass (Ljava/lang/String;)Z public fun canRelocatePath (Ljava/lang/String;)Z public fun exclude (Ljava/lang/String;)Lcom/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator; - public fun getExcludes ()Lorg/gradle/api/provider/SetProperty; - public fun getIncludes ()Lorg/gradle/api/provider/SetProperty; - public fun getPathPattern ()Lorg/gradle/api/provider/Property; - public fun getPattern ()Lorg/gradle/api/provider/Property; - public fun getRawString ()Lorg/gradle/api/provider/Property; - public fun getShadedPathPattern ()Lorg/gradle/api/provider/Property; - public fun getShadedPattern ()Lorg/gradle/api/provider/Property; + public fun getExcludes ()Ljava/util/Set; + public fun getIncludes ()Ljava/util/Set; + public fun getPathPattern ()Ljava/lang/String; + public fun getPattern ()Ljava/lang/String; + public fun getShadedPathPattern ()Ljava/lang/String; + public fun getShadedPattern ()Ljava/lang/String; public fun include (Ljava/lang/String;)Lcom/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator; + public fun isRawString ()Z public fun relocateClass (Lcom/github/jengelman/gradle/plugins/shadow/relocation/RelocateClassContext;)Ljava/lang/String; public fun relocatePath (Lcom/github/jengelman/gradle/plugins/shadow/relocation/RelocatePathContext;)Ljava/lang/String; } diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt index 12dd9cf4c..5bdd70564 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt @@ -1,11 +1,7 @@ package com.github.jengelman.gradle.plugins.shadow.relocation -import com.github.jengelman.gradle.plugins.shadow.internal.property import java.util.regex.Pattern import org.codehaus.plexus.util.SelectorUtils -import org.gradle.api.model.ObjectFactory -import org.gradle.api.provider.Property -import org.gradle.api.provider.SetProperty import org.gradle.api.tasks.Input import org.gradle.api.tasks.Optional @@ -16,77 +12,82 @@ import org.gradle.api.tasks.Optional * @author Mauro Talevi * @author John Engelman */ -@Suppress("LeakingThis") @CacheableRelocator public open class SimpleRelocator @JvmOverloads constructor( - objectFactory: ObjectFactory, pattern: String?, shadedPattern: String?, includes: List? = null, excludes: List? = null, - rawString: Boolean = false, + private val _isRawString: Boolean = false, ) : Relocator { + private val _pattern: String? + private val _pathPattern: String + private val _shadedPattern: String? + private val _shadedPathPattern: String + private val _includes = mutableSetOf() + private val _excludes = mutableSetOf() + + init { + if (_isRawString) { + _pathPattern = pattern.orEmpty() + _shadedPathPattern = shadedPattern.orEmpty() + _pattern = null // not used for raw string relocator + _shadedPattern = null // not used for raw string relocator + } else { + if (pattern == null) { + _pattern = "" + _pathPattern = "" + } else { + _pattern = pattern.replace('/', '.') + _pathPattern = pattern.replace('.', '/') + } + if (shadedPattern == null) { + _shadedPattern = "hidden.${_pattern}" + _shadedPathPattern = "hidden/$_pathPattern" + } else { + _shadedPattern = shadedPattern.replace('/', '.') + _shadedPathPattern = shadedPattern.replace('.', '/') + } + } + _includes += normalizePatterns(includes) + _excludes += normalizePatterns(excludes) + } @get:Input @get:Optional - public open val pattern: Property = objectFactory.property() + public open val pattern: String? get() = _pattern @get:Input - public open val pathPattern: Property = objectFactory.property() + public open val pathPattern: String get() = _pathPattern @get:Input @get:Optional - public open val shadedPattern: Property = objectFactory.property() + public open val shadedPattern: String? get() = _shadedPattern @get:Input - public open val shadedPathPattern: Property = objectFactory.property() + public open val shadedPathPattern: String get() = _shadedPathPattern @get:Input - public open val rawString: Property = objectFactory.property(rawString) + public open val isRawString: Boolean get() = _isRawString @get:Input - public open val includes: SetProperty = objectFactory.setProperty(String::class.java) + public open val includes: Set get() = _includes @get:Input - public open val excludes: SetProperty = objectFactory.setProperty(String::class.java) - - init { - if (rawString) { - pathPattern.set(pattern.orEmpty()) - shadedPathPattern.set(shadedPattern.orEmpty()) - // Don't need to assign pattern and shadedPattern for raw string relocator - } else { - if (pattern == null) { - this.pattern.set("") - this.pathPattern.set("") - } else { - this.pattern.set(pattern.replace('/', '.')) - this.pathPattern.set(pattern.replace('.', '/')) - } - if (shadedPattern == null) { - this.shadedPattern.set(this.pattern.map { "hidden.$it" }) - this.shadedPathPattern.set(this.pathPattern.map { "hidden/$it" }) - } else { - this.shadedPattern.set(shadedPattern.replace('/', '.')) - this.shadedPathPattern.set(shadedPattern.replace('.', '/')) - } - } - this.includes.addAll(normalizePatterns(includes)) - this.excludes.addAll(normalizePatterns(excludes)) - } + public open val excludes: Set get() = _excludes public open fun include(pattern: String): SimpleRelocator = apply { - includes.addAll(normalizePatterns(listOf(pattern))) + _includes += normalizePatterns(listOf(pattern)) } public open fun exclude(pattern: String): SimpleRelocator = apply { - excludes.addAll(normalizePatterns(listOf(pattern))) + _excludes += normalizePatterns(listOf(pattern)) } override fun canRelocatePath(path: String): Boolean { - if (rawString.get()) return Pattern.compile(pathPattern.get()).matcher(path).find() + if (_isRawString) return Pattern.compile(_pathPattern).matcher(path).find() // If string is too short - no need to perform expensive string operations - if (path.length < pathPattern.get().length) return false + if (path.length < _pathPattern.length) return false val adjustedPath = if (path.endsWith(".class")) { // Safeguard against strings containing only ".class" if (path.length == 6) return false @@ -96,34 +97,34 @@ public open class SimpleRelocator @JvmOverloads constructor( } // Allow for annoying option of an extra / on the front of a path. See MSHADE-119 comes from getClass().getResource("/a/b/c.properties"). val startIndex = if (adjustedPath.startsWith("/")) 1 else 0 - val pathStartsWithPattern = adjustedPath.startsWith(pathPattern.get(), startIndex) + val pathStartsWithPattern = adjustedPath.startsWith(_pathPattern, startIndex) return pathStartsWithPattern && isIncluded(adjustedPath) && !isExcluded(adjustedPath) } override fun canRelocateClass(className: String): Boolean { - return !rawString.get() && !className.contains('/') && canRelocatePath(className.replace('.', '/')) + return !_isRawString && !className.contains('/') && canRelocatePath(className.replace('.', '/')) } override fun relocatePath(context: RelocatePathContext): String { val path = context.path - context.stats.relocate(pathPattern.get(), shadedPathPattern.get()) - return if (rawString.get()) { - path.replace(pathPattern.get().toRegex(), shadedPathPattern.get()) + context.stats.relocate(_pathPattern, _shadedPathPattern) + return if (_isRawString) { + path.replace(_pathPattern.toRegex(), _shadedPathPattern) } else { - path.replaceFirst(pathPattern.get(), shadedPathPattern.get()) + path.replaceFirst(_pathPattern, _shadedPathPattern) } } override fun relocateClass(context: RelocateClassContext): String { - context.stats.relocate(pathPattern.get(), shadedPathPattern.get()) - return context.className.replaceFirst(pattern.orNull.orEmpty(), shadedPattern.orNull.orEmpty()) + context.stats.relocate(_pathPattern, _shadedPathPattern) + return context.className.replaceFirst(_pattern.orEmpty(), _shadedPattern.orEmpty()) } override fun applyToSourceContent(sourceContent: String): String { - return if (rawString.get()) { + return if (_isRawString) { sourceContent } else { - sourceContent.replace("\\b${pattern.orNull}".toRegex(), shadedPattern.orNull.orEmpty()) + sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern.orEmpty()) } } @@ -146,10 +147,10 @@ public open class SimpleRelocator @JvmOverloads constructor( } private fun isIncluded(path: String): Boolean { - return includes.get().isEmpty() || includes.get().any { SelectorUtils.matchPath(it, path, "/", true) } + return _includes.isEmpty() || _includes.any { SelectorUtils.matchPath(it, path, "/", true) } } private fun isExcluded(path: String): Boolean { - return excludes.get().any { SelectorUtils.matchPath(it, path, "/", true) } + return _excludes.any { SelectorUtils.matchPath(it, path, "/", true) } } } diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt index ee9c18ec7..f0a1ab085 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt @@ -213,7 +213,7 @@ public abstract class ShadowJar : destination: String, action: Action?, ): ShadowJar = apply { - val relocator = SimpleRelocator(objectFactory, pattern, destination) + val relocator = SimpleRelocator(pattern, destination) addRelocator(relocator, action) } @@ -289,7 +289,7 @@ public abstract class ShadowJar : jarFile.entries().toList() .filter { it.name.endsWith(".class") && it.name != "module-info.class" } .map { it.name.substringBeforeLast('/').replace('/', '.') } - .map { SimpleRelocator(objectFactory, it, "$prefix.$it") } + .map { SimpleRelocator(it, "$prefix.$it") } } } } diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorParameterTest.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorParameterTest.groovy index 3cab488f9..9e47404ca 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorParameterTest.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocatorParameterTest.groovy @@ -19,7 +19,6 @@ package com.github.jengelman.gradle.plugins.shadow.relocation -import org.gradle.testfixtures.ProjectBuilder import org.junit.jupiter.api.Test import static org.junit.jupiter.api.Assertions.fail @@ -32,7 +31,6 @@ import static org.junit.jupiter.api.Assertions.fail * @author John Engelman */ class SimpleRelocatorParameterTest { - private static final def objectFactory = ProjectBuilder.builder().build().objects @Test void testThatNullPatternInConstructorShouldNotThrowNullPointerException() { @@ -46,7 +44,7 @@ class SimpleRelocatorParameterTest { private static void constructThenFailOnNullPointerException(String pattern, String shadedPattern) { try { - new SimpleRelocator(objectFactory, pattern, shadedPattern) + new SimpleRelocator(pattern, shadedPattern) } catch (NullPointerException ignored) { fail("Constructor should not throw null pointer exceptions") 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 5d0eb0b98..f1488c523 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 @@ -20,7 +20,6 @@ package com.github.jengelman.gradle.plugins.shadow.relocation import com.github.jengelman.gradle.plugins.shadow.ShadowStats -import org.gradle.testfixtures.ProjectBuilder import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -39,7 +38,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals */ class SimpleRelocatorTest { - private static final def objectFactory = ProjectBuilder.builder().build().objects private static ShadowStats stats @BeforeEach @@ -51,7 +49,7 @@ class SimpleRelocatorTest { void testCanRelocatePath() { SimpleRelocator relocator - relocator = new SimpleRelocator(objectFactory, "org.foo", null) + relocator = new SimpleRelocator("org.foo", null) assertEquals(true, relocator.canRelocatePath("org/foo/Class")) assertEquals(true, relocator.canRelocatePath("org/foo/Class.class")) assertEquals(true, relocator.canRelocatePath("org/foo/bar/Class")) @@ -65,7 +63,7 @@ class SimpleRelocatorTest { assertEquals(false, relocator.canRelocatePath("/org/Foo/Class")) assertEquals(false, relocator.canRelocatePath("/org/Foo/Class.class")) - relocator = new SimpleRelocator(objectFactory, "org.foo", null, null, + relocator = new SimpleRelocator("org.foo", null, null, List.of("org.foo.Excluded", "org.foo.public.*", "org.foo.recurse.**", "org.foo.Public*Stuff")) assertEquals(true, relocator.canRelocatePath("org/foo/Class")) assertEquals(true, relocator.canRelocatePath("org/foo/Class.class")) @@ -92,7 +90,7 @@ class SimpleRelocatorTest { assertEquals(false, relocator.canRelocatePath("org/foo/recurse/sub/Class.class")) // Verify edge cases - relocator = new SimpleRelocator(objectFactory, "org.f", null) + relocator = new SimpleRelocator("org.f", null) assertEquals(false, relocator.canRelocatePath("")) // Empty path assertEquals(false, relocator.canRelocatePath(".class")) // only .class assertEquals(false, relocator.canRelocatePath("te")) // shorter than path pattern @@ -106,7 +104,7 @@ class SimpleRelocatorTest { SimpleRelocator relocator // Include with Regex - relocator = new SimpleRelocator(objectFactory, "org.foo", null, Collections.singletonList("%regex[org/foo/R(\\\$.*)?\$]"), null) + 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")) @@ -117,7 +115,7 @@ class SimpleRelocatorTest { assertEquals(false, relocator.canRelocatePath("org/R\$string.class")) // Exclude with Regex - relocator = new SimpleRelocator(objectFactory, "org.foo", null) + relocator = new SimpleRelocator("org.foo", null) relocator.exclude("%regex[org/foo/.*Factory[0-9].*]") assertEquals(true, relocator.canRelocatePath("org/foo/Factory.class")) assertEquals(true, relocator.canRelocatePath("org/foo/FooFactoryMain.class")) @@ -127,7 +125,7 @@ class SimpleRelocatorTest { assertEquals(false, relocator.canRelocatePath("org/foo/BarFactory2.class")) // Include with Regex and normal pattern - relocator = new SimpleRelocator(objectFactory, "org.foo", null, + relocator = new SimpleRelocator("org.foo", null, List.of("%regex[org/foo/.*Factory[0-9].*]", "org.foo.public.*")) assertEquals(true, relocator.canRelocatePath("org/foo/Factory1.class")) assertEquals(true, relocator.canRelocatePath("org/foo/public/Bar.class")) @@ -139,13 +137,13 @@ class SimpleRelocatorTest { void testCanRelocateClass() { SimpleRelocator relocator - relocator = new SimpleRelocator(objectFactory, "org.foo", null) + relocator = new SimpleRelocator("org.foo", null) 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(objectFactory, "org.foo", null, null, + relocator = new SimpleRelocator("org.foo", null, null, List.of("org.foo.Excluded", "org.foo.public.*", "org.foo.recurse.**", "org.foo.Public*Stuff")) assertEquals(true, relocator.canRelocateClass("org.foo.Class")) assertEquals(true, relocator.canRelocateClass("org.foo.excluded")) @@ -168,17 +166,17 @@ class SimpleRelocatorTest { void testCanRelocateRawString() { SimpleRelocator relocator - relocator = new SimpleRelocator(objectFactory, "org/foo", null, null, null, true) + relocator = new SimpleRelocator("org/foo", null, null, null, true) assertEquals(true, relocator.canRelocatePath("(I)org/foo/bar/Class")) - relocator = new SimpleRelocator(objectFactory, "^META-INF/org.foo.xml\$", null, null, null, true) + relocator = new SimpleRelocator("^META-INF/org.foo.xml\$", null, null, null, true) assertEquals(true, relocator.canRelocatePath("META-INF/org.foo.xml")) } //MSHADE-119, make sure that the easy part of this works. @Test void testCanRelocateAbsClassPath() { - SimpleRelocator relocator = new SimpleRelocator(objectFactory, "org.apache.velocity", "org.apache.momentum") + SimpleRelocator relocator = new SimpleRelocator("org.apache.velocity", "org.apache.momentum") assertEquals("/org/apache/momentum/mass.properties", relocator.relocatePath(pathContext("/org/apache/velocity/mass.properties"))) } @@ -187,10 +185,10 @@ class SimpleRelocatorTest { void testRelocatePath() { SimpleRelocator relocator - relocator = new SimpleRelocator(objectFactory, "org.foo", null) + relocator = new SimpleRelocator("org.foo", null) assertEquals("hidden/org/foo/bar/Class.class", relocator.relocatePath(pathContext("org/foo/bar/Class.class"))) - relocator = new SimpleRelocator(objectFactory, "org.foo", "private.stuff") + relocator = new SimpleRelocator("org.foo", "private.stuff") assertEquals("private/stuff/bar/Class.class", relocator.relocatePath(pathContext("org/foo/bar/Class.class"))) } @@ -198,10 +196,10 @@ class SimpleRelocatorTest { void testRelocateClass() { SimpleRelocator relocator - relocator = new SimpleRelocator(objectFactory, "org.foo", null) + relocator = new SimpleRelocator("org.foo", null) assertEquals("hidden.org.foo.bar.Class", relocator.relocateClass(classContext("org.foo.bar.Class"))) - relocator = new SimpleRelocator(objectFactory, "org.foo", "private.stuff") + relocator = new SimpleRelocator("org.foo", "private.stuff") assertEquals("private.stuff.bar.Class", relocator.relocateClass(classContext("org.foo.bar.Class"))) } @@ -209,10 +207,10 @@ class SimpleRelocatorTest { void testRelocateRawString() { SimpleRelocator relocator - relocator = new SimpleRelocator(objectFactory, "Lorg/foo", "Lhidden/org/foo", null, null, true) + relocator = new SimpleRelocator("Lorg/foo", "Lhidden/org/foo", null, null, true) assertEquals("(I)Lhidden/org/foo/bar/Class", relocator.relocatePath(pathContext("(I)Lorg/foo/bar/Class"))) - relocator = new SimpleRelocator(objectFactory, "^META-INF/org.foo.xml\$", "META-INF/hidden.org.foo.xml", null, null, true) + relocator = new SimpleRelocator("^META-INF/org.foo.xml\$", "META-INF/hidden.org.foo.xml", null, null, true) assertEquals("META-INF/hidden.org.foo.xml", relocator.relocatePath(pathContext("META-INF/org.foo.xml"))) } diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerSpec.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerSpec.groovy index b718f52b5..e48aaf66a 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerSpec.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerSpec.groovy @@ -5,7 +5,6 @@ import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator import com.github.jengelman.gradle.plugins.shadow.relocation.SimpleRelocator import org.apache.logging.log4j.core.config.plugins.processor.PluginCache import org.apache.tools.zip.ZipOutputStream -import org.gradle.testfixtures.ProjectBuilder import spock.lang.Specification import static java.util.Collections.singletonList @@ -18,7 +17,6 @@ import static org.apache.logging.log4j.core.config.plugins.processor.PluginProce * @see LinkedIn */ class Log4j2PluginsCacheFileTransformerSpec extends Specification { - private static final def objectFactory = ProjectBuilder.builder().build().objects Log4j2PluginsCacheFileTransformer transformer @@ -37,7 +35,7 @@ class Log4j2PluginsCacheFileTransformerSpec extends Specification { void "should transform"() { given: List relocators = new ArrayList<>() - relocators.add(new SimpleRelocator(objectFactory, null, null)) + relocators.add(new SimpleRelocator(null, null)) when: transformer.transform(new TransformerContext(PLUGIN_CACHE_FILE, getResourceStream(PLUGIN_CACHE_FILE), relocators)) @@ -51,7 +49,7 @@ class Log4j2PluginsCacheFileTransformerSpec extends Specification { String pattern = "org.apache.logging" String destination = "new.location.org.apache.logging" - List relocators = singletonList((Relocator) new SimpleRelocator(objectFactory, pattern, destination)) + List relocators = singletonList((Relocator) new SimpleRelocator(pattern, destination)) when: transformer.transform(new TransformerContext(PLUGIN_CACHE_FILE, getResourceStream(PLUGIN_CACHE_FILE), relocators, new ShadowStats())) From ac6982df72364ac7a3b7bf7cec685f6677eef96b Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 10:49:45 +0800 Subject: [PATCH 2/7] Still rename isRawString to rawString --- api/shadow.api | 2 +- .../plugins/shadow/relocation/SimpleRelocator.kt | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/shadow.api b/api/shadow.api index 8b962c1e4..7df232de7 100644 --- a/api/shadow.api +++ b/api/shadow.api @@ -194,10 +194,10 @@ public class com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocat public fun getIncludes ()Ljava/util/Set; public fun getPathPattern ()Ljava/lang/String; public fun getPattern ()Ljava/lang/String; + public fun getRawString ()Z public fun getShadedPathPattern ()Ljava/lang/String; public fun getShadedPattern ()Ljava/lang/String; public fun include (Ljava/lang/String;)Lcom/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator; - public fun isRawString ()Z public fun relocateClass (Lcom/github/jengelman/gradle/plugins/shadow/relocation/RelocateClassContext;)Ljava/lang/String; public fun relocatePath (Lcom/github/jengelman/gradle/plugins/shadow/relocation/RelocatePathContext;)Ljava/lang/String; } diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt index 5bdd70564..e2e436572 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt @@ -18,7 +18,7 @@ public open class SimpleRelocator @JvmOverloads constructor( shadedPattern: String?, includes: List? = null, excludes: List? = null, - private val _isRawString: Boolean = false, + private val _rawString: Boolean = false, ) : Relocator { private val _pattern: String? private val _pathPattern: String @@ -28,7 +28,7 @@ public open class SimpleRelocator @JvmOverloads constructor( private val _excludes = mutableSetOf() init { - if (_isRawString) { + if (_rawString) { _pathPattern = pattern.orEmpty() _shadedPathPattern = shadedPattern.orEmpty() _pattern = null // not used for raw string relocator @@ -68,7 +68,7 @@ public open class SimpleRelocator @JvmOverloads constructor( public open val shadedPathPattern: String get() = _shadedPathPattern @get:Input - public open val isRawString: Boolean get() = _isRawString + public open val rawString: Boolean get() = _rawString @get:Input public open val includes: Set get() = _includes @@ -85,7 +85,7 @@ public open class SimpleRelocator @JvmOverloads constructor( } override fun canRelocatePath(path: String): Boolean { - if (_isRawString) return Pattern.compile(_pathPattern).matcher(path).find() + 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 val adjustedPath = if (path.endsWith(".class")) { @@ -102,13 +102,13 @@ public open class SimpleRelocator @JvmOverloads constructor( } override fun canRelocateClass(className: String): Boolean { - return !_isRawString && !className.contains('/') && canRelocatePath(className.replace('.', '/')) + return !_rawString && !className.contains('/') && canRelocatePath(className.replace('.', '/')) } override fun relocatePath(context: RelocatePathContext): String { val path = context.path context.stats.relocate(_pathPattern, _shadedPathPattern) - return if (_isRawString) { + return if (_rawString) { path.replace(_pathPattern.toRegex(), _shadedPathPattern) } else { path.replaceFirst(_pathPattern, _shadedPathPattern) @@ -121,7 +121,7 @@ public open class SimpleRelocator @JvmOverloads constructor( } override fun applyToSourceContent(sourceContent: String): String { - return if (_isRawString) { + return if (_rawString) { sourceContent } else { sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern.orEmpty()) From a5a8f017db8420c802f4c51b456957e7af77834c Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 10:51:39 +0800 Subject: [PATCH 3/7] Optimize normalizePatterns --- .../shadow/relocation/SimpleRelocator.kt | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt index e2e436572..82d6734bf 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt @@ -128,24 +128,6 @@ public open class SimpleRelocator @JvmOverloads constructor( } } - private fun normalizePatterns(patterns: Collection?) = buildSet { - for (pattern in patterns.orEmpty()) { - // Regex patterns don't need to be normalized and stay as is - if (pattern.startsWith(SelectorUtils.REGEX_HANDLER_PREFIX)) { - add(pattern) - continue - } - - val classPattern = pattern.replace('.', '/') - add(classPattern) - - if (classPattern.endsWith("/*")) { - val packagePattern = classPattern.substring(0, classPattern.lastIndexOf('/')) - add(packagePattern) - } - } - } - private fun isIncluded(path: String): Boolean { return _includes.isEmpty() || _includes.any { SelectorUtils.matchPath(it, path, "/", true) } } @@ -153,4 +135,25 @@ public open class SimpleRelocator @JvmOverloads constructor( private fun isExcluded(path: String): Boolean { return _excludes.any { SelectorUtils.matchPath(it, path, "/", true) } } + + private companion object { + fun normalizePatterns(patterns: Collection?) = buildSet { + patterns ?: return@buildSet + for (pattern in patterns) { + // Regex patterns don't need to be normalized and stay as is + if (pattern.startsWith(SelectorUtils.REGEX_HANDLER_PREFIX)) { + add(pattern) + continue + } + + val classPattern = pattern.replace('.', '/') + add(classPattern) + + if (classPattern.endsWith("/*")) { + val packagePattern = classPattern.substring(0, classPattern.lastIndexOf('/')) + add(packagePattern) + } + } + } + } } From 11a0397475768d2a21e20abce4e8e3cbd416f632 Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 11:02:45 +0800 Subject: [PATCH 4/7] Mark pattern and shadedPattern non-null --- .../plugins/shadow/relocation/SimpleRelocator.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt index 82d6734bf..15311b17d 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt @@ -20,9 +20,9 @@ public open class SimpleRelocator @JvmOverloads constructor( excludes: List? = null, private val _rawString: Boolean = false, ) : Relocator { - private val _pattern: String? + private val _pattern: String private val _pathPattern: String - private val _shadedPattern: String? + private val _shadedPattern: String private val _shadedPathPattern: String private val _includes = mutableSetOf() private val _excludes = mutableSetOf() @@ -31,8 +31,8 @@ public open class SimpleRelocator @JvmOverloads constructor( if (_rawString) { _pathPattern = pattern.orEmpty() _shadedPathPattern = shadedPattern.orEmpty() - _pattern = null // not used for raw string relocator - _shadedPattern = null // not used for raw string relocator + _pattern = "" // not used for raw string relocator + _shadedPattern = "" // not used for raw string relocator } else { if (pattern == null) { _pattern = "" @@ -55,14 +55,14 @@ public open class SimpleRelocator @JvmOverloads constructor( @get:Input @get:Optional - public open val pattern: String? get() = _pattern + public open val pattern: String get() = _pattern @get:Input public open val pathPattern: String get() = _pathPattern @get:Input @get:Optional - public open val shadedPattern: String? get() = _shadedPattern + public open val shadedPattern: String get() = _shadedPattern @get:Input public open val shadedPathPattern: String get() = _shadedPathPattern @@ -117,14 +117,14 @@ public open class SimpleRelocator @JvmOverloads constructor( override fun relocateClass(context: RelocateClassContext): String { context.stats.relocate(_pathPattern, _shadedPathPattern) - return context.className.replaceFirst(_pattern.orEmpty(), _shadedPattern.orEmpty()) + return context.className.replaceFirst(_pattern, _shadedPattern) } override fun applyToSourceContent(sourceContent: String): String { return if (_rawString) { sourceContent } else { - sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern.orEmpty()) + sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern) } } From b05bc7d29b6f01fa72285f9c94a904ea0249aafe Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 11:04:37 +0800 Subject: [PATCH 5/7] Inline getters --- .../plugins/shadow/relocation/SimpleRelocator.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt index 15311b17d..eac57f46d 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt @@ -55,26 +55,26 @@ public open class SimpleRelocator @JvmOverloads constructor( @get:Input @get:Optional - public open val pattern: String get() = _pattern + public open val pattern: String = _pattern @get:Input - public open val pathPattern: String get() = _pathPattern + public open val pathPattern: String = _pathPattern @get:Input @get:Optional - public open val shadedPattern: String get() = _shadedPattern + public open val shadedPattern: String = _shadedPattern @get:Input - public open val shadedPathPattern: String get() = _shadedPathPattern + public open val shadedPathPattern: String = _shadedPathPattern @get:Input - public open val rawString: Boolean get() = _rawString + public open val rawString: Boolean = _rawString @get:Input - public open val includes: Set get() = _includes + public open val includes: Set = _includes @get:Input - public open val excludes: Set get() = _excludes + public open val excludes: Set = _excludes public open fun include(pattern: String): SimpleRelocator = apply { _includes += normalizePatterns(listOf(pattern)) From 7b4c3605300c3ceaf7479a48654176cf7891e0e0 Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 11:20:41 +0800 Subject: [PATCH 6/7] Note this change --- src/docs/changes/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/docs/changes/README.md b/src/docs/changes/README.md index 297931619..dd768fa42 100644 --- a/src/docs/changes/README.md +++ b/src/docs/changes/README.md @@ -3,6 +3,11 @@ ## [Unreleased] +**Fixed** + +- Revert "Migrate SimpleRelocator to using lazy properties" ([#1052](https://github.com/GradleUp/shadow/pull/1052)) + This fixes the relocation not working in `v9.0.0-beta1`. + ## [v9.0.0-beta1] (2024-11-27) From 7827a4e4fa632db226899387922e1b4237364be8 Mon Sep 17 00:00:00 2001 From: Goooler Date: Thu, 28 Nov 2024 11:51:32 +0800 Subject: [PATCH 7/7] Revert "Inline getters" This reverts commit b05bc7d29b6f01fa72285f9c94a904ea0249aafe. --- .../plugins/shadow/relocation/SimpleRelocator.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt index eac57f46d..15311b17d 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/SimpleRelocator.kt @@ -55,26 +55,26 @@ public open class SimpleRelocator @JvmOverloads constructor( @get:Input @get:Optional - public open val pattern: String = _pattern + public open val pattern: String get() = _pattern @get:Input - public open val pathPattern: String = _pathPattern + public open val pathPattern: String get() = _pathPattern @get:Input @get:Optional - public open val shadedPattern: String = _shadedPattern + public open val shadedPattern: String get() = _shadedPattern @get:Input - public open val shadedPathPattern: String = _shadedPathPattern + public open val shadedPathPattern: String get() = _shadedPathPattern @get:Input - public open val rawString: Boolean = _rawString + public open val rawString: Boolean get() = _rawString @get:Input - public open val includes: Set = _includes + public open val includes: Set get() = _includes @get:Input - public open val excludes: Set = _excludes + public open val excludes: Set get() = _excludes public open fun include(pattern: String): SimpleRelocator = apply { _includes += normalizePatterns(listOf(pattern))