michael-o commented on a change in pull request #6:
URL: https://github.com/apache/maven-clean-plugin/pull/6#discussion_r771429403
##########
File path: pom.xml
##########
@@ -65,7 +65,7 @@ under the License.
</distributionManagement>
<properties>
- <mavenVersion>3.1.1</mavenVersion>
+ <mavenVersion>3.3.1</mavenVersion>
Review comment:
This upgrade should be separate ticket.
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -115,9 +135,102 @@ public void delete( File basedir, Selector selector,
boolean followSymlinks, boo
File file = followSymlinks ? basedir : basedir.getCanonicalFile();
+ if ( selector == null && !followSymlinks && fastDir != null )
+ {
+ // If anything wrong happens, we'll just use the usual deletion
mechanism
+ if ( fastDelete( file ) )
+ {
+ return;
+ }
+ }
+
delete( file, "", selector, followSymlinks, failOnError, retryOnError
);
}
+ private boolean fastDelete( File baseDir )
+ {
+ // Handle the case where we use
${maven.multiModuleProjectDirectory}/target/.clean for example
+ if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() )
)
Review comment:
Can't we use `Path` for this? String comparison and Windows...
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -115,9 +135,102 @@ public void delete( File basedir, Selector selector,
boolean followSymlinks, boo
File file = followSymlinks ? basedir : basedir.getCanonicalFile();
+ if ( selector == null && !followSymlinks && fastDir != null )
+ {
+ // If anything wrong happens, we'll just use the usual deletion
mechanism
+ if ( fastDelete( file ) )
+ {
+ return;
+ }
+ }
+
delete( file, "", selector, followSymlinks, failOnError, retryOnError
);
}
+ private boolean fastDelete( File baseDir )
+ {
+ // Handle the case where we use
${maven.multiModuleProjectDirectory}/target/.clean for example
+ if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() )
)
+ {
+ try
+ {
+ File tmpDir = createTempDir( baseDir.getParentFile(),
baseDir.getName() + "." );
+ try
+ {
+ Files.move( baseDir.toPath(), tmpDir.toPath(),
StandardCopyOption.ATOMIC_MOVE );
+ if ( session != null )
+ {
+ session.getRequest().getData().put(
LAST_DIRECTORY_TO_DELETE, baseDir );
+ }
+ 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 );
Review comment:
Same here.
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -286,4 +399,182 @@ public void update( Result result )
}
+ static class BackgroundCleaner extends Thread
+ {
+
+ private static BackgroundCleaner instance;
+
+ private final Deque<File> filesToDelete = new ArrayDeque<>();
+
+ private final Cleaner cleaner;
+
+ 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 )
+ {
+ synchronized ( BackgroundCleaner.class )
+ {
+ if ( instance == null || !instance.doDelete( dir ) )
+ {
+ instance = new BackgroundCleaner( cleaner, dir );
+ }
+ }
+ }
+
+ static void sessionEnd()
+ {
+ synchronized ( BackgroundCleaner.class )
+ {
+ if ( instance != null )
+ {
+ instance.doSessionEnd();
+ }
+ }
+ }
+
+ private BackgroundCleaner( Cleaner cleaner, File dir )
+ {
+ this.cleaner = cleaner;
+ init( cleaner.fastDir, dir );
+ }
+
+ 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 fastDir, File dir )
+ {
+ if ( fastDir.isDirectory() )
+ {
+ File[] children = fastDir.listFiles();
+ if ( children != null && children.length > 0 )
+ {
+ for ( File child : children )
+ {
+ doDelete( child );
+ }
+ }
+ }
+ doDelete( dir );
+ }
+
+ synchronized File pollNext()
+ {
+ File basedir = filesToDelete.poll();
+ if ( basedir == null )
+ {
+ if ( cleaner.session != null )
+ {
+ Map<String, Object> data =
cleaner.session.getRequest().getData();
+ File lastFolder = ( File ) data.remove(
LAST_DIRECTORY_TO_DELETE );
Review comment:
dir
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -115,9 +135,102 @@ public void delete( File basedir, Selector selector,
boolean followSymlinks, boo
File file = followSymlinks ? basedir : basedir.getCanonicalFile();
+ if ( selector == null && !followSymlinks && fastDir != null )
+ {
+ // If anything wrong happens, we'll just use the usual deletion
mechanism
+ if ( fastDelete( file ) )
+ {
+ return;
+ }
+ }
+
delete( file, "", selector, followSymlinks, failOnError, retryOnError
);
}
+ private boolean fastDelete( File baseDir )
+ {
+ // Handle the case where we use
${maven.multiModuleProjectDirectory}/target/.clean for example
+ if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() )
)
+ {
+ try
+ {
+ File tmpDir = createTempDir( baseDir.getParentFile(),
baseDir.getName() + "." );
+ try
+ {
+ Files.move( baseDir.toPath(), tmpDir.toPath(),
StandardCopyOption.ATOMIC_MOVE );
+ if ( session != null )
+ {
+ session.getRequest().getData().put(
LAST_DIRECTORY_TO_DELETE, baseDir );
+ }
+ 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
+ // 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 );
Review comment:
"Unable to fast delete directory: " + e
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -286,4 +399,182 @@ public void update( Result result )
}
+ static class BackgroundCleaner extends Thread
+ {
+
+ private static BackgroundCleaner instance;
+
+ private final Deque<File> filesToDelete = new ArrayDeque<>();
+
+ private final Cleaner cleaner;
+
+ 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 )
+ {
+ synchronized ( BackgroundCleaner.class )
+ {
+ if ( instance == null || !instance.doDelete( dir ) )
+ {
+ instance = new BackgroundCleaner( cleaner, dir );
+ }
+ }
+ }
+
+ static void sessionEnd()
+ {
+ synchronized ( BackgroundCleaner.class )
+ {
+ if ( instance != null )
+ {
+ instance.doSessionEnd();
+ }
+ }
+ }
+
+ private BackgroundCleaner( Cleaner cleaner, File dir )
+ {
+ this.cleaner = cleaner;
+ init( cleaner.fastDir, dir );
+ }
+
+ 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 fastDir, File dir )
+ {
+ if ( fastDir.isDirectory() )
+ {
+ File[] children = fastDir.listFiles();
+ if ( children != null && children.length > 0 )
+ {
+ for ( File child : children )
+ {
+ doDelete( child );
+ }
+ }
+ }
+ doDelete( dir );
+ }
+
+ synchronized File pollNext()
+ {
+ File basedir = filesToDelete.poll();
+ if ( basedir == null )
+ {
+ if ( cleaner.session != null )
+ {
+ Map<String, Object> data =
cleaner.session.getRequest().getData();
+ File lastFolder = ( File ) data.remove(
LAST_DIRECTORY_TO_DELETE );
+ if ( lastFolder != null )
+ {
+ return lastFolder;
+ }
+ }
+ status = STOPPED;
+ notifyAll();
+ }
+ return basedir;
+ }
+
+ synchronized boolean doDelete( File dir )
+ {
+ if ( status == STOPPED )
+ {
+ return false;
+ }
+ filesToDelete.add( dir );
+ if ( status == NEW )
+ {
+ status = RUNNING;
+ notifyAll();
+ try
+ {
+ // The EventSpyDispatcher class is not exported by
maven-core, so work-around...
+ ExecutionListener executionListener =
cleaner.session.getRequest().getExecutionListener();
Review comment:
TEst for null?
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -115,9 +135,102 @@ public void delete( File basedir, Selector selector,
boolean followSymlinks, boo
File file = followSymlinks ? basedir : basedir.getCanonicalFile();
+ if ( selector == null && !followSymlinks && fastDir != null )
+ {
+ // If anything wrong happens, we'll just use the usual deletion
mechanism
+ if ( fastDelete( file ) )
+ {
+ return;
+ }
+ }
+
delete( file, "", selector, followSymlinks, failOnError, retryOnError
);
}
+ private boolean fastDelete( File baseDir )
+ {
+ // Handle the case where we use
${maven.multiModuleProjectDirectory}/target/.clean for example
+ if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() )
)
+ {
+ try
+ {
+ File tmpDir = createTempDir( baseDir.getParentFile(),
baseDir.getName() + "." );
+ try
+ {
+ Files.move( baseDir.toPath(), tmpDir.toPath(),
StandardCopyOption.ATOMIC_MOVE );
+ if ( session != null )
+ {
+ session.getRequest().getData().put(
LAST_DIRECTORY_TO_DELETE, baseDir );
+ }
+ baseDir = tmpDir;
+ }
+ catch ( IOException e )
+ {
+ tmpDir.delete();
+ if ( logDebug != null )
+ {
+ logDebug.log( "Unable to fast delete directory: " + e
);
Review comment:
Don't do this, pass the exception directly: `"Unable to fast delete
directory". e`
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -115,9 +135,102 @@ public void delete( File basedir, Selector selector,
boolean followSymlinks, boo
File file = followSymlinks ? basedir : basedir.getCanonicalFile();
+ if ( selector == null && !followSymlinks && fastDir != null )
+ {
+ // If anything wrong happens, we'll just use the usual deletion
mechanism
+ if ( fastDelete( file ) )
+ {
+ return;
+ }
+ }
+
delete( file, "", selector, followSymlinks, failOnError, retryOnError
);
}
+ private boolean fastDelete( File baseDir )
+ {
+ // Handle the case where we use
${maven.multiModuleProjectDirectory}/target/.clean for example
+ if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() )
)
+ {
+ try
+ {
+ File tmpDir = createTempDir( baseDir.getParentFile(),
baseDir.getName() + "." );
+ try
+ {
+ Files.move( baseDir.toPath(), tmpDir.toPath(),
StandardCopyOption.ATOMIC_MOVE );
+ if ( session != null )
+ {
+ session.getRequest().getData().put(
LAST_DIRECTORY_TO_DELETE, baseDir );
+ }
+ 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
+ // 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 );
+ }
+ }
+ }
+ else
+ {
+ if ( logDebug != null )
+ {
+ logDebug.log( "Unable to fast delete directory as the path "
+ + fastDir + " does not point to a directory or can not
be created" );
+ }
+ }
+ return false;
+ }
+
+ 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, prefix + Integer.toHexString( rnd
) );
Review comment:
What's wrong with:
https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempDirectory-java.nio.file.Path-java.lang.String-java.nio.file.attribute.FileAttribute...-?
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -286,4 +399,182 @@ public void update( Result result )
}
+ static class BackgroundCleaner extends Thread
+ {
+
+ private static BackgroundCleaner instance;
+
+ private final Deque<File> filesToDelete = new ArrayDeque<>();
+
+ private final Cleaner cleaner;
+
+ 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 )
+ {
+ synchronized ( BackgroundCleaner.class )
+ {
+ if ( instance == null || !instance.doDelete( dir ) )
+ {
+ instance = new BackgroundCleaner( cleaner, dir );
+ }
+ }
+ }
+
+ static void sessionEnd()
+ {
+ synchronized ( BackgroundCleaner.class )
+ {
+ if ( instance != null )
+ {
+ instance.doSessionEnd();
+ }
+ }
+ }
+
+ private BackgroundCleaner( Cleaner cleaner, File dir )
+ {
+ this.cleaner = cleaner;
+ init( cleaner.fastDir, dir );
+ }
+
+ 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 fastDir, File dir )
+ {
+ if ( fastDir.isDirectory() )
+ {
+ File[] children = fastDir.listFiles();
+ if ( children != null && children.length > 0 )
+ {
+ for ( File child : children )
+ {
+ doDelete( child );
+ }
+ }
+ }
+ doDelete( dir );
+ }
+
+ synchronized File pollNext()
+ {
+ File basedir = filesToDelete.poll();
+ if ( basedir == null )
+ {
+ if ( cleaner.session != null )
+ {
+ Map<String, Object> data =
cleaner.session.getRequest().getData();
+ File lastFolder = ( File ) data.remove(
LAST_DIRECTORY_TO_DELETE );
+ if ( lastFolder != null )
+ {
+ return lastFolder;
+ }
+ }
+ status = STOPPED;
+ notifyAll();
+ }
+ return basedir;
+ }
+
+ synchronized boolean doDelete( File dir )
+ {
+ if ( status == STOPPED )
+ {
+ return false;
+ }
+ filesToDelete.add( dir );
+ if ( status == NEW )
+ {
+ 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 );
Review comment:
same here
##########
File path: src/main/java/org/apache/maven/plugins/clean/Cleaner.java
##########
@@ -115,9 +135,102 @@ public void delete( File basedir, Selector selector,
boolean followSymlinks, boo
File file = followSymlinks ? basedir : basedir.getCanonicalFile();
+ if ( selector == null && !followSymlinks && fastDir != null )
+ {
+ // If anything wrong happens, we'll just use the usual deletion
mechanism
+ if ( fastDelete( file ) )
+ {
+ return;
+ }
+ }
+
delete( file, "", selector, followSymlinks, failOnError, retryOnError
);
}
+ private boolean fastDelete( File baseDir )
+ {
+ // Handle the case where we use
${maven.multiModuleProjectDirectory}/target/.clean for example
+ if ( fastDir.getAbsolutePath().startsWith( baseDir.getAbsolutePath() )
)
+ {
+ try
+ {
+ File tmpDir = createTempDir( baseDir.getParentFile(),
baseDir.getName() + "." );
+ try
+ {
+ Files.move( baseDir.toPath(), tmpDir.toPath(),
StandardCopyOption.ATOMIC_MOVE );
+ if ( session != null )
+ {
+ session.getRequest().getData().put(
LAST_DIRECTORY_TO_DELETE, baseDir );
+ }
+ 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
+ // 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 );
+ }
+ }
+ }
+ else
+ {
+ if ( logDebug != null )
+ {
+ logDebug.log( "Unable to fast delete directory as the path "
+ + fastDir + " does not point to a directory or can not
be created" );
Review comment:
cannot
--
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]
