rmannibucau commented on a change in pull request #95:
URL: https://github.com/apache/maven-shade-plugin/pull/95#discussion_r637082823



##########
File path: src/main/java/org/apache/maven/plugins/shade/DefaultShader.java
##########
@@ -694,6 +702,11 @@ private void addResource( Set<String> resources, 
JarOutputStream jos, String nam
 
         List<Relocator> relocators;
 
+        // Use thread-local, just in case 'map*' calls are ever done 
concurrently. Make sure that the using class
+        // initialises this value according to its needs, usually setting the 
value to false per file before starting
+        // relocation.
+        ThreadLocal<Boolean> wasRelocated = new ThreadLocal<>();

Review comment:
       Start by creating a ShadeClassRemapper (a visitor) and each time 
remapper can be call test it was called or not.
   You can even just use a boolean in the remapper  since it is single thread 
and can be simpler at the end but in terms of arch it is the visitor which 
would hold that logic by decorating remapper usages.
   
   Another thing to test is just to compare the byte[] before and after, guess 
in several cases it will be the same and it can be a sufficient first impl but 
the remapper decoration is what makes the code the more straight forward and 
less hacky IMHO.
   
   Can surely hack it next week if you need help about this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to