From 279572bf8c78a6684ee507d0fd18dc0c8f40b366 Mon Sep 17 00:00:00 2001 From: John Engelman Date: Fri, 27 Jun 2014 09:02:38 -0500 Subject: [PATCH] close #55 apply class remappings to source project files --- ChangeLog.md | 1 + .../shadow/tasks/ShadowCopyAction.groovy | 79 +++++++++++-------- .../plugins/shadow/RelocationSpec.groovy | 57 +++++++++++++ 3 files changed, 105 insertions(+), 32 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 5fb466bf5..430465e9f 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -5,6 +5,7 @@ v1.0.0 + All changes from 0.9.0-M1 to 0.9.0-M5 + Properly configure the ShadowJar task inputs to observe the include/excludes from the `dependencies` block. This allows UP-TO-DATE checking to work properly when changing the `dependencies` rules ([Issue #54](https://github.com/johnrengelman/shadow/issues/54)) ++ Apply relocation remappings to classes and imports in source project ([Issue #55](https://github.com/johnrengelman/shadow/issues/55)) v0.9.0-M5 ========= diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy index 0ed49a1b3..f909453ca 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.groovy @@ -5,6 +5,7 @@ import com.github.jengelman.gradle.plugins.shadow.impl.RelocatorRemapper import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator import com.github.jengelman.gradle.plugins.shadow.transformers.Transformer import groovy.util.logging.Slf4j +import org.apache.commons.io.FilenameUtils import org.apache.commons.io.IOUtils import org.apache.tools.zip.UnixStat import org.apache.tools.zip.Zip64RequiredException @@ -148,13 +149,17 @@ public class ShadowCopyAction implements CopyAction { private void visitFile(FileCopyDetails fileDetails) { if (!isArchive(fileDetails)) { try { - String path = fileDetails.relativePath.pathString - ZipEntry archiveEntry = new ZipEntry(path) - archiveEntry.setTime(fileDetails.lastModified) - archiveEntry.unixMode = (UnixStat.FILE_FLAG | fileDetails.mode) - zipOutStr.putNextEntry(archiveEntry) - fileDetails.copyTo(zipOutStr) - zipOutStr.closeEntry() + if (!remapper.hasRelocators()) { + String path = fileDetails.relativePath.pathString + ZipEntry archiveEntry = new ZipEntry(path) + archiveEntry.setTime(fileDetails.lastModified) + archiveEntry.unixMode = (UnixStat.FILE_FLAG | fileDetails.mode) + zipOutStr.putNextEntry(archiveEntry) + fileDetails.copyTo(zipOutStr) + zipOutStr.closeEntry() + } else { + remapClass(fileDetails) + } recordVisit(fileDetails.relativePath) } catch (Exception e) { throw new GradleException(String.format("Could not add %s to ZIP '%s'.", fileDetails, zipFile), e) @@ -216,37 +221,47 @@ public class ShadowCopyAction implements CopyAction { private void remapClass(RelativeArchivePath file, ZipFile archive) { if (file.classFile) { - InputStream is = archive.getInputStream(file.entry) - ClassReader cr = new ClassReader(is) + remapClass(archive.getInputStream(file.entry), file.pathString) + } + } - // We don't pass the ClassReader here. This forces the ClassWriter to rebuild the constant pool. - // Copying the original constant pool should be avoided because it would keep references - // to the original class names. This is not a problem at runtime (because these entries in the - // constant pool are never used), but confuses some tools such as Felix' maven-bundle-plugin - // that use the constant pool to determine the dependencies of a class. - ClassWriter cw = new ClassWriter(0) + private void remapClass(FileCopyDetails fileCopyDetails) { + if (FilenameUtils.getExtension(fileCopyDetails.name) == 'class') { + remapClass(fileCopyDetails.file.newInputStream(), fileCopyDetails.path) + } + } - ClassVisitor cv = new RemappingClassAdapter(cw, remapper) + private void remapClass(InputStream classInputStream, String path) { + InputStream is = classInputStream + ClassReader cr = new ClassReader(is) - try { - cr.accept(cv, ClassReader.EXPAND_FRAMES) - } catch (Throwable ise) { - throw new GradleException("Error in ASM processing class " + file.pathString, ise) - } + // We don't pass the ClassReader here. This forces the ClassWriter to rebuild the constant pool. + // Copying the original constant pool should be avoided because it would keep references + // to the original class names. This is not a problem at runtime (because these entries in the + // constant pool are never used), but confuses some tools such as Felix' maven-bundle-plugin + // that use the constant pool to determine the dependencies of a class. + ClassWriter cw = new ClassWriter(0) - byte[] renamedClass = cw.toByteArray() + ClassVisitor cv = new RemappingClassAdapter(cw, remapper) - // Need to take the .class off for remapping evaluation - String mappedName = remapper.map(file.pathString.substring(0, file.pathString.indexOf('.'))) + try { + cr.accept(cv, ClassReader.EXPAND_FRAMES) + } catch (Throwable ise) { + throw new GradleException("Error in ASM processing class " + path, ise) + } - try { - // Now we put it back on so the class file is written out with the right extension. - zipOutStr.putNextEntry(new ZipEntry(mappedName + ".class")) - IOUtils.copyLarge(new ByteArrayInputStream(renamedClass), zipOutStr) - zipOutStr.closeEntry() - } catch (ZipException e) { - log.warn("We have a duplicate " + mappedName + " in " + archive) - } + byte[] renamedClass = cw.toByteArray() + + // Need to take the .class off for remapping evaluation + String mappedName = remapper.map(path.substring(0, path.indexOf('.'))) + + try { + // Now we put it back on so the class file is written out with the right extension. + zipOutStr.putNextEntry(new ZipEntry(mappedName + ".class")) + IOUtils.copyLarge(new ByteArrayInputStream(renamedClass), zipOutStr) + zipOutStr.closeEntry() + } catch (ZipException e) { + log.warn("We have a duplicate " + mappedName + " in source project") } } diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/RelocationSpec.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/RelocationSpec.groovy index 0f0776c47..a5aaa7a77 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/RelocationSpec.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/RelocationSpec.groovy @@ -2,6 +2,7 @@ package com.github.jengelman.gradle.plugins.shadow import com.github.jengelman.gradle.plugins.shadow.util.PluginSpecification import org.gradle.testkit.functional.ExecutionResult +import spock.lang.Issue class RelocationSpec extends PluginSpecification { @@ -135,4 +136,60 @@ class RelocationSpec extends PluginSpecification { 'junit/framework/Protectable.class' ]) } + + @Issue('SHADOW-55') + def "remap class names for relocated files in project source"() { + given: + buildFile << """ + |apply plugin: 'java' + |apply plugin: ${ShadowPlugin.name} + | + |repositories { jcenter() } + | + |dependencies { + | compile 'junit:junit:3.8.2' + |} + | + |shadowJar { + | baseName = 'shadow' + | classifier = null + | relocate 'junit.framework', 'shadow.junit' + |} + """.stripMargin() + + file('src/main/java/shadow/ShadowTest.java') << ''' + |package shadow; + | + |import junit.framework.Test; + |import junit.framework.TestResult; + |public class ShadowTest implements Test { + | public int countTestCases() { return 0; } + | public void run(TestResult result) { } + |} + '''.stripMargin() + + when: + runner.arguments << 'shadowJar' + ExecutionResult result = runner.run() + + then: + success(result) + + and: + contains(output, [ + 'shadow/ShadowTest.class', + 'shadow/junit/Test.class', + ]) + + and: + doesNotContain(output, [ + 'junit/framework/Test.class' + ]) + + and: 'check that the class can be loaded. If the file was not relocated properly, we should get a NoDefClassFound' + // Isolated class loader with only the JVM system jars and the output jar from the test project + URLClassLoader classLoader = new URLClassLoader([output.toURI().toURL()] as URL[], + ClassLoader.systemClassLoader.parent) + classLoader.loadClass('shadow.ShadowTest') + } }