You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "efegokdemir (via GitHub)" <gi...@apache.org> on 2026/09/23 17:37:33 UTC

[PR] fix: report outdated resource deletion failures [maven-ear-plugin]

efegokdemir opened a new pull request, #560:
URL: https://github.com/apache/maven-ear-plugin/pull/560

   Fixes #517
   
   ## Summary
   
   `EarMojo` now reports a warning when an outdated work-directory resource cannot be deleted, instead of silently continuing with stale output.
   
   ## Changes
   
   - Check the return value of `File.delete()` in `deleteOutdatedResources`.
   - Log the affected resource when deletion fails.
   
   ## Testing
   
   - `JAVA_HOME=/opt/homebrew/Cellar/openjdk/27/libexec/openjdk.jdk/Contents/Home mvn -DskipITs -Dspotless.check.skip=true -Dspotless.apply.skip=true verify` — passed; 48 tests passed.
   - `git diff --check` — passed.
   - Checkstyle and RAT passed; four existing Checkstyle findings were ignored by the repository configuration.
   - Integration tests were not run because `-DskipITs` was used for the focused local validation.
   
   ## Notes
   
   The change is intentionally limited to observability of deletion failures; it does not change the cleanup policy or mask the failure. AI assistance was used in preparing this change; I reviewed the modified code and test results.
   
   - [x] I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004.


-- 
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] fix: report outdated resource deletion failures [maven-ear-plugin]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #560:
URL: https://github.com/apache/maven-ear-plugin/pull/560#discussion_r4360250438


##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -886,7 +886,9 @@ private void deleteOutdatedResources(final Collection<String> outdatedResources)
             if (resourceFile.lastModified() < startTime) {
                 getLog().debug("deleting outdated resource " + outdatedResource);
                 getLog().debug(outdatedResource + " last modified: " + resourceFile.lastModified());
-                resourceFile.delete();
+                if (!resourceFile.delete()) {
+                    getLog().warn("Failed to delete outdated resource: " + resourceFile);
+                }
             }

Review Comment:
   `File.lastModified()` returns `0L` when the file does not exist, which will almost always be `< startTime`. That can cause a warning for resources that are already missing (delete returns false), introducing noisy/incorrect warnings. Consider gating the deletion path on `resourceFile.exists()` (before the last-modified comparison and/or before warning), or only emitting the warning when the file still exists after the delete attempt.



##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -886,7 +886,9 @@ private void deleteOutdatedResources(final Collection<String> outdatedResources)
             if (resourceFile.lastModified() < startTime) {
                 getLog().debug("deleting outdated resource " + outdatedResource);
                 getLog().debug(outdatedResource + " last modified: " + resourceFile.lastModified());
-                resourceFile.delete();
+                if (!resourceFile.delete()) {
+                    getLog().warn("Failed to delete outdated resource: " + resourceFile);

Review Comment:
   `File.delete()` provides no failure details, and logging `resourceFile` may be a relative path, which can make the warning hard to act on. Consider logging `outdatedResource` and/or `resourceFile.getAbsolutePath()`; if you want actionable diagnostics, prefer `java.nio.file.Files.delete(...)` / `deleteIfExists(...)` so you can include the `IOException` message in the warning.



-- 
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] fix: report outdated resource deletion failures [maven-ear-plugin]

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #560:
URL: https://github.com/apache/maven-ear-plugin/pull/560#discussion_r4360109933


##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -886,7 +886,9 @@ private void deleteOutdatedResources(final Collection<String> outdatedResources)
             if (resourceFile.lastModified() < startTime) {
                 getLog().debug("deleting outdated resource " + outdatedResource);
                 getLog().debug(outdatedResource + " last modified: " + resourceFile.lastModified());
-                resourceFile.delete();
+                if (!resourceFile.delete()) {

Review Comment:
   How bad a problem is this? What's the impact of not deleting? I'm wondering if this justifies throwing an exception instead? 



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