You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2026/07/28 17:32:58 UTC
[PR] MEAR-305: Fix skipClassPathModification setting Class-Path empty [maven-ear-plugin]
elharo opened a new pull request, #539:
URL: https://github.com/apache/maven-ear-plugin/pull/539
When skipClassPathModification is true, the plugin should leave module manifests completely untouched. Previously, existing Class-Path entries were still updated/removed and the modified manifest was written back, potentially setting Class-Path to empty. Now all Class-Path modification logic is wrapped in a check for skipClassPathModification.
Fixes #180
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MEAR-305] Fix skipClassPathModification setting Class-Path empty [maven-ear-plugin]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo closed pull request #539: [MEAR-305] Fix skipClassPathModification setting Class-Path empty
URL: https://github.com/apache/maven-ear-plugin/pull/539
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MEAR-305] Fix skipClassPathModification setting Class-Path empty [maven-ear-plugin]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #539:
URL: https://github.com/apache/maven-ear-plugin/pull/539#discussion_r3667973016
##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -776,34 +776,34 @@ private void changeManifestClasspath(
}
// Modify the classpath entries in the manifest
- final boolean forceClassPathModification =
- javaEEVersion.lt(JavaEEVersion.FIVE) || defaultLibBundleDir == null;
- final boolean classPathExtension = !skipClassPathModification || forceClassPathModification;
- for (EarModule otherModule : getModules()) {
- if (module.equals(otherModule)) {
- continue;
- }
- final int moduleClassPathIndex = findModuleInClassPathElements(classPathElements, otherModule);
- if (moduleClassPathIndex != -1) {
- if (otherModule.isClassPathItem()) {
- classPathElements.set(moduleClassPathIndex, otherModule.getUri());
- } else {
- classPathElements.remove(moduleClassPathIndex);
+ if (!skipClassPathModification) {
+ final boolean forceClassPathModification =
+ javaEEVersion.lt(JavaEEVersion.FIVE) || defaultLibBundleDir == null;
+ final boolean classPathExtension = !skipClassPathModification || forceClassPathModification;
+ for (EarModule otherModule : getModules()) {
+ if (module.equals(otherModule)) {
+ continue;
+ }
+ final int moduleClassPathIndex = findModuleInClassPathElements(classPathElements, otherModule);
+ if (moduleClassPathIndex != -1) {
+ if (otherModule.isClassPathItem()) {
+ classPathElements.set(moduleClassPathIndex, otherModule.getUri());
+ } else {
+ classPathElements.remove(moduleClassPathIndex);
+ }
+ } else if (otherModule.isClassPathItem() && classPathExtension) {
+ classPathElements.add(otherModule.getUri());
}
- } else if (otherModule.isClassPathItem() && classPathExtension) {
- classPathElements.add(otherModule.getUri());
}
- }
- // Remove provided modules from classpath
- for (EarModule otherModule : getProvidedEarModules()) {
- final int moduleClassPathIndex = findModuleInClassPathElements(classPathElements, otherModule);
- if (moduleClassPathIndex != -1) {
- classPathElements.remove(moduleClassPathIndex);
+ // Remove provided modules from classpath
+ for (EarModule otherModule : getProvidedEarModules()) {
+ final int moduleClassPathIndex = findModuleInClassPathElements(classPathElements, otherModule);
+ if (moduleClassPathIndex != -1) {
+ classPathElements.remove(moduleClassPathIndex);
+ }
}
- }
- if (!skipClassPathModification || !classPathElements.isEmpty() || classPathExists) {
classPath.setValue(StringUtils.join(classPathElements.iterator(), " "));
mf.getMainSection().addConfiguredAttribute(classPath);
Review Comment:
The `Class-Path` attribute is still being written back unconditionally (lines 807-808), even when `skipClassPathModification` is true. This can still rewrite the manifest (and can still set `Class-Path` to an empty string when `classPathElements` is empty), contradicting the PR goal of leaving manifests untouched. Suggested fix: only set/add the `Class-Path` attribute when modification is allowed (e.g., move lines 807-808 inside the `if (!skipClassPathModification)` block, or restore a guard equivalent to the previous `if (!skipClassPathModification || !classPathElements.isEmpty() || classPathExists)` so you don’t add an empty attribute).
##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -776,34 +776,34 @@ private void changeManifestClasspath(
}
// Modify the classpath entries in the manifest
- final boolean forceClassPathModification =
- javaEEVersion.lt(JavaEEVersion.FIVE) || defaultLibBundleDir == null;
- final boolean classPathExtension = !skipClassPathModification || forceClassPathModification;
- for (EarModule otherModule : getModules()) {
- if (module.equals(otherModule)) {
- continue;
- }
- final int moduleClassPathIndex = findModuleInClassPathElements(classPathElements, otherModule);
- if (moduleClassPathIndex != -1) {
- if (otherModule.isClassPathItem()) {
- classPathElements.set(moduleClassPathIndex, otherModule.getUri());
- } else {
- classPathElements.remove(moduleClassPathIndex);
+ if (!skipClassPathModification) {
+ final boolean forceClassPathModification =
+ javaEEVersion.lt(JavaEEVersion.FIVE) || defaultLibBundleDir == null;
+ final boolean classPathExtension = !skipClassPathModification || forceClassPathModification;
Review Comment:
Inside `if (!skipClassPathModification)`, `classPathExtension = !skipClassPathModification || forceClassPathModification` will always evaluate to `true` because `!skipClassPathModification` is guaranteed. This makes `forceClassPathModification` effectively redundant here and obscures intent. Suggested fix: simplify the boolean(s) to reflect the actual decision (e.g., remove `classPathExtension` and the `&& classPathExtension` checks, or set `classPathExtension` based solely on what you still intend to gate within this 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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MEAR-305] Fix skipClassPathModification setting Class-Path empty [maven-ear-plugin]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #539:
URL: https://github.com/apache/maven-ear-plugin/pull/539#issuecomment-5117050549
Closed in favor of #541 which fixes the remaining issue with manifest persistence.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org