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

ASF GitHub Bot commented on MSHADE-467:
---------------------------------------

kriegaex commented on code in PR #210:
URL: 
https://github.com/apache/maven-shade-plugin/pull/210#discussion_r1460668776


##########
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java:
##########
@@ -1112,15 +1116,21 @@ private void 
rewriteDependencyReducedPomIfWeHaveReduction(
                     w.close();
                 }
 
-                ProjectBuildingRequest projectBuildingRequest =
-                        new 
DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
-                
projectBuildingRequest.setLocalRepository(session.getLocalRepository());
-                
projectBuildingRequest.setRemoteRepositories(project.getRemoteArtifactRepositories());
+                // Lock critical section to fix MSHADE-467
+                try {

Review Comment:
   LGTM, thanks for the great input. I made sure your authorship was respected 
in the latest commit. The CI build on my fork is passing. Before merging, would 
you like me to squash our two solutions into one commit? I wwould prefer to 
keep both commits for reference and to see the code evolution. Should the 
necessity for a reentrant lock arise in the future, at least we have an 
example. We also have one already for what is generally preferable, like in 
this case. As I cannot merge myself, I leave it up to you to decide.





> Dependency-reduced POM with missing exclusions in concurrent build
> ------------------------------------------------------------------
>
>                 Key: MSHADE-467
>                 URL: https://issues.apache.org/jira/browse/MSHADE-467
>             Project: Maven Shade Plugin
>          Issue Type: Bug
>    Affects Versions: 3.4.1, 3.5.0, 3.5.1
>            Reporter: Alexander Kriegisch
>            Priority: Major
>
> As discussed in [this maven-users 
> thread|https://lists.apache.org/thread/ljdbh9h0lqs1qsf9o8bnm35mtr85y4vr], 
> when running Maven builds for [this 
> reproducer|https://github.com/stl543/shadeMT] concurrently, some dependency 
> exclusions are missing from the generated dependency-reduced POMs. Run the 
> reproducer like this:
> {code:none}
> $ mvn clean package -T 4 | grep 'BUILD SUCCESS' && find . -name 
> "dependency-reduced-pom.xml" -exec bash -c 'cat $0 | grep exclu | wc -l ' {} 
> \;
> [INFO] BUILD SUCCESS
> 12
> 12
> 12
> 12
> {code}
> When running with {{-T 1}}, however, the correct result is 20.
> This concurrency issue might have been introduced by the MSHADE-413 fix, but 
> I am not sure.
> I have a local fix, synchronising on the injected mojo parameter 
> {{MavenSession session}}  in the lower section of 
> {{ShadeMojo::rewriteDependencyReducedPomIfWeHaveReduction}}. There might be a 
> better way to fix the problem, but I it leave that up to the PR reviewers to 
> come up with one, if any.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to