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



##########
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:
       @kriegaex the thread local issue is that it is easily breakable - it is 
fragile code, wrongly auto-document the code (threadlocal is mainly about 
multithreaded code and here you have a single thread) and is easily manipulable 
by code not belonging to this layer (it is not the remapper which owns that 
logic but the visitor which actually delegates to the writer or writer itself 
which creates the class). This is why I say it is a quick and dirty 
fix/workaround and not a future proof solution. Will work on it on monday 
forking your branch.




-- 
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:
us...@infra.apache.org


Reply via email to