You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by rm...@apache.org on 2021/08/16 10:47:00 UTC

[maven-shade-plugin] branch master updated (c13c9bb -> c648ccf)

This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/maven-shade-plugin.git.


    from c13c9bb  Bump slf4j.version from 1.7.31 to 1.7.32 (#113)
     new 1dca37c  [MSHADE-396] Improve SourceContent Shading
     new c648ccf  [MSHADE-396] Add explanation to shadeSourcesContent documentation

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/maven/plugins/shade/mojo/ShadeMojo.java | 16 ++++++--
 .../plugins/shade/relocation/SimpleRelocator.java  | 41 ++++++++++++++-----
 .../shade/relocation/SimpleRelocatorTest.java      | 46 ++++++++++++++++++----
 3 files changed, 83 insertions(+), 20 deletions(-)

[maven-shade-plugin] 02/02: [MSHADE-396] Add explanation to shadeSourcesContent documentation

Posted by rm...@apache.org.
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

commit c648ccfade767acd5a59893a1719b58ed02fad99
Author: Alexander Kriegisch <Al...@Kriegisch.name>
AuthorDate: Mon Jul 12 16:01:33 2021 +0700

    [MSHADE-396] Add explanation to shadeSourcesContent documentation
    
    Explain purpose and limitations of heuristic source code shading
    approach.
---
 .../org/apache/maven/plugins/shade/mojo/ShadeMojo.java   | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java b/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java
index 6cc019c..d717253 100644
--- a/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java
+++ b/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java
@@ -327,9 +327,19 @@ public class ShadeMojo
     private boolean createTestSourcesJar;
 
     /**
-     * When true, it will attempt to shade the contents of the java source files when creating the sources jar. When
-     * false, it will just relocate the java source files to the shaded paths, but will not modify the actual contents
-     * of the java source files.
+     * When true, it will attempt to shade the contents of Java source files when creating the sources JAR. When false,
+     * it will just relocate the Java source files to the shaded paths, but will not modify the actual source file
+     * contents.
+     * <p>
+     * <b>Please note:</b> This feature uses a heuristic search & replace approach which covers many, but definitely not
+     * all possible cases of source code shading and its excludes. There is no full Java parser behind this
+     * functionality, which would be the only way to get this right for Java language elements. As for matching within
+     * Java string constants, this is next to impossible to get 100% right, trying to guess if they are used in
+     * reflection or not.
+     * <p>
+     * Please understand that the source shading feature is not meant as a source code generator anyway, merely as a
+     * tool creating reasonably plausible source code when navigating to a relocated library class from an IDE,
+     * hopefully displaying source code which makes 95% sense - no more, no less.
      */
     @Parameter( property = "shadeSourcesContent", defaultValue = "false" )
     private boolean shadeSourcesContent;

[maven-shade-plugin] 01/02: [MSHADE-396] Improve SourceContent Shading

Posted by rm...@apache.org.
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

commit 1dca37c71649fa4f4aa110de3b78cc98d0e47eda
Author: Alexander Kriegisch <Al...@Kriegisch.name>
AuthorDate: Mon Jul 12 14:44:53 2021 +0700

    [MSHADE-396] Improve SourceContent Shading
    
    Improve search & replace heuristics without destroying previously
    correct replacements in my test project (AspectJ). This solution is
    still bound to fail in some situations, simply because it is just a
    heuristic approach and not a full Java parser correctly recognising
    package names in all possible situations in Java source code. As for
    matching within Java string constants, this is next to impossible to get
    100% right.
    
    But the source shading feature is not meant as a source code
    generator anyway, merely as a tool creating reasonably plausible source
    code when navigating to a relocated library class from an IDE, hopefully
    displaying source code which makes 95% sense - no more, no less.
---
 .../plugins/shade/relocation/SimpleRelocator.java  | 41 ++++++++++++++-----
 .../shade/relocation/SimpleRelocatorTest.java      | 46 ++++++++++++++++++----
 2 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java b/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java
index 083957f..3837a67 100644
--- a/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java
+++ b/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java
@@ -34,6 +34,23 @@ import java.util.regex.Pattern;
 public class SimpleRelocator
     implements Relocator
 {
+    /**
+     * Match dot, slash or space at end of string
+     */
+    private static final Pattern RX_ENDS_WITH_DOT_SLASH_SPACE = Pattern.compile( "[./ ]$" );
+
+    /**
+     * Match <ul>
+     *     <li>certain Java keywords + space</li>
+     *     <li>beginning of Javadoc link + optional line breaks and continuations with '*'</li>
+     * </ul>
+     * at end of string
+     */
+    private static final Pattern RX_ENDS_WITH_JAVA_KEYWORD = Pattern.compile(
+        "\\b(import|package|public|protected|private|static|final|synchronized|abstract|volatile) $"
+            + "|"
+            + "\\{@link( \\*)* $"
+    );
 
     private final String pattern;
 
@@ -104,7 +121,7 @@ public class SimpleRelocator
         {
             this.includes.addAll( includes );
         }
-        
+
         if ( excludes != null && !excludes.isEmpty() )
         {
             this.excludes.addAll( excludes );
@@ -121,7 +138,7 @@ public class SimpleRelocator
                     sourcePackageExcludes.add( exclude.substring( pattern.length() ).replaceFirst( "[.][*]$", "" ) );
                 }
                 // Excludes should be subpackages of the global pattern
-                else if ( exclude.startsWith( pathPattern ) )
+                if ( exclude.startsWith( pathPattern ) )
                 {
                     sourcePathExcludes.add( exclude.substring( pathPattern.length() ).replaceFirst( "[/][*]$", "" ) );
                 }
@@ -234,11 +251,8 @@ public class SimpleRelocator
         {
             return sourceContent;
         }
-        else
-        {
-            sourceContent = shadeSourceWithExcludes( sourceContent, pattern, shadedPattern, sourcePackageExcludes );
-            return shadeSourceWithExcludes( sourceContent, pathPattern, shadedPathPattern, sourcePathExcludes );
-        }
+        sourceContent = shadeSourceWithExcludes( sourceContent, pattern, shadedPattern, sourcePackageExcludes );
+        return shadeSourceWithExcludes( sourceContent, pathPattern, shadedPathPattern, sourcePathExcludes );
     }
 
     private String shadeSourceWithExcludes( String sourceContent, String patternFrom, String patternTo,
@@ -248,8 +262,11 @@ public class SimpleRelocator
         StringBuilder shadedSourceContent = new StringBuilder( sourceContent.length() * 11 / 10 );
         boolean isFirstSnippet = true;
         // Make sure that search pattern starts at word boundary and we look for literal ".", not regex jokers
-        for ( String snippet : sourceContent.split( "\\b" + patternFrom.replace( ".", "[.]" ) ) )
+        String[] snippets = sourceContent.split( "\\b" + patternFrom.replace( ".", "[.]" ) + "\\b" );
+        for ( int i = 0, snippetsLength = snippets.length; i < snippetsLength; i++ )
         {
+            String snippet = snippets[i];
+            String previousSnippet = isFirstSnippet ? "" : snippets[i - 1];
             boolean doExclude = false;
             for ( String excludedPattern : excludedPatterns )
             {
@@ -263,10 +280,14 @@ public class SimpleRelocator
             {
                 shadedSourceContent.append( snippet );
                 isFirstSnippet = false;
-        }
+            }
             else
             {
-                shadedSourceContent.append( doExclude ? patternFrom : patternTo ).append( snippet );
+                String previousSnippetOneLine = previousSnippet.replaceAll( "\\s+", " " );
+                boolean afterDotSlashSpace = RX_ENDS_WITH_DOT_SLASH_SPACE.matcher( previousSnippetOneLine ).find();
+                boolean afterJavaKeyWord = RX_ENDS_WITH_JAVA_KEYWORD.matcher( previousSnippetOneLine ).find();
+                boolean shouldExclude = doExclude || afterDotSlashSpace && !afterJavaKeyWord;
+                shadedSourceContent.append( shouldExclude ? patternFrom : patternTo ).append( snippet );
             }
         }
         return shadedSourceContent.toString();
diff --git a/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java b/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java
index 2934d0d..e85972a 100644
--- a/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java
+++ b/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java
@@ -8,9 +8,9 @@ package org.apache.maven.plugins.shade.relocation;
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -103,7 +103,7 @@ public class SimpleRelocatorTest
 
         relocator = new SimpleRelocator( "org/foo", null, null, null, true );
         assertTrue( relocator.canRelocatePath( "(I)org/foo/bar/Class;" ) );
-        
+
         relocator = new SimpleRelocator( "^META-INF/org.foo.xml$", null, null, null, true );
         assertTrue( relocator.canRelocatePath( "META-INF/org.foo.xml" ) );
     }
@@ -173,7 +173,7 @@ public class SimpleRelocatorTest
         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( "META-INF/org.foo.xml" ) );
     }
-    
+
     @Test
     public void testRelocateMavenFiles()
     {
@@ -191,6 +191,7 @@ public class SimpleRelocatorTest
 
     private static final String sourceFile =
             "package org.apache.maven.hello;\n" +
+            "package org.objectweb.asm;\n" +
             "\n" +
             "import foo.bar.Bar;\n" +
             "import zot.baz.Baz;\n" +
@@ -201,12 +202,19 @@ public class SimpleRelocatorTest
             "import org.apache.maven.In;\n" +
             "import org.apache.maven.e.InE;\n" +
             "import org.apache.maven.f.g.InFG;\n" +
+            "import java.io.IOException;\n" +
             "\n" +
+            "/**\n" +
+            " * Also check out {@link org.apache.maven.hello.OtherClass} and {@link\n" +
+            " * org.apache.maven.hello.YetAnotherClass}\n" +
+            " */\n" +
             "public class MyClass {\n" +
             "  private org.apache.maven.exclude1.x.X myX;\n" +
-            "  private org.apache.maven.h.H;\n" +
+            "  private org.apache.maven.h.H h;\n" +
+            "  private String ioInput;\n" +
             "\n" +
             "  public void doSomething() {\n" +
+            "    String io, val;\n" +
             "    String noRelocation = \"NoWordBoundaryXXXorg.apache.maven.In\";\n" +
             "    String relocationPackage = \"org.apache.maven.In\";\n" +
             "    String relocationPath = \"org/apache/maven/In\";\n" +
@@ -215,6 +223,7 @@ public class SimpleRelocatorTest
 
     private static final String relocatedFile =
             "package com.acme.maven.hello;\n" +
+            "package aj.org.objectweb.asm;\n" +
             "\n" +
             "import foo.bar.Bar;\n" +
             "import zot.baz.Baz;\n" +
@@ -225,12 +234,19 @@ public class SimpleRelocatorTest
             "import com.acme.maven.In;\n" +
             "import com.acme.maven.e.InE;\n" +
             "import com.acme.maven.f.g.InFG;\n" +
+            "import java.io.IOException;\n" +
             "\n" +
+            "/**\n" +
+            " * Also check out {@link com.acme.maven.hello.OtherClass} and {@link\n" +
+            " * com.acme.maven.hello.YetAnotherClass}\n" +
+            " */\n" +
             "public class MyClass {\n" +
             "  private org.apache.maven.exclude1.x.X myX;\n" +
-            "  private com.acme.maven.h.H;\n" +
+            "  private com.acme.maven.h.H h;\n" +
+            "  private String ioInput;\n" +
             "\n" +
             "  public void doSomething() {\n" +
+            "    String io, val;\n" +
             "    String noRelocation = \"NoWordBoundaryXXXorg.apache.maven.In\";\n" +
             "    String relocationPackage = \"com.acme.maven.In\";\n" +
             "    String relocationPath = \"com/acme/maven/In\";\n" +
@@ -250,9 +266,25 @@ public class SimpleRelocatorTest
     @Test
     public void testRelocateSourceWithExcludes()
     {
+        // Main relocator with in-/excludes
         SimpleRelocator relocator = new SimpleRelocator( "org.apache.maven", "com.acme.maven",
                 Arrays.asList( "foo.bar", "zot.baz" ),
                 Arrays.asList( "irrelevant.exclude", "org.apache.maven.exclude1", "org.apache.maven.sub.exclude2" ) );
-        assertEquals( relocatedFile,  relocator.applyToSourceContent( sourceFile ) );
+        // Make sure not to replace variables 'io' and 'ioInput', package 'java.io'
+        SimpleRelocator ioRelocator = new SimpleRelocator( "io", "shaded.io", null, null );
+        // Check corner case which was not working in PR #100
+        SimpleRelocator asmRelocator = new SimpleRelocator( "org.objectweb.asm", "aj.org.objectweb.asm", null, null );
+        // Make sure not to replace 'foo' package by path-like 'shaded/foo'
+        SimpleRelocator fooRelocator = new SimpleRelocator( "foo", "shaded.foo", null, Arrays.asList( "foo.bar" ) );
+        assertEquals(
+            relocatedFile,
+            fooRelocator.applyToSourceContent(
+                asmRelocator.applyToSourceContent(
+                    ioRelocator.applyToSourceContent(
+                        relocator.applyToSourceContent( sourceFile )
+                    )
+                )
+            )
+        );
     }
 }