You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "desruisseaux (via GitHub)" <gi...@apache.org> on 2025/10/12 10:02:14 UTC
[PR] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
desruisseaux opened a new pull request, #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481
This pull request combines 3 tasks in 3 separated commits:
* Upgrade from Maven 4.0.0-rc-2 to 4.0.0-rc-4.
* Remove the `useDefaultManifestFile` configuration parameter, which was deprecated since Maven 3.0.0.
* Edition in Javadoc (mostly formatting) and minor code simplifications.
The changes in this pull request should not cause significant change in the plugin behaviour, except for the removal of the `useDefaultManifestFile` option and when a null argument is given to the `AbstractJarMojo.getJarFile(Path, String, String)` method. In the latter case, the `IllegalArgumentException` has been replaced by `NullPointerException`.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3412314228
We have an approval and I saw no objection. Should we process with the merge?
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3536334885
> This PR seems to have broken the CI, and should not have been merged yet.
At the time that this pull request was submitted, the GitHub checks were successful. The failure that we observe in #503 came later. Strangely, Jenkins also fails to build the master branch of maven-clean-plugin and maven-compiler-plugins for the same reason, but those failures do not appear in GitHub actions (contrarily to this maven-jar-plugin).
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2425506631
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -291,27 +282,23 @@ public Path createArchive() throws MojoException {
/**
* Generates the JAR.
- * @throws MojoException in case of an error.
+ *
+ * @throws MojoException in case of an error
*/
@Override
public void execute() throws MojoException {
- if (useDefaultManifestFile) {
- throw new MojoException("You are using 'useDefaultManifestFile' which has been removed"
- + " from the maven-jar-plugin. "
- + "Please see the link >>Using Your Own Manifest File<< on the plugin site.");
- }
-
if (skipIfEmpty && isEmpty(getClassesDirectory())) {
- getLog().info("Skipping packaging of the " + getType());
+ getLog().info("Skipping packaging of the " + getType() + '.');
Review Comment:
Done
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3536357380
no way - merging PR if build is failing?
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "Bukama (via GitHub)" <gi...@apache.org>.
Bukama commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2424427969
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -158,76 +145,79 @@ public abstract class AbstractJarMojo implements org.apache.maven.api.plugin.Moj
private String outputTimestamp;
/**
+ * Whether to detect multi-release JAR files.
* If the JAR contains the {@code META-INF/versions} directory it will be detected as a multi-release JAR file
- * ("MRJAR"), adding the {@code Multi-Release: true} attribute to the main section of the JAR MANIFEST.MF.
+ * ("MRJAR"), adding the {@code Multi-Release: true} attribute to the main section of the JAR {@code MANIFEST.MF}.
*
* @since 3.4.0
*/
@Parameter(property = "maven.jar.detectMultiReleaseJar", defaultValue = "true")
private boolean detectMultiReleaseJar;
/**
- * The mojo logger
+ * The <abbr>MOJO</abbr> logger.
*/
@Inject
private Log log;
/**
- * Return the specific output directory to serve as the root for the archive.
- * @return get classes directory.
+ * Creates a new <abbr>MOJO</abbr>.
+ */
+ protected AbstractJarMojo() {}
+
+ /**
+ * {@return the specific output directory to serve as the root for the archive}.
*/
protected abstract Path getClassesDirectory();
/**
- * Return the {@link #project MavenProject}
+ * Return the {@linkplain #project Maven project}.
*
- * @return the MavenProject.
+ * @return the Maven project
*/
protected final Project getProject() {
return project;
}
+ /**
+ * {@return the <abbr>MOJO</abbr> logger}.
Review Comment:
Inline-Return <3
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "Bukama (via GitHub)" <gi...@apache.org>.
Bukama commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2424446279
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -291,27 +282,23 @@ public Path createArchive() throws MojoException {
/**
* Generates the JAR.
- * @throws MojoException in case of an error.
+ *
+ * @throws MojoException in case of an error
*/
@Override
public void execute() throws MojoException {
- if (useDefaultManifestFile) {
- throw new MojoException("You are using 'useDefaultManifestFile' which has been removed"
- + " from the maven-jar-plugin. "
- + "Please see the link >>Using Your Own Manifest File<< on the plugin site.");
- }
-
if (skipIfEmpty && isEmpty(getClassesDirectory())) {
- getLog().info("Skipping packaging of the " + getType());
+ getLog().info("Skipping packaging of the " + getType() + '.');
Review Comment:
I often use `String.format´ for such situations, to have the full log message in one place and pass the placeholders later to it.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3414453757
@desruisseaux Please assign appropriate label to PR according to the type of change.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux merged PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2438364831
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -344,10 +331,11 @@ private boolean projectHasAlreadySetAnArtifact() {
/**
* Return {@code true} in case where the classifier is not {@code null} and contains something else than white spaces.
*
- * @return {@code true} if the classifier is set.
+ * @param classifier the classifier to verify
+ * @return {@code true} if the classifier is set
*/
- protected boolean hasClassifier() {
- return getClassifier() != null && !getClassifier().trim().isEmpty();
+ private static boolean hasClassifier(String classifier) {
+ return classifier != null && !classifier.isBlank();
Review Comment:
Keep isEmpty, as blank classifiers are not rejected by maven yet.
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -344,10 +331,11 @@ private boolean projectHasAlreadySetAnArtifact() {
/**
* Return {@code true} in case where the classifier is not {@code null} and contains something else than white spaces.
*
- * @return {@code true} if the classifier is set.
+ * @param classifier the classifier to verify
+ * @return {@code true} if the classifier is set
*/
- protected boolean hasClassifier() {
- return getClassifier() != null && !getClassifier().trim().isEmpty();
+ private static boolean hasClassifier(String classifier) {
+ return classifier != null && !classifier.isBlank();
Review Comment:
Mmh, I missed the trim.
So fine...
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "Bukama (via GitHub)" <gi...@apache.org>.
Bukama commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2424427969
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -158,76 +145,79 @@ public abstract class AbstractJarMojo implements org.apache.maven.api.plugin.Moj
private String outputTimestamp;
/**
+ * Whether to detect multi-release JAR files.
* If the JAR contains the {@code META-INF/versions} directory it will be detected as a multi-release JAR file
- * ("MRJAR"), adding the {@code Multi-Release: true} attribute to the main section of the JAR MANIFEST.MF.
+ * ("MRJAR"), adding the {@code Multi-Release: true} attribute to the main section of the JAR {@code MANIFEST.MF}.
*
* @since 3.4.0
*/
@Parameter(property = "maven.jar.detectMultiReleaseJar", defaultValue = "true")
private boolean detectMultiReleaseJar;
/**
- * The mojo logger
+ * The <abbr>MOJO</abbr> logger.
*/
@Inject
private Log log;
/**
- * Return the specific output directory to serve as the root for the archive.
- * @return get classes directory.
+ * Creates a new <abbr>MOJO</abbr>.
+ */
+ protected AbstractJarMojo() {}
+
+ /**
+ * {@return the specific output directory to serve as the root for the archive}.
*/
protected abstract Path getClassesDirectory();
/**
- * Return the {@link #project MavenProject}
+ * Return the {@linkplain #project Maven project}.
*
- * @return the MavenProject.
+ * @return the Maven project
*/
protected final Project getProject() {
return project;
}
+ /**
+ * {@return the <abbr>MOJO</abbr> logger}.
Review Comment:
Inline-Return <3
But you don't need a dot at the end. It is added automatically when using inline `{@return}`.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3536391935
So can we turn on Jenkins for PRs from forks too? If not, maybe we can just move everything to GitHub actions and disable Jenkins completely? Whether you use a fork or a branch, shouldn't change which checks are run.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3536406823
> no way - merging PR if build is failing?
The build was not failing when this PR was merged, at least not on GitHub actions. It was failing on Jenkins, but I was not aware of that. Looking at [the Jenkins page](https://ci-maven.apache.org/job/Maven/job/maven-box/), Jenkins builds were failing for the master branches of many projects for more than 4 months, and I don't know if anyone paid attention.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3536361820
Jenkins is used only for branches from repo ... for PR from forks is not used ... so PR can pass without jenkins build
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3523617899
This PR seems to have broken the CI, and should not have been merged yet.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#issuecomment-3508564718
On jenkins we have Maven 4-rc-3 ....
https://cwiki.apache.org/confluence/display/INFRA/Maven+Installation+Matrix
We should ask infra for update .... or disable jenkis build ...
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2425499707
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -158,76 +145,79 @@ public abstract class AbstractJarMojo implements org.apache.maven.api.plugin.Moj
private String outputTimestamp;
/**
+ * Whether to detect multi-release JAR files.
* If the JAR contains the {@code META-INF/versions} directory it will be detected as a multi-release JAR file
- * ("MRJAR"), adding the {@code Multi-Release: true} attribute to the main section of the JAR MANIFEST.MF.
+ * ("MRJAR"), adding the {@code Multi-Release: true} attribute to the main section of the JAR {@code MANIFEST.MF}.
*
* @since 3.4.0
*/
@Parameter(property = "maven.jar.detectMultiReleaseJar", defaultValue = "true")
private boolean detectMultiReleaseJar;
/**
- * The mojo logger
+ * The <abbr>MOJO</abbr> logger.
*/
@Inject
private Log log;
/**
- * Return the specific output directory to serve as the root for the archive.
- * @return get classes directory.
+ * Creates a new <abbr>MOJO</abbr>.
+ */
+ protected AbstractJarMojo() {}
+
+ /**
+ * {@return the specific output directory to serve as the root for the archive}.
*/
protected abstract Path getClassesDirectory();
/**
- * Return the {@link #project MavenProject}
+ * Return the {@linkplain #project Maven project}.
*
- * @return the MavenProject.
+ * @return the Maven project
*/
protected final Project getProject() {
return project;
}
+ /**
+ * {@return the <abbr>MOJO</abbr> logger}.
Review Comment:
Just tested and I confirm. With an explicit dot after `}`, the `javadoc` tools generates a sentence ending with 2 dots. I will need to pass over the other plugins since I have used the `{@return …}.` form in many places.
--
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] Upgrade to Maven 4.0.0-rc-4 and editions [maven-jar-plugin]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #481:
URL: https://github.com/apache/maven-jar-plugin/pull/481#discussion_r2438745550
##########
src/main/java/org/apache/maven/plugins/jar/AbstractJarMojo.java:
##########
@@ -344,10 +331,11 @@ private boolean projectHasAlreadySetAnArtifact() {
/**
* Return {@code true} in case where the classifier is not {@code null} and contains something else than white spaces.
*
- * @return {@code true} if the classifier is set.
+ * @param classifier the classifier to verify
+ * @return {@code true} if the classifier is set
*/
- protected boolean hasClassifier() {
- return getClassifier() != null && !getClassifier().trim().isEmpty();
+ private static boolean hasClassifier(String classifier) {
+ return classifier != null && !classifier.isBlank();
Review Comment:
I wonder if we should use `isEmpty()` anyway, for consistency with the rest of Maven?
--
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