gnodet commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4057965843


##########
impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java:
##########
@@ -898,13 +899,12 @@ public Set<Artifact> getArtifacts() {
             if (artifactFilter == null || resolvedArtifacts == null) {
                 artifacts = new LinkedHashSet<>();
             } else {
-                Set<Artifact> result = new 
LinkedHashSet<>(resolvedArtifacts.size() * 2);
+                artifacts = new LinkedHashSet<>(resolvedArtifacts.size() * 2);
                 for (Artifact artifact : resolvedArtifacts) {
                     if (artifactFilter.include(artifact)) {
-                        result.add(artifact);
+                        artifacts.add(artifact);
                     }

Review Comment:
   Fixed in 2272eae465: restored the local `result` variable so `artifacts` is 
only assigned after the loop completes. Concurrent readers can no longer 
observe a partially-populated set.



##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java:
##########
@@ -289,8 +294,11 @@ public UpgradeResult doApply(UpgradeContext context, 
Map<Path, Document> pomMap)
                 }
             }
 
+            // Clean up temp directory
+            cleanupTempDirectory(tempDir);

Review Comment:
   Fixed in 2272eae465: declared `tempDir` before the `try` block (initialized 
to `null`) and moved `cleanupTempDirectory()` into a `finally` block with a 
null guard. The temp directory is now always cleaned up, even when an exception 
is thrown.



##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java:
##########
@@ -560,28 +568,12 @@ private boolean upgradePropertyVersion(
             return false; // Found in current POM, no upgrade needed
         }

Review Comment:
   Fixed in 2272eae465: restored the ancestor-directory guard — 
`currentPomPath`/`currentDir` lookup and the `candidateDir.startsWith` check — 
so only POMs in ancestor directories (potential Maven parents) are searched 
when looking up a property cross-POM.



-- 
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.

To unsubscribe, e-mail: [email protected]

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

Reply via email to