You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Pankraz76 (via GitHub)" <gi...@apache.org> on 2025/05/27 11:22:26 UTC
[PR] avoid falsy catch on `mkdirs()` [maven]
Pankraz76 opened a new pull request, #2396:
URL: https://github.com/apache/maven/pull/2396
- https://github.com/apache/maven/pull/2395
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2111951540
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
Review Comment:
yes will give dedicated PR for this.
https://docs.openrewrite.org/recipes/staticanalysis/missingoverrideannotation
another good example why we need to buckle up tooling.
https://pmd.sourceforge.io/pmd-6.3.0/pmd_rules_java_bestpractices.html#missingoverride
- https://github.com/apache/maven/pull/2364
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2111553285
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
Review Comment:
Not sure how old this code is, perhaps pre-Java 5. This and a lot of other methods are missing @Override annotations. I also see a lot fo old javadoc style annotations in these classes.
##########
its/core-it-support/core-it-plugins/maven-it-plugin-uses-wagon/src/main/java/org/apache/maven/plugin/coreit/LookupWagonMojo.java:
##########
@@ -93,8 +93,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
+ outputFile.getParentFile().mkdirs();
Review Comment:
This code is pretty old and not up to 2025 standards. I think we should merge the try with resources before making this change. Otherwise, line splitting is making it even less clear.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110059677
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-clean-plugin/src/main/java/org/apache/maven/plugin/coreit/CleanMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
```suggestion
```
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2396:
URL: https://github.com/apache/maven/pull/2396#issuecomment-2936965168
its simply false catch and hindering `try-with-resources` statement promoting bare overhead.
will give dedicated example. But yes it should be done in combination with `autoclosable` to have logical unit.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2396:
URL: https://github.com/apache/maven/pull/2396#issuecomment-2915543289
Caused by: org.apache.maven.plugin.MojoExecutionException: The following files had format violations:
src/main/java/org/apache/maven/plugin/coreit/CleanMojo.java
@@ -70,7 +70,6 @@
············outputFile·=·new·File(project.getBasedir(),·pathname).getAbsoluteFile();
········}
-
········outputFile.getParentFile().mkdirs();
········try·{
············outputFile.createNewFile();
Run 'mvn spotless:apply' to fix these violations.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2111951540
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
Review Comment:
yes will give dedicated PR for this.
https://docs.openrewrite.org/recipes/staticanalysis/missingoverrideannotation
another good example why we need to buckle up tooling.
https://pmd.sourceforge.io/pmd-6.3.0/pmd_rules_java_bestpractices.html#missingoverride
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110086980
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-deploy-plugin/src/main/java/org/apache/maven/plugin/coreit/DeployMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
will dedicate in PR to grasp this simple.
- https://github.com/apache/maven/pull/2395
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2109264385
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-compiler-plugin/src/main/java/org/apache/maven/plugin/coreit/TestCompileMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
delete
##########
its/core-it-support/core-it-plugins/maven-it-plugin-configuration/src/main/java/org/apache/maven/plugin/coreit/PropertiesUtil.java:
##########
@@ -70,8 +70,8 @@ public static Properties read(File inputFile) throws MojoExecutionException {
public static void write(File outputFile, Properties props) throws MojoExecutionException {
OutputStream os = null;
Review Comment:
This could use try with resources
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-clean-plugin/src/main/java/org/apache/maven/plugin/coreit/CleanMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
we should delete this line
##########
its/core-it-support/core-it-plugins/maven-it-plugin-class-loader/maven-it-plugin-class-loader/src/main/java/org/apache/maven/plugin/coreit/PropertiesUtil.java:
##########
@@ -36,8 +36,8 @@ class PropertiesUtil {
public static void write(File outputFile, Properties props) throws MojoExecutionException {
OutputStream out = null;
Review Comment:
This could use try with resources
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-compiler-plugin/src/main/java/org/apache/maven/plugin/coreit/CompileMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
delete
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckMojo.java:
##########
@@ -118,8 +118,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
Review Comment:
This could use try with resources
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-deploy-plugin/src/main/java/org/apache/maven/plugin/coreit/DeployMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
This could use try with resources
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
+ outputFile.getParentFile().mkdirs();
Review Comment:
check return value?
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -134,8 +134,8 @@ public void run() {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
Review Comment:
This could use try with resources
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2109728748
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
+ outputFile.getParentFile().mkdirs();
Review Comment:
The code should look like this, but `mkdir` failing is very rare and not part of the happy path. Since this step is used for the subsequent try-with-resources block, it would naturally result in the same `IOError` anyway—without requiring an explicit check.
This is the logical approach, though it wasn’t handled before either.
Currently, there’s no explicit code handling this state. We *could* add preemptive checks, of course. Writing the missing `if` statements—which is what I’d normally expect—would change the code slightly more, as it introduces validation for one new previously unchecked outcome.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110128993
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
+ outputFile.getParentFile().mkdirs();
Review Comment:
dedicated concern/PR. lets check up afterward.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet closed pull request #2396: avoid false `IO` catch on `mkdirs()`
URL: https://github.com/apache/maven/pull/2396
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110061380
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-compiler-plugin/src/main/java/org/apache/maven/plugin/coreit/TestCompileMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
```suggestion
```
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2111960785
##########
its/core-it-support/core-it-plugins/maven-it-plugin-uses-wagon/src/main/java/org/apache/maven/plugin/coreit/LookupWagonMojo.java:
##########
@@ -93,8 +93,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
+ outputFile.getParentFile().mkdirs();
Review Comment:
yes we can do the try with but then do it twice as there are only 4 ATM and with this change its a lot more, thats why i see this as enable.
Lets make whats possible, doing step by step to reduce overhead.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2396:
URL: https://github.com/apache/maven/pull/2396#issuecomment-2937016628
> I see no benefit in this change.
indeed, not all, as its a mix of:
- https://github.com/apache/maven/pull/2426
- https://github.com/apache/maven/pull/2395
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2396:
URL: https://github.com/apache/maven/pull/2396#issuecomment-2912155464
is this any good?
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2109728748
##########
its/core-it-suite/src/test/resources/mng-2135/plugin/src/main/java/coreit/ItMojo.java:
##########
@@ -52,8 +52,8 @@ public class ItMojo extends AbstractMojo {
private File outputFile;
public void execute() throws MojoExecutionException {
+ outputFile.getParentFile().mkdirs();
Review Comment:
The code should look like this, but `mkdir` failing is very rare and not part of the happy path. Since this step is used for the subsequent try-with-resources block, it would naturally result in the same `IOError` anyway—without requiring an explicit check—while also preventing an attempt to read a resource that couldn’t be created in the first place. This is the logical approach, though it wasn’t handled before either.
Currently, there’s no explicit code handling this state. We *could* add preemptive checks, of course. Writing the missing `if` statements—which is what I’d normally expect—would change the code slightly more, as it introduces validation for three previously unchecked outcomes.
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2111975276
##########
its/core-it-support/core-it-plugins/maven-it-plugin-uses-wagon/src/main/java/org/apache/maven/plugin/coreit/LookupWagonMojo.java:
##########
@@ -93,8 +93,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
+ outputFile.getParentFile().mkdirs();
Review Comment:
<img width="1135" alt="image" src="https://github.com/user-attachments/assets/b701d181-1b6c-4d50-8221-ae595df79e6d" />
- https://github.com/apache/maven/pull/2401
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110061815
##########
its/core-it-support/core-it-plugins/maven-it-plugin-core-stubs/maven-compiler-plugin/src/main/java/org/apache/maven/plugin/coreit/CompileMojo.java:
##########
@@ -72,8 +72,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file: " + outputFile);
Review Comment:
```suggestion
```
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110131191
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckMojo.java:
##########
@@ -118,8 +118,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
Review Comment:
will dedicate in
- https://github.com/apache/maven/pull/2395
##########
its/core-it-support/core-it-plugins/maven-it-plugin-active-collection/src/main/java/org/apache/maven/plugin/coreit/CheckThreadSafetyMojo.java:
##########
@@ -134,8 +134,8 @@ public void run() {
getLog().info("[MAVEN-CORE-IT-LOG] Creating output file " + outputFile);
OutputStream out = null;
Review Comment:
will dedicate in
- https://github.com/apache/maven/pull/2395
--
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] avoid false `IO` catch on `mkdirs()` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2396:
URL: https://github.com/apache/maven/pull/2396#discussion_r2110131765
##########
its/core-it-support/core-it-plugins/maven-it-plugin-class-loader/maven-it-plugin-class-loader/src/main/java/org/apache/maven/plugin/coreit/PropertiesUtil.java:
##########
@@ -36,8 +36,8 @@ class PropertiesUtil {
public static void write(File outputFile, Properties props) throws MojoExecutionException {
OutputStream out = null;
Review Comment:
will dedicate in
- https://github.com/apache/maven/pull/2395
##########
its/core-it-support/core-it-plugins/maven-it-plugin-configuration/src/main/java/org/apache/maven/plugin/coreit/PropertiesUtil.java:
##########
@@ -70,8 +70,8 @@ public static Properties read(File inputFile) throws MojoExecutionException {
public static void write(File outputFile, Properties props) throws MojoExecutionException {
OutputStream os = null;
Review Comment:
will dedicate in
- https://github.com/apache/maven/pull/2395
--
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