[MCLEAN-124] Provide more accurate reason when can not delete a resource

[jira-importer]

Slawomir Jaranowski opened MCLEAN-124 and commented

Currently we have only in stack trace something like:

Caused by: java.io.IOException: Failed to delete /Users/runner/work/maven-surefire/maven-surefire/surefire-its/target
    at org.apache.maven.plugins.clean.Cleaner.delete (Cleaner.java:300)
    at org.apache.maven.plugins.clean.Cleaner.delete (Cleaner.java:250)
    at org.apache.maven.plugins.clean.Cleaner.delete (Cleaner.java:124)

It will be usable if we have reported a reason ... why can not be deleted

---

Issue Links:

• SUREFIRE-2269 Error during clean project after build GitHub - Mac OS

Remote Links:

• GitHub Pull Request #60
  
• GitHub Pull Request #64
  
• GitHub Pull Request #84
  

Backported to: next-release-3.x

[jira-importer]

Michael Osipov commented

What do you if Java does not provide more information?

[jira-importer]

Slawomir Jaranowski commented

What do you if Java does not provide more information?

We need to try ... I see that we use exception.addSuppressed(e); but in stactrace is not printed ...

[jira-importer]

Michael Osipov commented

This is a difference, of course!

[jira-importer]

Peter De Maeyer commented

The stack trace is coming from Cleaner.delete:300, which is Maven code, not Java (JDK) code. There is no Java exception there. There are multiple complications there: on Windows or not, retries with Thread.sleep{^}(1)^ and back-off delays.

This issue is missing a concrete situation illustrating an actual lack of information. Java's own File.delete API, which is used underneath, doesn't give more accurate information either besides true "delete successful" or false "delete unsuccessful". We could migrate to Java 7's improved Files.delete API, which instead throws exceptions with more accurate descriptions, but I would not do anything more beyond that.

^(1)^ Thread.sleep is almost always a bad idea. Could that maybe explain MCLEAN-102? In this case in particular, it looks like a random hack - what problem where they even trying to fix? But anyway, that is out of scope here.

[jira-importer]

Peter De Maeyer commented

I created a PR for this, leveraging Java 7's new Files + Path API over the old File API.
I also added a JUnit test covering the new code (changed lines) for 100%.

Future improvements, out of scope of this issue:

• For this issue, I limited the use of the Files + Path API to this very specific bit of code. It would be an improvement to migrate all the code away from the legacy File API to the new Files + Path API. Not just for the Clean plugin, but for all Maven plugins.
• Re-evaluate the need for the retries and especially the Thread.sleep in the Cleaner class, and solve the problem (if any) differently. Expecting that those who introduced it would have written a test to illustrate the use case, I temporarily removed the Thread.sleep to see which test would fail, which would give me an indication of why the code needs to be there, but all tests just passed. So I'm still clueless why that hack was introduced in the first place.

[jira-importer]

Peter De Maeyer commented

I think the fix version should be next-release-3.x, given that it was merged to the maven-clean-plugin-3.x branch.

[jira-importer]

Slawomir Jaranowski commented

Peter De Maeyer - thanks, fixed version as 3.x

Do you have a plan to do the same for master branch 4.x?

[jira-importer]

Peter De Maeyer commented

sjaranowski, no, I did not have such a plan, because I assumed that the 3.x branch would be periodically merged into the master 4.x branch by the code owners. Is that not the case? Do I need to go through yet another PR to get it merged to the master 4.x branch?

[jira-importer]

Peter De Maeyer commented

By the way, I notice a lot of clutter in comments from the ASF GitHub Bot. Consider switching that off. There is already a link to the PR in GitHub, that should cover it, no?