rmannibucau commented on a change in pull request #95:
URL: https://github.com/apache/maven-shade-plugin/pull/95#discussion_r636874735
##########
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:
A thread local is a hack in the sense it is a solution to keep track of
a data in a multithreaded application with reference breakage which is not the
case here. Here you have 1 instance of classreader/writer per class file which
can track the state of the rewritten class (plain boolean).
An example of such thing can be seen in sirona:
https://github.com/rmannibucau/sirona/blob/trunk/agent/javaagent/src/main/java/com/github/rmannibucau/sirona/javaagent/SironaClassVisitor.java#L139
So no additional instantiation in object count, no plugin refactoring or so
and simpler code path since the state becomes an attribute and not an unrelated
threadlocal so overall I don't see any reason to use a quick and dirty option
for this.
hope it makes sense
--
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]
