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.

Sign up for GitHub

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

[MSHADE-364] drop duplicates handled by a transformer #48

Merged
merged 1 commit into from
May 14, 2020
Merged

[MSHADE-364] drop duplicates handled by a transformer #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 20 additions & 2 deletions src/main/java/org/apache/maven/plugins/shade/DefaultShader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ else if ( shadeRequest.isShadeSourcesContent() && name.endsWith( ".java" ) )

addResource( resources, jos, mappedName, entry.getTime(), in );
}
else
{
duplicates.remove( name, jar );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could there be a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added https://github.com/apache/maven-shade-plugin/pull/48/files#diff-bc8729c3b7ec6843be39d656cb9b2d40R96

}
}
}
}
Expand Down Expand Up @@ -338,11 +342,25 @@ private void logSummaryOfDuplicates( Multimap<Collection<File>, String> overlapp
final Collection<String> overlaps = new ArrayList<>();
if ( !classes.isEmpty() )
{
overlaps.add( "classes" );
if ( resources.size() == 1 )
{
overlaps.add( "class" );
}
else
{
overlaps.add( "classes" );
}
}
if ( !resources.isEmpty() )
{
overlaps.add( "resources" );
if ( resources.size() == 1 )
{
overlaps.add( "resource" );
}
else
{
overlaps.add( "resources" );
}
}

final List<String> all = new ArrayList<>( classes.size() + resources.size() );
Expand Down
161 changes: 114 additions & 47 deletions src/test/java/org/apache/maven/plugins/shade/DefaultShaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,36 @@
*/

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.lang.reflect.Field;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.StandardCharsets;
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 java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;

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.AppendingTransformer;
import org.apache.maven.plugins.shade.resource.ComponentsXmlResourceTransformer;
import org.apache.maven.plugins.shade.resource.ManifestResourceTransformer;
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.junit.Rule;
import org.junit.rules.TemporaryFolder;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.Opcodes;
Expand All @@ -57,47 +66,8 @@ public class DefaultShaderTest

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.replace( '\\', '/' ).trim() );
}

@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.replace( '\\', '/' ).trim() );
}

@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;
}
});
final MockLogger logs = new MockLogger();
shader.enableLogging(logs);

// 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
Expand All @@ -113,16 +83,67 @@ public Logger getChildLogger( final String s )
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 failureWarnMessage = logs.warnMessages.toString();
assertTrue(failureWarnMessage, logs.warnMessages.contains(
"plexus-utils-1.4.1.jar, test-project-1.0-SNAPSHOT.jar define 1 overlapping resource:"));
assertTrue(failureWarnMessage, logs.warnMessages.contains("- META-INF/MANIFEST.MF"));

final String failureDebugMessage = debugMessages.toString();
assertTrue(failureDebugMessage, debugMessages.contains(
final String failureDebugMessage = logs.debugMessages.toString();
assertTrue(failureDebugMessage, logs.debugMessages.contains(
"We have a duplicate META-INF/MANIFEST.MF in src/test/jars/plexus-utils-1.4.1.jar" ));
}

public void testOverlappingResourcesAreLoggedExceptATransformerHandlesIt() throws Exception {
TemporaryFolder temporaryFolder = new TemporaryFolder();
Set<File> set = new LinkedHashSet<>();
temporaryFolder.create();
File j1 = temporaryFolder.newFile("j1.jar");
try ( JarOutputStream jos = new JarOutputStream(new FileOutputStream( j1 ) ) )
{
jos.putNextEntry(new JarEntry( "foo.txt" ));
jos.write("c1".getBytes(StandardCharsets.UTF_8));
jos.closeEntry();
}
File j2 = temporaryFolder.newFile("j2.jar");
try ( JarOutputStream jos = new JarOutputStream(new FileOutputStream( j2 ) ) )
{
jos.putNextEntry(new JarEntry( "foo.txt" ));
jos.write("c2".getBytes(StandardCharsets.UTF_8));
jos.closeEntry();
}
set.add( j1 );
set.add( j2 );

AppendingTransformer transformer = new AppendingTransformer();
Field resource = AppendingTransformer.class.getDeclaredField( "resource" );
resource.setAccessible( true );
resource.set( transformer, "foo.txt" );

ShadeRequest shadeRequest = new ShadeRequest();
shadeRequest.setJars( set );
shadeRequest.setRelocators( Collections.<Relocator>emptyList() );
shadeRequest.setResourceTransformers( Collections.<ResourceTransformer>singletonList( transformer) );
shadeRequest.setFilters( Collections.<Filter>emptyList() );
shadeRequest.setUberJar( new File( "target/foo-custom_testOverlappingResourcesAreLogged.jar" ) );

DefaultShader shaderWithTransformer = new DefaultShader();
final MockLogger logWithTransformer = new MockLogger();
shaderWithTransformer.enableLogging( logWithTransformer );
shaderWithTransformer.shade( shadeRequest );

DefaultShader shaderWithoutTransformer = new DefaultShader();
MockLogger logWithoutTransformer = new MockLogger();
shaderWithoutTransformer.enableLogging( logWithoutTransformer );
shadeRequest.setResourceTransformers( Collections.<ResourceTransformer>emptyList() );
shaderWithoutTransformer.shade( shadeRequest );

temporaryFolder.delete();

assertTrue(logWithTransformer.warnMessages.toString(), logWithTransformer.warnMessages.isEmpty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the purpose of this PR to add another warning to the log?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove it

assertTrue(logWithoutTransformer.warnMessages.toString(), logWithoutTransformer.warnMessages.containsAll(
Arrays.<String>asList( "j1.jar, j2.jar define 1 overlapping resource:", "- foo.txt" ) ) );
}

public void testShaderWithDefaultShadedPattern()
throws Exception
{
Expand Down Expand Up @@ -274,4 +295,50 @@ private static DefaultShader newShader()
return s;
}

private static class MockLogger extends AbstractLogger
{
private final List<String> debugMessages = new ArrayList<>();
private final List<String> warnMessages = new ArrayList<>();

private MockLogger()
{
super( Logger.LEVEL_INFO, "test" );
}

@Override
public void debug( String s, Throwable throwable )
{
debugMessages.add( s.replace( '\\', '/' ).trim() );
}

@Override
public void info( String s, Throwable throwable )
{
// no-op
}

@Override
public void warn( String s, Throwable throwable )
{
warnMessages.add( s.replace( '\\', '/' ).trim() );
}

@Override
public void error( String s, Throwable throwable )
{
// no-op
}

@Override
public void fatalError( String s, Throwable throwable )
{
// no-op
}

@Override
public Logger getChildLogger( String s )
{
return this;
}
}
}