This is an automated email from the ASF dual-hosted git repository. rmannibucau pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/maven-shade-plugin.git
The following commit(s) were added to refs/heads/master by this push: new 2682f6e [MSHADE-306] - log all duplicates to not silently swallow a duplicate and an implicit selection 2682f6e is described below commit 2682f6ec4212dad227f691e40f6067969dc503b3 Author: Romain Manni-Bucau <rmannibu...@apache.org> AuthorDate: Sat Dec 15 20:44:05 2018 +0100 [MSHADE-306] - log all duplicates to not silently swallow a duplicate and an implicit selection --- .../apache/maven/plugins/shade/DefaultShader.java | 48 +++++++++++--- .../maven/plugins/shade/DefaultShaderTest.java | 73 ++++++++++++++++++++++ 2 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/shade/DefaultShader.java b/src/main/java/org/apache/maven/plugins/shade/DefaultShader.java index c4d677a..80ab074 100644 --- a/src/main/java/org/apache/maven/plugins/shade/DefaultShader.java +++ b/src/main/java/org/apache/maven/plugins/shade/DefaultShader.java @@ -30,9 +30,11 @@ import java.io.Writer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Set; import java.util.jar.JarEntry; @@ -226,9 +228,9 @@ public class DefaultShader } } + duplicates.put( name, jar ); if ( name.endsWith( ".class" ) ) { - duplicates.put( name, jar ); addRemappedClass( remapper, jos, jar, name, in ); } else if ( shadeRequest.isShadeSourcesContent() && name.endsWith( ".java" ) ) @@ -248,6 +250,7 @@ public class DefaultShader // Avoid duplicates that aren't accounted for by the resource transformers if ( resources.contains( mappedName ) ) { + getLogger().debug( "We have a duplicate " + name + " in " + jar ); return; } @@ -314,28 +317,55 @@ public class DefaultShader jarzS.add( jjar.getName() ); } - List<String> classes = new ArrayList<>(); + Collections.sort( jarzS ); // deterministic messages to be able to compare outputs (useful on CI) - for ( String clazz : overlapping.get( jarz ) ) + List<String> classes = new LinkedList<>(); + List<String> resources = new LinkedList<>(); + + for ( String name : overlapping.get( jarz ) ) { - classes.add( clazz.replace( ".class", "" ).replace( "/", "." ) ); + if ( name.endsWith( ".class" ) ) + { + classes.add( name.replace( ".class", "" ).replace( "/", "." ) ); + } + else + { + resources.add( name ); + } } //CHECKSTYLE_OFF: LineLength + final Collection<String> overlaps = new ArrayList<>(); + if ( !classes.isEmpty() ) + { + overlaps.add( "classes" ); + } + if ( !resources.isEmpty() ) + { + overlaps.add( "resources" ); + } + + final List<String> all = new ArrayList<>( classes.size() + resources.size() ); + all.addAll( classes ); + all.addAll( resources ); + getLogger().warn( - Joiner.on( ", " ).join( jarzS ) + " define " + classes.size() + " overlapping classes: " ); + Joiner.on( ", " ).join( jarzS ) + " define " + all.size() + + " overlapping " + Joiner.on( " and " ).join( overlaps ) + ": " ); //CHECKSTYLE_ON: LineLength + Collections.sort( all ); + int max = 10; - for ( int i = 0; i < Math.min( max, classes.size() ); i++ ) + for ( int i = 0; i < Math.min( max, all.size() ); i++ ) { - getLogger().warn( " - " + classes.get( i ) ); + getLogger().warn( " - " + all.get( i ) ); } - if ( classes.size() > max ) + if ( all.size() > max ) { - getLogger().warn( " - " + ( classes.size() - max ) + " more..." ); + getLogger().warn( " - " + ( all.size() - max ) + " more..." ); } } diff --git a/src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java b/src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java index 08eedea..54be8dd 100644 --- a/src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java +++ b/src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java @@ -20,21 +20,25 @@ package org.apache.maven.plugins.shade; */ import java.io.File; +import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import junit.framework.TestCase; +import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.shade.filter.Filter; import org.apache.maven.plugins.shade.relocation.Relocator; import org.apache.maven.plugins.shade.relocation.SimpleRelocator; import org.apache.maven.plugins.shade.resource.ComponentsXmlResourceTransformer; import org.apache.maven.plugins.shade.resource.ResourceTransformer; +import org.codehaus.plexus.logging.AbstractLogger; import org.codehaus.plexus.logging.Logger; import org.codehaus.plexus.logging.console.ConsoleLogger; import org.objectweb.asm.ClassReader; @@ -51,6 +55,75 @@ public class DefaultShaderTest private static final String[] EXCLUDES = new String[] { "org/codehaus/plexus/util/xml/Xpp3Dom", "org/codehaus/plexus/util/xml/pull.*" }; + public void testOverlappingResourcesAreLogged() throws IOException, MojoExecutionException { + final DefaultShader shader = new DefaultShader(); + final List<String> debugMessages = new ArrayList<>(); + final List<String> warnMessages = new ArrayList<>(); + shader.enableLogging( new AbstractLogger( + Logger.LEVEL_INFO, "TEST_DefaultShaderTest_testOverlappingResourcesAreLogged" ) + { + @Override + public void debug( final String s, final Throwable throwable ) + { + debugMessages.add(s); + } + + @Override + public void info( final String s, final Throwable throwable ) + { + // no-op + } + + @Override + public void warn( final String s, final Throwable throwable ) + { + warnMessages.add(s); + } + + @Override + public void error( final String s, final Throwable throwable ) + { + // no-op + } + + @Override + public void fatalError( final String s, final Throwable throwable ) + { + // no-op + } + + @Override + public Logger getChildLogger( final String s ) + { + return this; + } + }); + + // we will shade two jars and expect to see META-INF/MANIFEST.MF overlaps, this will always be true + // but this can lead to a broken deployment if intended for OSGi or so, so even this should be logged + final Set<File> set = new LinkedHashSet<File>(); + set.add( new File( "src/test/jars/test-project-1.0-SNAPSHOT.jar" ) ); + set.add( new File( "src/test/jars/plexus-utils-1.4.1.jar" ) ); + + final ShadeRequest shadeRequest = new ShadeRequest(); + shadeRequest.setJars( set ); + shadeRequest.setRelocators( Collections.<Relocator>emptyList() ); + shadeRequest.setResourceTransformers( Collections.<ResourceTransformer>emptyList() ); + shadeRequest.setFilters( Collections.<Filter>emptyList() ); + shadeRequest.setUberJar( new File( "target/foo-custom_testOverlappingResourcesAreLogged.jar" ) ); + shader.shade( shadeRequest ); + + final String failureWarnMessage = warnMessages.toString(); + assertTrue(failureWarnMessage, warnMessages.contains( + "plexus-utils-1.4.1.jar, test-project-1.0-SNAPSHOT.jar define 1 overlapping resources: ")); + assertTrue(failureWarnMessage, warnMessages.contains(" - META-INF/MANIFEST.MF")); + + final String failureDebugMessage = debugMessages.toString(); + assertTrue(failureDebugMessage, debugMessages.contains( + "We have a duplicate META-INF/MANIFEST.MF in src/test/jars/plexus-utils-1.4.1.jar" + .replace('/', File.separatorChar))); + } + public void testShaderWithDefaultShadedPattern() throws Exception {