gnodet commented on code in PR #362:
URL:
https://github.com/apache/maven-clean-plugin/pull/362#discussion_r4121910148
##########
src/main/java/org/apache/maven/plugins/clean/CleanMojo.java:
##########
@@ -276,7 +273,10 @@ public void execute() {
if (fast && session != null) {
Path tmpDir = fastDir;
if (tmpDir == null) {
- tmpDir =
session.getRootDirectory().resolve("target").resolve(".clean");
+ Path rootDir = session.getRootDirectory();
+ tmpDir = rootDir != null
+ ? rootDir.resolve("target").resolve(".clean")
+ :
Path.of(System.getProperty("java.io.tmpdir")).resolve(".clean");
Review Comment:
Fixed in 54971330ecfe1c4d20251dd150d57efd25e94a14 — replaced null check with
try/catch for IllegalStateException, since the API is @Nonnull but throws ISE
when root directory is not set.
##########
src/main/java/org/apache/maven/plugins/clean/CleanMojo.java:
##########
@@ -276,7 +273,10 @@ public void execute() {
if (fast && session != null) {
Path tmpDir = fastDir;
if (tmpDir == null) {
- tmpDir =
session.getRootDirectory().resolve("target").resolve(".clean");
+ Path rootDir = session.getRootDirectory();
+ tmpDir = rootDir != null
+ ? rootDir.resolve("target").resolve(".clean")
+ :
Path.of(System.getProperty("java.io.tmpdir")).resolve(".clean");
Review Comment:
Fixed in 54971330ecfe1c4d20251dd150d57efd25e94a14 — applied the suggested
try/catch pattern.
##########
src/main/java/org/apache/maven/plugins/clean/CleanMojo.java:
##########
@@ -182,24 +182,21 @@ public class CleanMojo implements
org.apache.maven.api.plugin.Mojo {
private boolean excludeDefaultDirectories;
/**
- * Enables fast clean if possible. If set to {@code true}, when the plugin
is executed, a directory to
- * be deleted will be atomically moved inside the {@code
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. If any problem occurs during
the atomic move of the directories,
- * the plugin will default to the traditional deletion mechanism.
+ * Enables fast clean. When set to {@code true}, each directory to be
deleted is first atomically moved
+ * inside the {@code maven.clean.fastDir} staging directory, immediately
freeing the original path, and
+ * the actual file deletion is then performed in the background. If an
atomic move is not supported
+ * (e.g. cross-device), the plugin falls back to immediate synchronous
deletion transparently.
*
- * <p>Note that for small projects with few files to delete, the "fast"
clean tends to be actually slower.
- * It is also more at risk that errors occurring during the deletion of a
file get unnoticed, or are noticed
- * late in the build process. This option should be used only when it has
been verified to be worth.</p>
+ * <p>This is the default mode as of 4.0.0: the atomic move is essentially
free, so even small projects
+ * benefit from the freed directory being available immediately. The
background deletion of the staging
+ * area does not block the build and any failure there does not affect
build correctness.</p>
Review Comment:
Fixed in 54971330ecfe1c4d20251dd150d57efd25e94a14 — qualified the statement
to distinguish failOnError=true (synchronous) vs failOnError=false (background)
behavior.
--
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]
