From 42f42743bf9c1edfd99f1badf1c488953940f396 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 16:16:25 +0100 Subject: [PATCH 01/24] Add a fast clean option --- .../apache/maven/plugins/clean/CleanMojo.java | 8 +- .../apache/maven/plugins/clean/Cleaner.java | 154 +++++++++++++++++- 2 files changed, 160 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 0561ad75..ea1b38d8 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -161,6 +161,12 @@ public class CleanMojo @Parameter( property = "maven.clean.excludeDefaultDirectories", defaultValue = "false" ) private boolean excludeDefaultDirectories; + @Parameter( property = "maven.clean.fast", defaultValue = "false" ) + private boolean fast; + + @Parameter( property = "maven.clean.fastFolder", defaultValue = "${maven.multiModuleProjectDirectory}/.mvn/clean" ) + private File fastFolder; + /** * Deletes file-sets in the following project build directory order: (source) directory, output directory, test * directory, report directory, and then the additional file-sets. @@ -177,7 +183,7 @@ public void execute() return; } - Cleaner cleaner = new Cleaner( getLog(), isVerbose() ); + Cleaner cleaner = new Cleaner( getLog(), isVerbose(), fast ? fastFolder : null ); try { diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index a15a221b..f1c0d58c 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -21,6 +21,11 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.concurrent.ThreadLocalRandom; import org.apache.maven.plugin.logging.Log; import org.apache.maven.shared.utils.Os; @@ -44,13 +49,15 @@ class Cleaner private final Logger logWarn; + private final File fastFolder; + /** * Creates a new cleaner. * * @param log The logger to use, may be null to disable logging. * @param verbose Whether to perform verbose logging. */ - Cleaner( final Log log, boolean verbose ) + Cleaner( final Log log, boolean verbose, File fastFolder ) { logDebug = ( log == null || !log.isDebugEnabled() ) ? null : new Logger() { @@ -77,6 +84,8 @@ public void log( CharSequence message ) }; logVerbose = verbose ? logInfo : logDebug; + + this.fastFolder = fastFolder; } /** @@ -115,9 +124,63 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo File file = followSymlinks ? basedir : basedir.getCanonicalFile(); + if ( selector == null && !followSymlinks && fastFolder != null ) + { + if ( fastDelete( file ) ) + { + return; + } + } + delete( file, "", selector, followSymlinks, failOnError, retryOnError ); } + private boolean fastDelete( File baseDir ) + { + fastFolder.mkdirs(); + if ( fastFolder.isDirectory() ) + { + try + { + File tmpDir = createTempDir( fastFolder ); + File dstDir = new File( tmpDir, baseDir.getName() ); + Files.move( baseDir.toPath(), dstDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); + BackgroundCleaner.delete( fastFolder, tmpDir ); + return true; + } + catch ( IOException e ) + { + if ( logDebug != null ) + { + logDebug.log( "Unable to fast delete directory: " + e ); + } + } + } + else + { + if ( logDebug != null ) + { + logDebug.log( "Unable to fast delete directory as the path " + + fastFolder + " does not point to a directory" ); + } + } + return false; + } + + private File createTempDir( File baseFolder ) throws IOException + { + for ( int i = 0; i < 10; i++ ) + { + int rnd = ThreadLocalRandom.current().nextInt(); + File tmpDir = new File( baseFolder, Integer.toHexString( rnd ) ); + if ( tmpDir.mkdir() ) + { + return tmpDir; + } + } + throw new IOException( "Can not create temp folder" ); + } + /** * Deletes the specified file or directory. * @@ -286,4 +349,93 @@ private interface Logger } + static class BackgroundCleaner extends Thread + { + + private static BackgroundCleaner instance; + + private final Deque filesToDelete = new ArrayDeque<>(); + + private final Cleaner cleaner = new Cleaner( null, false, null ); + + private int status = 0; + + public static void delete( File fastFolder, File folder ) + { + synchronized ( BackgroundCleaner.class ) + { + if ( instance == null || !instance.doDelete( folder ) ) + { + instance = new BackgroundCleaner( fastFolder, folder ); + } + } + } + + private BackgroundCleaner( File fastFolder, File folder ) + { + init( fastFolder, folder ); + } + + public void run() + { + while ( true ) + { + File basedir = pollNext(); + if ( basedir == null ) + { + break; + } + try + { + cleaner.delete( basedir, "", null, false, false, true ); + } + catch ( IOException e ) + { + // do not display errors + } + } + } + + synchronized void init( File fastFolder, File folder ) + { + if ( fastFolder.isDirectory() ) + { + File[] children = fastFolder.listFiles(); + if ( children != null && children.length > 0 ) + { + for ( File child : children ) + { + doDelete( child ); + } + } + } + doDelete( folder ); + } + + synchronized File pollNext() + { + File basedir = filesToDelete.poll(); + if ( basedir == null ) + { + status = 2; + } + return basedir; + } + + synchronized boolean doDelete( File folder ) + { + if ( status == 2 ) + { + return false; + } + filesToDelete.add( folder ); + if ( status == 0 ) + { + status = 1; + start(); + } + return true; + } + } + } From 3b3fdbed49a97849a8f9da6941bc54d5f5dc4308 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 16:55:29 +0100 Subject: [PATCH 02/24] folder -> dir --- .../apache/maven/plugins/clean/CleanMojo.java | 6 +-- .../apache/maven/plugins/clean/Cleaner.java | 46 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index ea1b38d8..cd60920f 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -164,8 +164,8 @@ public class CleanMojo @Parameter( property = "maven.clean.fast", defaultValue = "false" ) private boolean fast; - @Parameter( property = "maven.clean.fastFolder", defaultValue = "${maven.multiModuleProjectDirectory}/.mvn/clean" ) - private File fastFolder; + @Parameter( property = "maven.clean.fastDir", defaultValue = "${maven.multiModuleProjectDirectory}/.mvn/clean" ) + private File fastDir; /** * Deletes file-sets in the following project build directory order: (source) directory, output directory, test @@ -183,7 +183,7 @@ public void execute() return; } - Cleaner cleaner = new Cleaner( getLog(), isVerbose(), fast ? fastFolder : null ); + Cleaner cleaner = new Cleaner( getLog(), isVerbose(), fast ? fastDir : null ); try { diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index f1c0d58c..3f266223 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -49,7 +49,7 @@ class Cleaner private final Logger logWarn; - private final File fastFolder; + private final File fastDir; /** * Creates a new cleaner. @@ -57,7 +57,7 @@ class Cleaner * @param log The logger to use, may be null to disable logging. * @param verbose Whether to perform verbose logging. */ - Cleaner( final Log log, boolean verbose, File fastFolder ) + Cleaner( final Log log, boolean verbose, File fastDir ) { logDebug = ( log == null || !log.isDebugEnabled() ) ? null : new Logger() { @@ -85,7 +85,7 @@ public void log( CharSequence message ) logVerbose = verbose ? logInfo : logDebug; - this.fastFolder = fastFolder; + this.fastDir = fastDir; } /** @@ -124,7 +124,7 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo File file = followSymlinks ? basedir : basedir.getCanonicalFile(); - if ( selector == null && !followSymlinks && fastFolder != null ) + if ( selector == null && !followSymlinks && fastDir != null ) { if ( fastDelete( file ) ) { @@ -137,15 +137,15 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo private boolean fastDelete( File baseDir ) { - fastFolder.mkdirs(); - if ( fastFolder.isDirectory() ) + fastDir.mkdirs(); + if ( fastDir.isDirectory() ) { try { - File tmpDir = createTempDir( fastFolder ); + File tmpDir = createTempDir( fastDir ); File dstDir = new File( tmpDir, baseDir.getName() ); Files.move( baseDir.toPath(), dstDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); - BackgroundCleaner.delete( fastFolder, tmpDir ); + BackgroundCleaner.delete( fastDir, tmpDir ); return true; } catch ( IOException e ) @@ -161,24 +161,24 @@ private boolean fastDelete( File baseDir ) if ( logDebug != null ) { logDebug.log( "Unable to fast delete directory as the path " - + fastFolder + " does not point to a directory" ); + + fastDir + " does not point to a directory" ); } } return false; } - private File createTempDir( File baseFolder ) throws IOException + private File createTempDir( File baseDir ) throws IOException { for ( int i = 0; i < 10; i++ ) { int rnd = ThreadLocalRandom.current().nextInt(); - File tmpDir = new File( baseFolder, Integer.toHexString( rnd ) ); + File tmpDir = new File( baseDir, Integer.toHexString( rnd ) ); if ( tmpDir.mkdir() ) { return tmpDir; } } - throw new IOException( "Can not create temp folder" ); + throw new IOException( "Can not create temp directory" ); } /** @@ -360,20 +360,20 @@ static class BackgroundCleaner extends Thread private int status = 0; - public static void delete( File fastFolder, File folder ) + public static void delete( File fastDir, File dir ) { synchronized ( BackgroundCleaner.class ) { - if ( instance == null || !instance.doDelete( folder ) ) + if ( instance == null || !instance.doDelete( dir ) ) { - instance = new BackgroundCleaner( fastFolder, folder ); + instance = new BackgroundCleaner( fastDir, dir ); } } } - private BackgroundCleaner( File fastFolder, File folder ) + private BackgroundCleaner( File fastDir, File dir ) { - init( fastFolder, folder ); + init( fastDir, dir ); } public void run() @@ -396,11 +396,11 @@ public void run() } } - synchronized void init( File fastFolder, File folder ) + synchronized void init( File fastDir, File dir ) { - if ( fastFolder.isDirectory() ) + if ( fastDir.isDirectory() ) { - File[] children = fastFolder.listFiles(); + File[] children = fastDir.listFiles(); if ( children != null && children.length > 0 ) { for ( File child : children ) @@ -409,7 +409,7 @@ synchronized void init( File fastFolder, File folder ) } } } - doDelete( folder ); + doDelete( dir ); } synchronized File pollNext() @@ -422,13 +422,13 @@ synchronized File pollNext() return basedir; } - synchronized boolean doDelete( File folder ) + synchronized boolean doDelete( File dir ) { if ( status == 2 ) { return false; } - filesToDelete.add( folder ); + filesToDelete.add( dir ); if ( status == 0 ) { status = 1; From 40183751bed81054df6b6e8ccaeb4e04cf7825c9 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 16:55:51 +0100 Subject: [PATCH 03/24] Avoid magic constants --- .../org/apache/maven/plugins/clean/Cleaner.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 3f266223..00f2e91c 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -358,7 +358,11 @@ static class BackgroundCleaner extends Thread private final Cleaner cleaner = new Cleaner( null, false, null ); - private int status = 0; + private static final int NEW = 0; + private static final int RUNNING = 1; + private static final int STOPPED = 2; + + private int status = NEW; public static void delete( File fastDir, File dir ) { @@ -417,21 +421,21 @@ synchronized File pollNext() File basedir = filesToDelete.poll(); if ( basedir == null ) { - status = 2; + status = STOPPED; } return basedir; } synchronized boolean doDelete( File dir ) { - if ( status == 2 ) + if ( status == STOPPED ) { return false; } filesToDelete.add( dir ); - if ( status == 0 ) + if ( status == NEW ) { - status = 1; + status = RUNNING; start(); } return true; From 40902c3e2e22940822240a593847a9903ed356f8 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 17:00:06 +0100 Subject: [PATCH 04/24] Add a bit of doc to explain the fallback to the usual deletion mechanism --- src/main/java/org/apache/maven/plugins/clean/Cleaner.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 00f2e91c..b263bcf3 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -126,6 +126,7 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo if ( selector == null && !followSymlinks && fastDir != null ) { + // If anything wrong happens, we'll just use the usual deletion mechanism if ( fastDelete( file ) ) { return; @@ -144,6 +145,10 @@ private boolean fastDelete( File baseDir ) { File tmpDir = createTempDir( fastDir ); File dstDir = new File( tmpDir, baseDir.getName() ); + // Note that by specifying the ATOMIC_MOVE, we expect an exception to be thrown + // if the path leads to a directory on another mountpoint. If this is the case + // or any other exception occurs, an exception will be thrown in which case + // the method will return false and the usual deletion will be performed. Files.move( baseDir.toPath(), dstDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); BackgroundCleaner.delete( fastDir, tmpDir ); return true; From 8dfd61284e19effea0203618467b6ad9b8089ca6 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 21:49:06 +0100 Subject: [PATCH 05/24] Handle use case where the fastDir is inside the target folder --- .../apache/maven/plugins/clean/Cleaner.java | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index b263bcf3..26cc91f0 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -138,12 +138,42 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo private boolean fastDelete( File baseDir ) { - fastDir.mkdirs(); - if ( fastDir.isDirectory() ) + // Handle the case where we use ${maven.multiModuleProjectDirectory}/target/clean for example + if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() ) ) { try { - File tmpDir = createTempDir( fastDir ); + File tmpDir = createTempDir( baseDir.getParentFile(), baseDir.getName() + "." ); + try + { + Files.move( baseDir.toPath(), tmpDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); + baseDir = tmpDir; + } + catch ( IOException e ) + { + tmpDir.delete(); + if ( logDebug != null ) + { + logDebug.log( "Unable to fast delete directory: " + e ); + } + return false; + } + } + catch ( IOException e ) + { + if ( logDebug != null ) + { + logDebug.log( "Unable to fast delete directory: " + e ); + } + return false; + } + } + // Create fastDir and the needed parents if needed + if ( fastDir.mkdirs() || fastDir.isDirectory() ) + { + try + { + File tmpDir = createTempDir( fastDir, "" ); File dstDir = new File( tmpDir, baseDir.getName() ); // Note that by specifying the ATOMIC_MOVE, we expect an exception to be thrown // if the path leads to a directory on another mountpoint. If this is the case @@ -166,18 +196,18 @@ private boolean fastDelete( File baseDir ) if ( logDebug != null ) { logDebug.log( "Unable to fast delete directory as the path " - + fastDir + " does not point to a directory" ); + + fastDir + " does not point to a directory or can not be created" ); } } return false; } - private File createTempDir( File baseDir ) throws IOException + private File createTempDir( File baseDir, String prefix ) throws IOException { for ( int i = 0; i < 10; i++ ) { int rnd = ThreadLocalRandom.current().nextInt(); - File tmpDir = new File( baseDir, Integer.toHexString( rnd ) ); + File tmpDir = new File( baseDir, prefix + Integer.toHexString( rnd ) ); if ( tmpDir.mkdir() ) { return tmpDir; From 0a2030ddb969d7bbd1c198ec48d3c8f94ee6082f Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 22:52:42 +0100 Subject: [PATCH 06/24] Wait for file deletion to finish at the end of the maven session --- pom.xml | 2 +- .../apache/maven/plugins/clean/CleanMojo.java | 8 +- .../apache/maven/plugins/clean/Cleaner.java | 87 +++++++++++++++++-- 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index d15d566d..9db5a252 100644 --- a/pom.xml +++ b/pom.xml @@ -114,7 +114,7 @@ under the License. org.apache.maven maven-core ${mavenVersion} - test + provided junit diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index cd60920f..681880c7 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -19,6 +19,7 @@ * under the License. */ +import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.Mojo; @@ -27,6 +28,8 @@ import java.io.File; import java.io.IOException; +import javax.inject.Inject; + /** * Goal which cleans the build. *

@@ -167,6 +170,9 @@ public class CleanMojo @Parameter( property = "maven.clean.fastDir", defaultValue = "${maven.multiModuleProjectDirectory}/.mvn/clean" ) private File fastDir; + @Inject + private MavenSession session; + /** * Deletes file-sets in the following project build directory order: (source) directory, output directory, test * directory, report directory, and then the additional file-sets. @@ -183,7 +189,7 @@ public void execute() return; } - Cleaner cleaner = new Cleaner( getLog(), isVerbose(), fast ? fastDir : null ); + Cleaner cleaner = new Cleaner( session, getLog(), isVerbose(), fast ? fastDir : null ); try { diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 26cc91f0..d48bc6fd 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -25,8 +25,13 @@ import java.nio.file.StandardCopyOption; import java.util.ArrayDeque; import java.util.Deque; +import java.util.List; import java.util.concurrent.ThreadLocalRandom; +import org.apache.maven.eventspy.AbstractEventSpy; +import org.apache.maven.eventspy.EventSpy; +import org.apache.maven.execution.ExecutionEvent; +import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.logging.Log; import org.apache.maven.shared.utils.Os; import org.apache.maven.shared.utils.io.FileUtils; @@ -41,6 +46,8 @@ class Cleaner private static final boolean ON_WINDOWS = Os.isFamily( Os.FAMILY_WINDOWS ); + private final MavenSession session; + private final Logger logDebug; private final Logger logInfo; @@ -57,8 +64,9 @@ class Cleaner * @param log The logger to use, may be null to disable logging. * @param verbose Whether to perform verbose logging. */ - Cleaner( final Log log, boolean verbose, File fastDir ) + Cleaner( MavenSession session, final Log log, boolean verbose, File fastDir ) { + this.session = session; logDebug = ( log == null || !log.isDebugEnabled() ) ? null : new Logger() { public void log( CharSequence message ) @@ -180,7 +188,7 @@ private boolean fastDelete( File baseDir ) // or any other exception occurs, an exception will be thrown in which case // the method will return false and the usual deletion will be performed. Files.move( baseDir.toPath(), dstDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); - BackgroundCleaner.delete( fastDir, tmpDir ); + BackgroundCleaner.delete( this, tmpDir ); return true; } catch ( IOException e ) @@ -391,7 +399,7 @@ static class BackgroundCleaner extends Thread private final Deque filesToDelete = new ArrayDeque<>(); - private final Cleaner cleaner = new Cleaner( null, false, null ); + private final Cleaner cleaner; private static final int NEW = 0; private static final int RUNNING = 1; @@ -399,20 +407,32 @@ static class BackgroundCleaner extends Thread private int status = NEW; - public static void delete( File fastDir, File dir ) + public static void delete( Cleaner cleaner, File dir ) { synchronized ( BackgroundCleaner.class ) { if ( instance == null || !instance.doDelete( dir ) ) { - instance = new BackgroundCleaner( fastDir, dir ); + instance = new BackgroundCleaner( cleaner, dir ); + } + } + } + + static void sessionEnd() + { + synchronized ( BackgroundCleaner.class ) + { + if ( instance != null ) + { + instance.doSessionEnd(); } } } - private BackgroundCleaner( File fastDir, File dir ) + private BackgroundCleaner( Cleaner cleaner, File dir ) { - init( fastDir, dir ); + this.cleaner = cleaner; + init( cleaner.fastDir, dir ); } public void run() @@ -457,6 +477,7 @@ synchronized File pollNext() if ( basedir == null ) { status = STOPPED; + notifyAll(); } return basedir; } @@ -471,10 +492,62 @@ synchronized boolean doDelete( File dir ) if ( status == NEW ) { status = RUNNING; + notifyAll(); + List spies = cleaner.session.getRequest().getEventSpyDispatcher().getEventSpies(); + boolean hasSessionListener = false; + for ( EventSpy spy : spies ) + { + if ( spy instanceof SessionListener ) + { + hasSessionListener = true; + break; + } + } + if ( !hasSessionListener ) + { + spies.add( new SessionListener() ); + } start(); } return true; } + + synchronized void doSessionEnd() + { + if ( status != STOPPED ) + { + try + { + cleaner.logInfo.log( "Waiting for background file deletion" ); + while ( status != STOPPED ) + { + wait(); + } + } + catch ( InterruptedException e ) + { + // ignore + } + } + } + + } + + static class SessionListener extends AbstractEventSpy + { + @Override + public void onEvent( Object event ) + { + if ( event instanceof ExecutionEvent ) + { + ExecutionEvent ee = ( ExecutionEvent ) event; + if ( ee.getType() == ExecutionEvent.Type.SessionEnded ) + { + + BackgroundCleaner.sessionEnd(); + } + } + } } } From 5e3d0869395ceb3fddfb2319662ea469c0559378 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 23:19:54 +0100 Subject: [PATCH 07/24] Use [project]/target/.clean as the temporary cleaning folder, make sure that one is deleted at the end --- .../apache/maven/plugins/clean/CleanMojo.java | 2 +- .../apache/maven/plugins/clean/Cleaner.java | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 681880c7..6af79ea3 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -167,7 +167,7 @@ public class CleanMojo @Parameter( property = "maven.clean.fast", defaultValue = "false" ) private boolean fast; - @Parameter( property = "maven.clean.fastDir", defaultValue = "${maven.multiModuleProjectDirectory}/.mvn/clean" ) + @Parameter( property = "maven.clean.fastDir", defaultValue = "${maven.multiModuleProjectDirectory}/target/.clean" ) private File fastDir; @Inject diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index d48bc6fd..d1ef806e 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -46,6 +46,8 @@ class Cleaner private static final boolean ON_WINDOWS = Os.isFamily( Os.FAMILY_WINDOWS ); + private static final String LAST_DIRECTORY_TO_DELETE = Cleaner.class.getName() + ".lastDirectoryToDelete"; + private final MavenSession session; private final Logger logDebug; @@ -146,7 +148,7 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo private boolean fastDelete( File baseDir ) { - // Handle the case where we use ${maven.multiModuleProjectDirectory}/target/clean for example + // Handle the case where we use ${maven.multiModuleProjectDirectory}/target/.clean for example if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() ) ) { try @@ -155,6 +157,7 @@ private boolean fastDelete( File baseDir ) try { Files.move( baseDir.toPath(), tmpDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); + session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir ); baseDir = tmpDir; } catch ( IOException e ) @@ -476,8 +479,17 @@ synchronized File pollNext() File basedir = filesToDelete.poll(); if ( basedir == null ) { - status = STOPPED; - notifyAll(); + File lastFolder = ( File ) cleaner.session.getRequest().getData().get( LAST_DIRECTORY_TO_DELETE ); + if ( lastFolder != null ) + { + cleaner.session.getRequest().getData().remove( LAST_DIRECTORY_TO_DELETE ); + return lastFolder; + } + else + { + status = STOPPED; + notifyAll(); + } } return basedir; } From c807f9e58dc9aabc6f351ae7098b60e94f46924e Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 23:24:39 +0100 Subject: [PATCH 08/24] Require maven 3.3.1 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9db5a252..541ce33c 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,7 @@ under the License. - 3.1.1 + 3.3.1 7 2.22.2 3.6.1 From 2b80a51f4a1e2c2097adadf22da09489d7cd3c54 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Dec 2021 23:41:31 +0100 Subject: [PATCH 09/24] Fix unit tests and avoid the usage of EventSpyDispatcher --- .../apache/maven/plugins/clean/CleanMojo.java | 7 ++-- .../apache/maven/plugins/clean/Cleaner.java | 38 +++++++++++++------ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 6af79ea3..b870f410 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -22,14 +22,14 @@ import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; +import org.eclipse.sisu.Nullable; import java.io.File; import java.io.IOException; -import javax.inject.Inject; - /** * Goal which cleans the build. *

@@ -170,7 +170,8 @@ public class CleanMojo @Parameter( property = "maven.clean.fastDir", defaultValue = "${maven.multiModuleProjectDirectory}/target/.clean" ) private File fastDir; - @Inject + @Component + @Nullable private MavenSession session; /** diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index d1ef806e..69de5bc1 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -21,11 +21,13 @@ import java.io.File; import java.io.IOException; +import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.util.ArrayDeque; import java.util.Deque; import java.util.List; +import java.util.Map; import java.util.concurrent.ThreadLocalRandom; import org.apache.maven.eventspy.AbstractEventSpy; @@ -157,7 +159,10 @@ private boolean fastDelete( File baseDir ) try { Files.move( baseDir.toPath(), tmpDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); - session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir ); + if ( session != null ) + { + session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir ); + } baseDir = tmpDir; } catch ( IOException e ) @@ -479,17 +484,17 @@ synchronized File pollNext() File basedir = filesToDelete.poll(); if ( basedir == null ) { - File lastFolder = ( File ) cleaner.session.getRequest().getData().get( LAST_DIRECTORY_TO_DELETE ); - if ( lastFolder != null ) + if ( cleaner.session != null ) { - cleaner.session.getRequest().getData().remove( LAST_DIRECTORY_TO_DELETE ); - return lastFolder; - } - else - { - status = STOPPED; - notifyAll(); + Map data = cleaner.session.getRequest().getData(); + File lastFolder = ( File ) data.remove( LAST_DIRECTORY_TO_DELETE ); + if ( lastFolder != null ) + { + return lastFolder; + } } + status = STOPPED; + notifyAll(); } return basedir; } @@ -505,7 +510,18 @@ synchronized boolean doDelete( File dir ) { status = RUNNING; notifyAll(); - List spies = cleaner.session.getRequest().getEventSpyDispatcher().getEventSpies(); + List spies; + try + { + // The EventSpyDispatcher class is not exported by maven-core, so work-around... + Object eventSpyDispatcher = cleaner.session.getRequest().getEventSpyDispatcher(); + Method method = eventSpyDispatcher.getClass().getMethod( "getEventSpies" ); + spies = ( List ) method.invoke( eventSpyDispatcher ); + } + catch ( Exception e ) + { + return false; + } boolean hasSessionListener = false; for ( EventSpy spy : spies ) { From 5b6b7c12b2da7533cfda109a54e6de627e9a0ac3 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 13:27:20 +0100 Subject: [PATCH 10/24] Use the field which is available in both 3.x and 4.x until we get a proper api --- src/main/java/org/apache/maven/plugins/clean/Cleaner.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 69de5bc1..3321bfa3 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -21,7 +21,7 @@ import java.io.File; import java.io.IOException; -import java.lang.reflect.Method; +import java.lang.reflect.Field; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.util.ArrayDeque; @@ -515,8 +515,9 @@ synchronized boolean doDelete( File dir ) { // The EventSpyDispatcher class is not exported by maven-core, so work-around... Object eventSpyDispatcher = cleaner.session.getRequest().getEventSpyDispatcher(); - Method method = eventSpyDispatcher.getClass().getMethod( "getEventSpies" ); - spies = ( List ) method.invoke( eventSpyDispatcher ); + Field field = eventSpyDispatcher.getClass().getField( "eventSpies" ); + field.setAccessible( true ); + spies = ( List ) field.get( eventSpyDispatcher ); } catch ( Exception e ) { From b3830d02ab31d14c5d627d5c949347639b3c6b9a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 15:17:12 +0100 Subject: [PATCH 11/24] Relying on EventtSpy does not work with plain maven, so use the ExecutionListener instead --- .../apache/maven/plugins/clean/Cleaner.java | 62 +++++++++---------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 3321bfa3..a44f7f03 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -21,18 +21,17 @@ import java.io.File; import java.io.IOException; -import java.lang.reflect.Field; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.util.ArrayDeque; import java.util.Deque; -import java.util.List; import java.util.Map; import java.util.concurrent.ThreadLocalRandom; -import org.apache.maven.eventspy.AbstractEventSpy; -import org.apache.maven.eventspy.EventSpy; -import org.apache.maven.execution.ExecutionEvent; +import org.apache.maven.execution.ExecutionListener; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.logging.Log; import org.apache.maven.shared.utils.Os; @@ -510,31 +509,26 @@ synchronized boolean doDelete( File dir ) { status = RUNNING; notifyAll(); - List spies; try { // The EventSpyDispatcher class is not exported by maven-core, so work-around... - Object eventSpyDispatcher = cleaner.session.getRequest().getEventSpyDispatcher(); - Field field = eventSpyDispatcher.getClass().getField( "eventSpies" ); - field.setAccessible( true ); - spies = ( List ) field.get( eventSpyDispatcher ); - } - catch ( Exception e ) - { - return false; - } - boolean hasSessionListener = false; - for ( EventSpy spy : spies ) - { - if ( spy instanceof SessionListener ) + ExecutionListener executionListener = cleaner.session.getRequest().getExecutionListener(); + if ( !Proxy.isProxyClass( executionListener.getClass() ) + || !( Proxy.getInvocationHandler( executionListener ) instanceof SpyInvocationHandler ) ) { - hasSessionListener = true; - break; + ExecutionListener listener = ( ExecutionListener ) Proxy.newProxyInstance( + ExecutionListener.class.getClassLoader(), + new Class[] { ExecutionListener.class }, + new SpyInvocationHandler( executionListener ) ); + cleaner.session.getRequest().setExecutionListener( listener ); } } - if ( !hasSessionListener ) + catch ( Exception e ) { - spies.add( new SessionListener() ); + cleaner.logWarn.log( + "Unable to access maven core event spies. " + + "All files may not be deleted when maven stops. " + + "Please report this issue. " + e ); } start(); } @@ -562,21 +556,25 @@ synchronized void doSessionEnd() } - static class SessionListener extends AbstractEventSpy + static class SpyInvocationHandler implements InvocationHandler { + private final ExecutionListener delegate; + + SpyInvocationHandler( ExecutionListener delegate ) + { + this.delegate = delegate; + } + @Override - public void onEvent( Object event ) + public Object invoke( Object proxy, Method method, Object[] args ) throws Throwable { - if ( event instanceof ExecutionEvent ) + if ( "sessionEnded".equals( method.getName() ) ) { - ExecutionEvent ee = ( ExecutionEvent ) event; - if ( ee.getType() == ExecutionEvent.Type.SessionEnded ) - { - - BackgroundCleaner.sessionEnd(); - } + BackgroundCleaner.sessionEnd(); } + return method.invoke( delegate, args ); } + } } From 68c3aa67d04b02a6ee8d657146c0506c66a795d0 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 15:28:03 +0100 Subject: [PATCH 12/24] Extract in a method and add a bit of doc --- .../apache/maven/plugins/clean/Cleaner.java | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index a44f7f03..42fc4b40 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -509,32 +509,33 @@ synchronized boolean doDelete( File dir ) { status = RUNNING; notifyAll(); - try - { - // The EventSpyDispatcher class is not exported by maven-core, so work-around... - ExecutionListener executionListener = cleaner.session.getRequest().getExecutionListener(); - if ( !Proxy.isProxyClass( executionListener.getClass() ) - || !( Proxy.getInvocationHandler( executionListener ) instanceof SpyInvocationHandler ) ) - { - ExecutionListener listener = ( ExecutionListener ) Proxy.newProxyInstance( - ExecutionListener.class.getClassLoader(), - new Class[] { ExecutionListener.class }, - new SpyInvocationHandler( executionListener ) ); - cleaner.session.getRequest().setExecutionListener( listener ); - } - } - catch ( Exception e ) - { - cleaner.logWarn.log( - "Unable to access maven core event spies. " - + "All files may not be deleted when maven stops. " - + "Please report this issue. " + e ); - } + wrapExecutionListener(); start(); } return true; } + /** + * If this has not been done already, we wrap the ExecutionListener inside a proxy + * which simply delegates call to the previous listener. When the session ends, it will + * also call {@link BackgroundCleaner#sessionEnd()}. + * There's no clean API to do that properly as this is a very unusual use case for a plugin + * to outlive its main execution. + */ + private void wrapExecutionListener() + { + ExecutionListener executionListener = cleaner.session.getRequest().getExecutionListener(); + if ( !Proxy.isProxyClass( executionListener.getClass() ) + || !( Proxy.getInvocationHandler( executionListener ) instanceof SpyInvocationHandler ) ) + { + ExecutionListener listener = ( ExecutionListener ) Proxy.newProxyInstance( + ExecutionListener.class.getClassLoader(), + new Class[] { ExecutionListener.class }, + new SpyInvocationHandler( executionListener ) ); + cleaner.session.getRequest().setExecutionListener( listener ); + } + } + synchronized void doSessionEnd() { if ( status != STOPPED ) From e2e8a6511c382a1f317175e9942740a7a5297d01 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 16:08:16 +0100 Subject: [PATCH 13/24] Missed that folder -> dir rename --- src/main/java/org/apache/maven/plugins/clean/Cleaner.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 42fc4b40..127644b6 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -486,10 +486,10 @@ synchronized File pollNext() if ( cleaner.session != null ) { Map data = cleaner.session.getRequest().getData(); - File lastFolder = ( File ) data.remove( LAST_DIRECTORY_TO_DELETE ); - if ( lastFolder != null ) + File lastDir = ( File ) data.remove( LAST_DIRECTORY_TO_DELETE ); + if ( lastDir != null ) { - return lastFolder; + return lastDir; } } status = STOPPED; From 7bff5b5a6cdfe9c21f56906ca70b014b3625a1e3 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 16:09:14 +0100 Subject: [PATCH 14/24] Refactor the fastDelete method to use nio Path instead of File, add warnings about the Logger interface --- .../apache/maven/plugins/clean/Cleaner.java | 76 +++++++++---------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 127644b6..ffe77462 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -25,11 +25,11 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.ArrayDeque; import java.util.Deque; import java.util.Map; -import java.util.concurrent.ThreadLocalRandom; import org.apache.maven.execution.ExecutionListener; import org.apache.maven.execution.MavenSession; @@ -147,17 +147,20 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo delete( file, "", selector, followSymlinks, failOnError, retryOnError ); } - private boolean fastDelete( File baseDir ) + private boolean fastDelete( File baseDirFile ) { + Path baseDir = baseDirFile.toPath(); + Path fastDir = this.fastDir.toPath(); // Handle the case where we use ${maven.multiModuleProjectDirectory}/target/.clean for example - if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() ) ) + if ( fastDir.toAbsolutePath().startsWith( baseDir.toAbsolutePath() ) ) { try { - File tmpDir = createTempDir( baseDir.getParentFile(), baseDir.getName() + "." ); + String prefix = baseDir.getFileName().toString() + "."; + Path tmpDir = Files.createTempDirectory( baseDir.getParent(), prefix ); try { - Files.move( baseDir.toPath(), tmpDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); + Files.move( baseDir, tmpDir, StandardCopyOption.REPLACE_EXISTING ); if ( session != null ) { session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir ); @@ -166,69 +169,60 @@ private boolean fastDelete( File baseDir ) } catch ( IOException e ) { - tmpDir.delete(); - if ( logDebug != null ) - { - logDebug.log( "Unable to fast delete directory: " + e ); - } - return false; + Files.delete( tmpDir ); + throw e; } } catch ( IOException e ) { if ( logDebug != null ) { + // TODO: this Logger interface cannot log exceptions and needs refactoring logDebug.log( "Unable to fast delete directory: " + e ); } return false; } } // Create fastDir and the needed parents if needed - if ( fastDir.mkdirs() || fastDir.isDirectory() ) + try { - try + if ( !Files.isDirectory( fastDir ) ) { - File tmpDir = createTempDir( fastDir, "" ); - File dstDir = new File( tmpDir, baseDir.getName() ); - // Note that by specifying the ATOMIC_MOVE, we expect an exception to be thrown - // if the path leads to a directory on another mountpoint. If this is the case - // or any other exception occurs, an exception will be thrown in which case - // the method will return false and the usual deletion will be performed. - Files.move( baseDir.toPath(), dstDir.toPath(), StandardCopyOption.ATOMIC_MOVE ); - BackgroundCleaner.delete( this, tmpDir ); - return true; - } - catch ( IOException e ) - { - if ( logDebug != null ) - { - logDebug.log( "Unable to fast delete directory: " + e ); - } + Files.createDirectories( fastDir ); } } - else + catch ( IOException e ) { if ( logDebug != null ) { + // TODO: this Logger interface cannot log exceptions and needs refactoring logDebug.log( "Unable to fast delete directory as the path " - + fastDir + " does not point to a directory or can not be created" ); + + fastDir + " does not point to a directory or cannot be created: " + e ); } + return false; } - return false; - } - private File createTempDir( File baseDir, String prefix ) throws IOException - { - for ( int i = 0; i < 10; i++ ) + try { - int rnd = ThreadLocalRandom.current().nextInt(); - File tmpDir = new File( baseDir, prefix + Integer.toHexString( rnd ) ); - if ( tmpDir.mkdir() ) + Path tmpDir = Files.createTempDirectory( fastDir, "" ); + Path dstDir = tmpDir.resolve( baseDir.getFileName() ); + // Note that by specifying the ATOMIC_MOVE, we expect an exception to be thrown + // if the path leads to a directory on another mountpoint. If this is the case + // or any other exception occurs, an exception will be thrown in which case + // the method will return false and the usual deletion will be performed. + Files.move( baseDir, dstDir, StandardCopyOption.ATOMIC_MOVE ); + BackgroundCleaner.delete( this, tmpDir.toFile() ); + return true; + } + catch ( IOException e ) + { + if ( logDebug != null ) { - return tmpDir; + // TODO: this Logger interface cannot log exceptions and needs refactoring + logDebug.log( "Unable to fast delete directory: " + e ); } + return false; } - throw new IOException( "Can not create temp directory" ); } /** From 605656bc4a46894a5e991656faeb3a94256b3740 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 16:16:45 +0100 Subject: [PATCH 15/24] Add support for the ExecutionListener being null --- .../java/org/apache/maven/plugins/clean/Cleaner.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index ffe77462..ecee60b4 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -135,7 +135,7 @@ public void delete( File basedir, Selector selector, boolean followSymlinks, boo File file = followSymlinks ? basedir : basedir.getCanonicalFile(); - if ( selector == null && !followSymlinks && fastDir != null ) + if ( selector == null && !followSymlinks && fastDir != null && session != null ) { // If anything wrong happens, we'll just use the usual deletion mechanism if ( fastDelete( file ) ) @@ -519,7 +519,8 @@ synchronized boolean doDelete( File dir ) private void wrapExecutionListener() { ExecutionListener executionListener = cleaner.session.getRequest().getExecutionListener(); - if ( !Proxy.isProxyClass( executionListener.getClass() ) + if ( executionListener == null + || !Proxy.isProxyClass( executionListener.getClass() ) || !( Proxy.getInvocationHandler( executionListener ) instanceof SpyInvocationHandler ) ) { ExecutionListener listener = ( ExecutionListener ) Proxy.newProxyInstance( @@ -567,7 +568,11 @@ public Object invoke( Object proxy, Method method, Object[] args ) throws Throwa { BackgroundCleaner.sessionEnd(); } - return method.invoke( delegate, args ); + if ( delegate != null ) + { + return method.invoke(delegate, args); + } + return null; } } From 2340ce1f80994b716cfc9310abaa1ff0072a7bf9 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 16:17:24 +0100 Subject: [PATCH 16/24] Fix checkstyle --- src/main/java/org/apache/maven/plugins/clean/Cleaner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index ecee60b4..424eb326 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -570,7 +570,7 @@ public Object invoke( Object proxy, Method method, Object[] args ) throws Throwa } if ( delegate != null ) { - return method.invoke(delegate, args); + return method.invoke( delegate, args ); } return null; } From ac67308c7087756f677179f9cc617fe66ea3f803 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 16:46:32 +0100 Subject: [PATCH 17/24] A File is expected to be used --- src/main/java/org/apache/maven/plugins/clean/Cleaner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 424eb326..ff2a42dd 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -163,7 +163,7 @@ private boolean fastDelete( File baseDirFile ) Files.move( baseDir, tmpDir, StandardCopyOption.REPLACE_EXISTING ); if ( session != null ) { - session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir ); + session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir.toFile() ); } baseDir = tmpDir; } From 035c756f385af7bd0ae24f33f1cf8690558df375 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 16:48:08 +0100 Subject: [PATCH 18/24] Revert maven 3.3.1 requirement down to 3.1.1 --- pom.xml | 2 +- .../apache/maven/plugins/clean/CleanMojo.java | 25 +++++++++++++++++-- .../apache/maven/plugins/clean/Cleaner.java | 9 ++++--- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 541ce33c..9db5a252 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,7 @@ under the License. - 3.3.1 + 3.1.1 7 2.22.2 3.6.1 diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index b870f410..220bdfdd 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -167,7 +167,10 @@ public class CleanMojo @Parameter( property = "maven.clean.fast", defaultValue = "false" ) private boolean fast; - @Parameter( property = "maven.clean.fastDir", defaultValue = "${maven.multiModuleProjectDirectory}/target/.clean" ) + @Parameter( defaultValue = "${maven.multiModuleProjectDirectory}" ) + private String multiModuleProjectDirectory; + + @Parameter( property = "maven.clean.fastDir" ) private File fastDir; @Component @@ -190,7 +193,25 @@ public void execute() return; } - Cleaner cleaner = new Cleaner( session, getLog(), isVerbose(), fast ? fastDir : null ); + File fastDir; + if ( fast && this.fastDir != null ) + { + fastDir = this.fastDir; + } + else if ( fast && multiModuleProjectDirectory != null ) + { + fastDir = new File( multiModuleProjectDirectory, "target/.clean" ); + } + else + { + fastDir = null; + if ( fast ) + { + getLog().warn( "Fast deletion requires maven 3.3.1" ); + } + } + + Cleaner cleaner = new Cleaner( session, getLog(), isVerbose(), fastDir ); try { diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index ff2a42dd..64473c3f 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -29,13 +29,13 @@ import java.nio.file.StandardCopyOption; import java.util.ArrayDeque; import java.util.Deque; -import java.util.Map; import org.apache.maven.execution.ExecutionListener; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.logging.Log; import org.apache.maven.shared.utils.Os; import org.apache.maven.shared.utils.io.FileUtils; +import org.eclipse.aether.SessionData; /** * Cleans directories. @@ -163,7 +163,7 @@ private boolean fastDelete( File baseDirFile ) Files.move( baseDir, tmpDir, StandardCopyOption.REPLACE_EXISTING ); if ( session != null ) { - session.getRequest().getData().put( LAST_DIRECTORY_TO_DELETE, baseDir.toFile() ); + session.getRepositorySession().getData().set( LAST_DIRECTORY_TO_DELETE, baseDir.toFile() ); } baseDir = tmpDir; } @@ -479,10 +479,11 @@ synchronized File pollNext() { if ( cleaner.session != null ) { - Map data = cleaner.session.getRequest().getData(); - File lastDir = ( File ) data.remove( LAST_DIRECTORY_TO_DELETE ); + SessionData data = cleaner.session.getRepositorySession().getData(); + File lastDir = ( File ) data.get( LAST_DIRECTORY_TO_DELETE ); if ( lastDir != null ) { + data.set( LAST_DIRECTORY_TO_DELETE, null ); return lastDir; } } From d737e200178d111fa410c001a460bfcfcfc2575c Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 17:16:38 +0100 Subject: [PATCH 19/24] Clean mojo properties --- .../apache/maven/plugins/clean/CleanMojo.java | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 220bdfdd..04d24bc4 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -22,10 +22,8 @@ import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; -import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; -import org.eclipse.sisu.Nullable; import java.io.File; import java.io.IOException; @@ -164,17 +162,29 @@ public class CleanMojo @Parameter( property = "maven.clean.excludeDefaultDirectories", defaultValue = "false" ) private boolean excludeDefaultDirectories; + /** + * Enables fast clean if possible. If set to true, when the plugin is executed, a directory to + * be deleted will be atomically moved inside the maven.clean.fastDir directory and a thread will + * be launched to delete the needed files in the background. When the build is completed, maven will wait + * until all the files have been deleted. + * + * @since 3.2 + */ @Parameter( property = "maven.clean.fast", defaultValue = "false" ) private boolean fast; - @Parameter( defaultValue = "${maven.multiModuleProjectDirectory}" ) - private String multiModuleProjectDirectory; - + /** + * When fast clean is specified, the fastDir property will be used as the location where directories + * to be deleted will be moved prior to background deletion. If not specified, the + * ${maven.multiModuleProjectDirectory}/target/.clean directory will be used. If the + * ${build.directory} has been modified, you'll have to adjust this property explicitly. + * + * @since 3.2 + */ @Parameter( property = "maven.clean.fastDir" ) private File fastDir; - @Component - @Nullable + @Parameter( defaultValue = "${session}", readonly = true ) private MavenSession session; /** @@ -193,6 +203,8 @@ public void execute() return; } + String multiModuleProjectDirectory = session != null + ? session.getSystemProperties().getProperty( "maven.multiModuleProjectDirectory" ) : null; File fastDir; if ( fast && this.fastDir != null ) { @@ -207,7 +219,9 @@ else if ( fast && multiModuleProjectDirectory != null ) fastDir = null; if ( fast ) { - getLog().warn( "Fast deletion requires maven 3.3.1" ); + getLog().warn( "Fast clean requires maven 3.3.1 or newer, " + + "or an explicit directory to be specified with the 'fastDir' configuration of " + + "this plugin, or the 'maven.clean.fastDir' system property to be set." ); } } From 8a5159f4fcd2b17062a16c0c94efb2b1fb37e017 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Dec 2021 17:20:31 +0100 Subject: [PATCH 20/24] A bit more doc --- src/main/java/org/apache/maven/plugins/clean/CleanMojo.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 04d24bc4..c2a7a11c 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -166,7 +166,8 @@ public class CleanMojo * Enables fast clean if possible. If set to true, when the plugin is executed, a directory to * be deleted will be atomically moved inside the maven.clean.fastDir directory and a thread will * be launched to delete the needed files in the background. When the build is completed, maven will wait - * until all the files have been deleted. + * until all the files have been deleted. If any problem occurs during the atomic move of the directories, + * the plugin will default to the traditional deletion mechanism. * * @since 3.2 */ @@ -178,6 +179,9 @@ public class CleanMojo * to be deleted will be moved prior to background deletion. If not specified, the * ${maven.multiModuleProjectDirectory}/target/.clean directory will be used. If the * ${build.directory} has been modified, you'll have to adjust this property explicitly. + * In order for fast clean to work correctly, this directory must reside on the same mountpoint that the + * various directories that will be deleted. This is usually the case when the whole code tree is setup to + * reside on a standard drive. * * @since 3.2 */ From 7bfb97a9568e2dabab905358c671de2f99e0f22d Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sun, 19 Dec 2021 20:43:16 +0100 Subject: [PATCH 21/24] Small improvements --- .../java/org/apache/maven/plugins/clean/CleanMojo.java | 9 +++++---- .../java/org/apache/maven/plugins/clean/Cleaner.java | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index c2a7a11c..7af3805e 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -179,11 +179,12 @@ public class CleanMojo * to be deleted will be moved prior to background deletion. If not specified, the * ${maven.multiModuleProjectDirectory}/target/.clean directory will be used. If the * ${build.directory} has been modified, you'll have to adjust this property explicitly. - * In order for fast clean to work correctly, this directory must reside on the same mountpoint that the - * various directories that will be deleted. This is usually the case when the whole code tree is setup to - * reside on a standard drive. + * In order for fast clean to work correctly, this directory and the various directories that will be deleted + * should usually reside on the same volume. The exact conditions are system dependant though, but if an atomic + * move is not supported, the standard deletion mechanism will be used. * * @since 3.2 + * @see #fast */ @Parameter( property = "maven.clean.fastDir" ) private File fastDir; @@ -225,7 +226,7 @@ else if ( fast && multiModuleProjectDirectory != null ) { getLog().warn( "Fast clean requires maven 3.3.1 or newer, " + "or an explicit directory to be specified with the 'fastDir' configuration of " - + "this plugin, or the 'maven.clean.fastDir' system property to be set." ); + + "this plugin, or the 'maven.clean.fastDir' user property to be set." ); } } diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 64473c3f..be9cc8fb 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -432,6 +432,7 @@ static void sessionEnd() private BackgroundCleaner( Cleaner cleaner, File dir ) { + super( "mvn-background-cleaner" ); this.cleaner = cleaner; init( cleaner.fastDir, dir ); } From 3ca9c815b41900e66e9e49d8084a328550c4f50e Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sun, 19 Dec 2021 20:46:25 +0100 Subject: [PATCH 22/24] Add a comment about session being eventually null during unit tests --- src/main/java/org/apache/maven/plugins/clean/Cleaner.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index be9cc8fb..93cb3a48 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -49,6 +49,9 @@ class Cleaner private static final String LAST_DIRECTORY_TO_DELETE = Cleaner.class.getName() + ".lastDirectoryToDelete"; + /** + * The maven session. This is typically non-null in a real run, but it can be during unit tests. + */ private final MavenSession session; private final Logger logDebug; From 8a8fd3933e32fc5b0d722b44fb2007ade4e2e7d9 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 4 Jan 2022 10:18:16 +0100 Subject: [PATCH 23/24] Add a fastMode to further specify when files are actually deleted --- .../apache/maven/plugins/clean/CleanMojo.java | 29 ++++++++++- .../apache/maven/plugins/clean/Cleaner.java | 50 ++++++++++++------- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 7af3805e..74769c62 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -49,6 +49,12 @@ public class CleanMojo extends AbstractMojo { + public static final String FAST_MODE_BACKGROUND = "background"; + + public static final String FAST_MODE_AT_END = "at-end"; + + public static final String FAST_MODE_DEFER = "defer"; + /** * This is where build results go. */ @@ -189,6 +195,19 @@ public class CleanMojo @Parameter( property = "maven.clean.fastDir" ) private File fastDir; + /** + * Mode to use when using fast clean. Values are: background to start deletion immediately and + * waiting for all files to be deleted when the session ends, at-end to indicate that the actual + * deletion should be performed synchronously when the session ends, or defer to specify that + * the actual file deletion should be started in the background when the session ends (this should only be used + * when maven is embedded in a long running process). + * + * @since 3.2 + * @see #fast + */ + @Parameter( property = "maven.clean.fastMode", defaultValue = FAST_MODE_BACKGROUND ) + private String fastMode; + @Parameter( defaultValue = "${session}", readonly = true ) private MavenSession session; @@ -229,8 +248,16 @@ else if ( fast && multiModuleProjectDirectory != null ) + "this plugin, or the 'maven.clean.fastDir' user property to be set." ); } } + if ( fast && !FAST_MODE_BACKGROUND.equals( fastMode ) + && !FAST_MODE_AT_END.equals( fastMode ) + && !FAST_MODE_DEFER.equals( fastMode ) ) + { + getLog().warn( "Illegal value '" + fastMode + "' for fastMode. Allowed values are '" + + FAST_MODE_BACKGROUND + "', '" + FAST_MODE_AT_END + "' and '" + FAST_MODE_DEFER + "'. " + + "Reverting to use the default value '" + FAST_MODE_BACKGROUND + "'." ); + } - Cleaner cleaner = new Cleaner( session, getLog(), isVerbose(), fastDir ); + Cleaner cleaner = new Cleaner( session, getLog(), isVerbose(), fastDir, fastMode ); try { diff --git a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java index 93cb3a48..015bf7af 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -37,6 +37,9 @@ import org.apache.maven.shared.utils.io.FileUtils; import org.eclipse.aether.SessionData; +import static org.apache.maven.plugins.clean.CleanMojo.FAST_MODE_BACKGROUND; +import static org.apache.maven.plugins.clean.CleanMojo.FAST_MODE_DEFER; + /** * Cleans directories. * @@ -64,13 +67,15 @@ class Cleaner private final File fastDir; + private final String fastMode; + /** * Creates a new cleaner. - * * @param log The logger to use, may be null to disable logging. * @param verbose Whether to perform verbose logging. + * @param fastMode The fast deletion mode */ - Cleaner( MavenSession session, final Log log, boolean verbose, File fastDir ) + Cleaner( MavenSession session, final Log log, boolean verbose, File fastDir, String fastMode ) { this.session = session; logDebug = ( log == null || !log.isDebugEnabled() ) ? null : new Logger() @@ -100,6 +105,7 @@ public void log( CharSequence message ) logVerbose = verbose ? logInfo : logDebug; this.fastDir = fastDir; + this.fastMode = fastMode; } /** @@ -214,7 +220,7 @@ private boolean fastDelete( File baseDirFile ) // or any other exception occurs, an exception will be thrown in which case // the method will return false and the usual deletion will be performed. Files.move( baseDir, dstDir, StandardCopyOption.ATOMIC_MOVE ); - BackgroundCleaner.delete( this, tmpDir.toFile() ); + BackgroundCleaner.delete( this, tmpDir.toFile(), fastMode ); return true; } catch ( IOException e ) @@ -396,7 +402,7 @@ private interface Logger } - static class BackgroundCleaner extends Thread + private static class BackgroundCleaner extends Thread { private static BackgroundCleaner instance; @@ -405,19 +411,21 @@ static class BackgroundCleaner extends Thread private final Cleaner cleaner; + private final String fastMode; + private static final int NEW = 0; private static final int RUNNING = 1; private static final int STOPPED = 2; private int status = NEW; - public static void delete( Cleaner cleaner, File dir ) + public static void delete( Cleaner cleaner, File dir, String fastMode ) { synchronized ( BackgroundCleaner.class ) { if ( instance == null || !instance.doDelete( dir ) ) { - instance = new BackgroundCleaner( cleaner, dir ); + instance = new BackgroundCleaner( cleaner, dir, fastMode ); } } } @@ -433,10 +441,11 @@ static void sessionEnd() } } - private BackgroundCleaner( Cleaner cleaner, File dir ) + private BackgroundCleaner( Cleaner cleaner, File dir, String fastMode ) { super( "mvn-background-cleaner" ); this.cleaner = cleaner; + this.fastMode = fastMode; init( cleaner.fastDir, dir ); } @@ -504,13 +513,13 @@ synchronized boolean doDelete( File dir ) return false; } filesToDelete.add( dir ); - if ( status == NEW ) + if ( status == NEW && FAST_MODE_BACKGROUND.equals( fastMode ) ) { status = RUNNING; notifyAll(); - wrapExecutionListener(); start(); } + wrapExecutionListener(); return true; } @@ -540,17 +549,24 @@ synchronized void doSessionEnd() { if ( status != STOPPED ) { - try + if ( status == NEW ) { - cleaner.logInfo.log( "Waiting for background file deletion" ); - while ( status != STOPPED ) - { - wait(); - } + start(); } - catch ( InterruptedException e ) + if ( !FAST_MODE_DEFER.equals( fastMode ) ) { - // ignore + try + { + cleaner.logInfo.log( "Waiting for background file deletion" ); + while ( status != STOPPED ) + { + wait(); + } + } + catch ( InterruptedException e ) + { + // ignore + } } } } From 3fc91dea63001c183f511f7b1f091fbd77057998 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 4 Jan 2022 10:29:15 +0100 Subject: [PATCH 24/24] Throw an exception instead of a warning --- src/main/java/org/apache/maven/plugins/clean/CleanMojo.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java index 74769c62..4b5ba86b 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -252,9 +252,8 @@ else if ( fast && multiModuleProjectDirectory != null ) && !FAST_MODE_AT_END.equals( fastMode ) && !FAST_MODE_DEFER.equals( fastMode ) ) { - getLog().warn( "Illegal value '" + fastMode + "' for fastMode. Allowed values are '" - + FAST_MODE_BACKGROUND + "', '" + FAST_MODE_AT_END + "' and '" + FAST_MODE_DEFER + "'. " - + "Reverting to use the default value '" + FAST_MODE_BACKGROUND + "'." ); + throw new IllegalArgumentException( "Illegal value '" + fastMode + "' for fastMode. Allowed values are '" + + FAST_MODE_BACKGROUND + "', '" + FAST_MODE_AT_END + "' and '" + FAST_MODE_DEFER + "'." ); } Cleaner cleaner = new Cleaner( session, getLog(), isVerbose(), fastDir, fastMode );