gnodet commented on code in PR #328:
URL:
https://github.com/apache/maven-clean-plugin/pull/328#discussion_r4083025107
##########
src/main/java/org/apache/maven/plugins/clean/BackgroundCleaner.java:
##########
@@ -162,24 +235,22 @@ String fastDeleteError(IOException e) {
/**
* Deletes the specified directory and its contents in a background thread.
+ * This method is synchronized to support concurrent calls from parallel
module builds.
*
- * @param basedir the directory to delete, must not be {@code null}
+ * @param baseDir the directory to delete, must not be {@code null}
+ * @param force whether to force the deletion of read-only files
+ * @param retryOnError whether to undertake a batch retry of failed
deletions
* @return whether this method was able to register the background task
* @throws IOException if an error occurred while preparing the task
before execution in a background thread
*/
- @Override
- boolean fastDelete(Path baseDir) throws IOException {
+ synchronized boolean fastDelete(Path baseDir, boolean force, boolean
retryOnError) throws IOException {
Review Comment:
Same situation as `fastDeleteError`: the method is called by
`Cleaner.fastDelete(Path)` (a different class in the same package), so it
cannot be made private. The `Cleaner` wrapper is itself private, keeping the
method effectively invisible outside the package.
##########
src/main/java/org/apache/maven/plugins/clean/Cleaner.java:
##########
@@ -307,20 +328,28 @@ private boolean isClearAll() {
/**
* Deletes the specified directory and its contents in a background thread.
- * The default implementation returns {@code false}.
+ * If a {@link BackgroundCleaner} has been {@linkplain
#setBackgroundCleaner set},
+ * delegates to it with this cleaner's {@link #force} and {@link
#retryOnError} values.
+ * Otherwise returns {@code false}.
*
* @param basedir the directory to delete, must not be {@code null}
* @return whether this method was able to register the background task
* @throws IOException if an error occurred while preparing the task
before execution in a background thread
*/
boolean fastDelete(Path baseDir) throws IOException {
Review Comment:
Fixed in 91f53ca: `fastDelete(Path)` is now `private`.
##########
src/main/java/org/apache/maven/plugins/clean/Cleaner.java:
##########
@@ -307,20 +328,28 @@ private boolean isClearAll() {
/**
* Deletes the specified directory and its contents in a background thread.
- * The default implementation returns {@code false}.
+ * If a {@link BackgroundCleaner} has been {@linkplain
#setBackgroundCleaner set},
+ * delegates to it with this cleaner's {@link #force} and {@link
#retryOnError} values.
+ * Otherwise returns {@code false}.
*
* @param basedir the directory to delete, must not be {@code null}
* @return whether this method was able to register the background task
* @throws IOException if an error occurred while preparing the task
before execution in a background thread
*/
boolean fastDelete(Path baseDir) throws IOException {
+ if (backgroundCleaner != null) {
+ return backgroundCleaner.fastDelete(baseDir, force, retryOnError);
+ }
return false;
}
/**
* Returns an error message to show to user if the fast delete failed.
*/
String fastDeleteError(IOException e) {
Review Comment:
Fixed in 91f53ca: `fastDeleteError(IOException)` is now `private`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
