[jira] [Commented] (MSHADE-252) shadeSourcesContent is broken when combined with partial relocation

["Alexander Kriegisch (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MSHADE-252?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17314377#comment-17314377 ] 

Alexander Kriegisch commented on MSHADE-252:
--------------------------------------------

[~snyangzhenyu99], there are several problems with your setup:
 * There seems to be a typo in your {{shadedPattern}}. You should remove the leading dot because the dot is just a package separator and does not make sense in a leading position. It would not even be a legal package name and not compile in Java.
 * Please avoid shading a package into a subpackage of its own like {{com.fake}} to {{com.fake.shaded}}. You could potentially be getting some kind of recursive problem and depending on the algorithm used for replacing package and path names you could be lucky and it works or not. In your case it works for binaries because Maven Shade uses an ASM utility class for relocating binaries, which seems to do a good job.
 * You are not so lucky with the source code, simply because there is and always has been a bug in sources shading: If a source file matches {{pattern}}, the replacement to {{shadedPattern}} will always be done *globally* throughout the file using a naive {{sourceContent.replaceAll( "\\b" + pattern, shadedPattern )}}, which would also not work correctly for a simple (non-recurive) relocation such as {{com.fake}} to {{org.acme.shaded}}, because not only is there the naive {{replaceAll}} call, but also any exclusions are completely ignored by the plugin during replacement.

Now after the bad news, here are the good ones:
 * I have fixed that bug, scratching my own itch, because I had the same problem for the non-recursive case in Maven Shade 3.2.4. I am going to create a pull request. I have never contributed to this project before, so let us hope the PR is going to be accepted and a bugfix release published soon.
 * Somewhat to my own surprise, my bugfix even works in your case, i.e. when shading a package into its own subpackage with excludes. (You have got to remove the leading dot though, like I said.)

> shadeSourcesContent is broken when combined with partial relocation
> -------------------------------------------------------------------
>
>                 Key: MSHADE-252
>                 URL: https://issues.apache.org/jira/browse/MSHADE-252
>             Project: Maven Shade Plugin
>          Issue Type: Bug
>    Affects Versions: 2.4.3, 3.0.0
>            Reporter: Zhenyu Yang
>            Priority: Major
>              Labels: easyfix
>
> per description in https://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#shadeSourcesContent, when set it to true, 
> "it will attempt to shade the contents of the java source files when creating the sources jar." However, it seems will blindly shade all the source files include those are excluded from relocation rules.
> This could be illustrated with a simple example:
> Assume there are two classes defined in two packages as below:
> {code:title=A.java}
> package com.fake.fooA;
> import com.fake.fooB;
> public Class A {}
> {code}
> {code:title=B.java}
> package com.fake.fooB;
> import com.fake.fooA;
> public class B {}
> {code}
> and the maven config looks like:
> {code:xml}
>      <plugin>
>         <groupId>org.apache.maven.plugins</groupId>
>         <artifactId>maven-shade-plugin</artifactId>
>         <version>2.4.3</version>
>         <executions>
>           <!-- Run shade goal on package phase -->
>           <execution>
>             <phase>package</phase>
>             <goals>
>               <goal>shade</goal>
>             </goals>
>             <configuration>
>               <shadedArtifactAttached>false</shadedArtifactAttached>
>               <createSourcesJar>true</createSourcesJar>
>               <shadeSourcesContent>true</shadeSourcesContent>
>               <relocations>
>                 <relocation>
>                   <pattern>com.fake</pattern>
>                   <shadedPattern>.com.fake.shaded</shadedPattern>
>                   <excludes>
>                     <exclude>com.fake.fooA.*</exclude>
>                   </excludes>
>                 </relocation>
>                </relocations> 
> {code}
> Then the shade plugin will modify the B's source file to be:
> {code}
> package com.fake.shaded.fooB;
> import com.fake.shaded.fooA;
> public class B {}
> {code}
> Notice that package A's path was also updated, which is wrong as it's not got relocated.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)