Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert changes in SimpleRelocator #1052

Merged
merged 7 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions api/shadow.api
Original file line number Diff line number Diff line change
Expand Up @@ -181,22 +181,22 @@ 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 <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V
public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V
public fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Z)V
public synthetic fun <init> (Lorg/gradle/api/model/ObjectFactory;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/util/List;Z)V
public synthetic fun <init> (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 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 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;
Expand Down
5 changes: 5 additions & 0 deletions src/docs/changes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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<String>? = null,
excludes: List<String>? = null,
rawString: Boolean = false,
private val _rawString: Boolean = false,
) : Relocator {
private val _pattern: String
private val _pathPattern: String
private val _shadedPattern: String
private val _shadedPathPattern: String
private val _includes = mutableSetOf<String>()
private val _excludes = mutableSetOf<String>()

init {
if (_rawString) {
_pathPattern = pattern.orEmpty()
_shadedPathPattern = shadedPattern.orEmpty()
_pattern = "" // not used for raw string relocator
_shadedPattern = "" // 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<String> = objectFactory.property()
public open val pattern: String = _pattern

@get:Input
public open val pathPattern: Property<String> = objectFactory.property()
public open val pathPattern: String = _pathPattern

@get:Input
@get:Optional
public open val shadedPattern: Property<String> = objectFactory.property()
public open val shadedPattern: String = _shadedPattern

@get:Input
public open val shadedPathPattern: Property<String> = objectFactory.property()
public open val shadedPathPattern: String = _shadedPathPattern

@get:Input
public open val rawString: Property<Boolean> = objectFactory.property(rawString)
public open val rawString: Boolean = _rawString

@get:Input
public open val includes: SetProperty<String> = objectFactory.setProperty(String::class.java)
public open val includes: Set<String> = _includes

@get:Input
public open val excludes: SetProperty<String> = 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<String> = _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 (_rawString) 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
Expand All @@ -96,60 +97,63 @@ 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 !_rawString && !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 (_rawString) {
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, _shadedPattern)
}

override fun applyToSourceContent(sourceContent: String): String {
return if (rawString.get()) {
return if (_rawString) {
sourceContent
} else {
sourceContent.replace("\\b${pattern.orNull}".toRegex(), shadedPattern.orNull.orEmpty())
}
}

private fun normalizePatterns(patterns: Collection<String>?) = 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)
}
sourceContent.replace("\\b$_pattern".toRegex(), _shadedPattern)
}
}

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) }
}

private companion object {
fun normalizePatterns(patterns: Collection<String>?) = 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)
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public abstract class ShadowJar :
destination: String,
action: Action<SimpleRelocator>?,
): ShadowJar = apply {
val relocator = SimpleRelocator(objectFactory, pattern, destination)
val relocator = SimpleRelocator(pattern, destination)
addRelocator(relocator, action)
}

Expand Down Expand Up @@ -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") }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand All @@ -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")
Expand Down
Loading