Copilot commented on code in PR #639:
URL: https://github.com/apache/maven-war-plugin/pull/639#discussion_r3652633508
##########
src/main/java/org/apache/maven/plugins/war/packaging/OverlayPackagingTask.java:
##########
@@ -61,6 +64,9 @@ public void performPackaging(WarPackagingContext context)
throws MojoExecutionEx
// Step1: Extract if necessary
final File tmpDir = unpackOverlay(context, overlay);
+ // Step1b: Remove jars from overlay that conflict with managed
dependencies
+ filterConflictingDependencyJars(context, tmpDir);
+
Review Comment:
filterConflictingDependencyJars() deletes jars from the overlay unpack
directory (tmpDir). Because unpackOverlay() reuses that directory when it’s
non-empty and newer than the overlay artifact, this permanently mutates the
cached unpacked overlay and can affect subsequent builds (e.g., if
dependencyManagement changes, or if another execution expects the original
overlay contents). Consider avoiding in-place mutation (e.g., exclude during
copy, unpack into a per-execution directory, or restore/re-unpack after copying
when deletions occurred).
##########
src/main/java/org/apache/maven/plugins/war/packaging/OverlayPackagingTask.java:
##########
@@ -127,4 +133,72 @@ protected File getOverlayTempDirectory(WarPackagingContext
context, Overlay over
}
return result;
}
+
+ /**
+ * Removes jars from the overlay's {@code WEB-INF/lib} that conflict with
the project's managed
+ * dependencies. When a project uses dependencyManagement to pin a
version, any jar from the
+ * overlay with the same artifactId but a different version is removed,
ensuring only the
+ * dependency-managed version ends up in {@code WEB-INF/lib}.
+ *
+ * @param context the packaging context
+ * @param overlayDir the unpacked overlay directory
+ */
+ private void filterConflictingDependencyJars(WarPackagingContext context,
File overlayDir) {
+ File libDir = new File(overlayDir, "WEB-INF/lib");
+ if (!libDir.isDirectory()) {
+ return;
+ }
+
+ Set<String> projectArtifactIds = new HashSet<>();
+ for (Artifact artifact : context.getProject().getArtifacts()) {
+ if (!artifact.isOptional() && "jar".equals(artifact.getType())) {
+ projectArtifactIds.add(artifact.getArtifactId());
+ }
+ }
Review Comment:
filterConflictingDependencyJars() builds the projectArtifactIds set from all
project artifacts, but ArtifactsPackagingTask only packages runtime-scope
artifacts (ScopeArtifactFilter(Artifact.SCOPE_RUNTIME)). This can cause overlay
jars to be deleted even when the corresponding project artifact won’t be
packaged into WEB-INF/lib (e.g., provided scope), resulting in a missing
dependency at runtime.
##########
src/main/java/org/apache/maven/plugins/war/packaging/OverlayPackagingTask.java:
##########
@@ -127,4 +133,72 @@ protected File getOverlayTempDirectory(WarPackagingContext
context, Overlay over
}
return result;
}
+
+ /**
+ * Removes jars from the overlay's {@code WEB-INF/lib} that conflict with
the project's managed
+ * dependencies. When a project uses dependencyManagement to pin a
version, any jar from the
+ * overlay with the same artifactId but a different version is removed,
ensuring only the
+ * dependency-managed version ends up in {@code WEB-INF/lib}.
+ *
+ * @param context the packaging context
+ * @param overlayDir the unpacked overlay directory
+ */
+ private void filterConflictingDependencyJars(WarPackagingContext context,
File overlayDir) {
+ File libDir = new File(overlayDir, "WEB-INF/lib");
+ if (!libDir.isDirectory()) {
+ return;
+ }
+
+ Set<String> projectArtifactIds = new HashSet<>();
+ for (Artifact artifact : context.getProject().getArtifacts()) {
+ if (!artifact.isOptional() && "jar".equals(artifact.getType())) {
+ projectArtifactIds.add(artifact.getArtifactId());
+ }
+ }
+
+ if (projectArtifactIds.isEmpty()) {
+ return;
+ }
+
+ File[] overlayJars = libDir.listFiles((dir, name) ->
name.endsWith(".jar"));
+ if (overlayJars == null) {
+ return;
+ }
+
+ for (File overlayJar : overlayJars) {
+ String jarName = overlayJar.getName();
+ String artifactId = extractArtifactId(jarName);
+ if (artifactId != null && projectArtifactIds.contains(artifactId))
{
+ context.getLog()
+ .debug("Removing dependency [" + jarName + "] from
overlay [" + overlay.getId()
+ + "]; managed version in project already
provides it");
+ overlayJar.delete();
+ }
Review Comment:
overlayJar.delete() return value is ignored. If the delete fails, the log
still claims the jar was removed, and duplicate versions may remain in the
final WAR without any signal.
##########
src/it/MWAR-389/verify.bsh:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.*;
+import org.codehaus.plexus.util.*;
+
+/*
+ * Test for MWAR-220 / issue #389:
+ * dependencyManagement with overlays should not result in multiple versions
+ * of the same dependency in WEB-INF/lib.
+ *
+ * The overlay-war is built with plexus-utils 3.0.24.
+ * The main-war has dependencyManagement pinning plexus-utils to the managed
+ * version (@plexusUtilVersion@), and uses overlay-war as an overlay.
+ *
+ * If the issue still exists, both versions will appear in main-war's
WEB-INF/lib.
+ * If fixed, only the managed version will appear.
+ */
+
+String managedVersion = plexusUtilVersion;
+
+// --- Check overlay WAR content ---
+File overlayTarget = new File( basedir, "overlay-war/target" );
+File overlayWar = new File( overlayTarget, "overlay-war-1.0-SNAPSHOT" );
+File overlayLib = new File( overlayWar, "WEB-INF/lib" );
+
+String[] overlayJars = overlayLib.list();
+boolean overlayHasOld = false;
+boolean overlayHasManaged = false;
+
+for ( int i = 0; i < overlayJars.length; i++ )
Review Comment:
overlayLib.list() can return null (missing directory / IO error), which will
cause a NullPointerException when accessing overlayJars.length. Other IT verify
scripts check directory existence before listing.
This issue also appears on line 80 of the same file.
##########
src/main/java/org/apache/maven/plugins/war/packaging/OverlayPackagingTask.java:
##########
@@ -127,4 +133,72 @@ protected File getOverlayTempDirectory(WarPackagingContext
context, Overlay over
}
return result;
}
+
+ /**
+ * Removes jars from the overlay's {@code WEB-INF/lib} that conflict with
the project's managed
+ * dependencies. When a project uses dependencyManagement to pin a
version, any jar from the
+ * overlay with the same artifactId but a different version is removed,
ensuring only the
+ * dependency-managed version ends up in {@code WEB-INF/lib}.
+ *
+ * @param context the packaging context
+ * @param overlayDir the unpacked overlay directory
+ */
+ private void filterConflictingDependencyJars(WarPackagingContext context,
File overlayDir) {
+ File libDir = new File(overlayDir, "WEB-INF/lib");
+ if (!libDir.isDirectory()) {
+ return;
+ }
+
+ Set<String> projectArtifactIds = new HashSet<>();
+ for (Artifact artifact : context.getProject().getArtifacts()) {
+ if (!artifact.isOptional() && "jar".equals(artifact.getType())) {
+ projectArtifactIds.add(artifact.getArtifactId());
+ }
+ }
+
+ if (projectArtifactIds.isEmpty()) {
+ return;
+ }
+
+ File[] overlayJars = libDir.listFiles((dir, name) ->
name.endsWith(".jar"));
+ if (overlayJars == null) {
+ return;
+ }
+
+ for (File overlayJar : overlayJars) {
+ String jarName = overlayJar.getName();
+ String artifactId = extractArtifactId(jarName);
+ if (artifactId != null && projectArtifactIds.contains(artifactId))
{
+ context.getLog()
+ .debug("Removing dependency [" + jarName + "] from
overlay [" + overlay.getId()
+ + "]; managed version in project already
provides it");
+ overlayJar.delete();
+ }
+ }
+ }
+
+ /**
+ * Extracts the Maven artifactId from a jar filename following the
+ * {@code artifactId-version(-classifier)?.jar} convention.
+ *
+ * @param jarName the jar filename
+ * @return the artifactId, or null if it cannot be determined
+ */
+ private static String extractArtifactId(String jarName) {
+ if (jarName == null || !jarName.endsWith(".jar")) {
+ return null;
+ }
+ String baseName = jarName.substring(0, jarName.length() - 4);
+ int versionStart = -1;
+ for (int i = 0; i < baseName.length(); i++) {
+ if (Character.isDigit(baseName.charAt(i))) {
+ versionStart = i;
+ break;
+ }
+ }
+ if (versionStart > 0 && baseName.charAt(versionStart - 1) == '-') {
+ return baseName.substring(0, versionStart - 1);
+ }
+ return null;
+ }
Review Comment:
extractArtifactId() assumes the first digit in the filename starts the
version. This fails for common artifactIds containing digits (e.g.,
log4j-api-2.24.0.jar) and will silently skip filtering those conflicts.
--
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]
