gnodet commented on code in PR #362:
URL:
https://github.com/apache/maven-clean-plugin/pull/362#discussion_r4159655133
##########
src/main/java/org/apache/maven/plugins/clean/CleanMojo.java:
##########
@@ -276,7 +275,15 @@ public void execute() {
if (fast && session != null) {
Path tmpDir = fastDir;
if (tmpDir == null) {
- tmpDir =
session.getRootDirectory().resolve("target").resolve(".clean");
+ Path rootDir;
+ try {
+ rootDir = session.getRootDirectory();
+ } catch (IllegalStateException e) {
+ rootDir = null;
+ }
+ tmpDir = rootDir != null
+ ? rootDir.resolve("target").resolve(".clean")
+ :
Path.of(System.getProperty("java.io.tmpdir")).resolve(".clean");
Review Comment:
Good catch on the `REPLACE_EXISTING` concern in principle, but it doesn't
apply here: the `tmpDir` from `CleanMojo` becomes `fastDir` in
`BackgroundCleaner`, which is the *parent* staging directory. `Files.move`
targets a unique subdirectory created inside it via
`Files.createTempDirectory(fastDir, prefix)` — so there's no collision risk and
`REPLACE_EXISTING` is already present anyway.
The simplification of the try/catch has been done in a follow-up PR: #364.
Note that in Maven 4, `getRootDirectory()` never throws during a standard
build, so the fallback is effectively unreachable from the CLI.
##########
src/main/java/org/apache/maven/plugins/clean/CleanMojo.java:
##########
@@ -149,10 +149,10 @@ public class CleanMojo implements
org.apache.maven.api.plugin.Mojo {
/**
* Indicates whether the build will continue even if there are clean
errors.
*
- * <p><b>Note:</b> when {@link #fast} is {@code true}, this parameter has
no effect on the
- * background deletion path. A session-end listener cannot structurally
fail the build in
- * Maven, so errors from background deletions are always logged as
warnings regardless of
- * this setting. Use {@code fast=false} if you need the build to fail on
clean errors.</p>
+ * <p>When {@link #fast} is {@code true} (the default) and this parameter
is {@code true} (the default),
+ * the deletion of each directory runs synchronously so that any error can
fail the build
Review Comment:
This was based on a stale version of the javadoc in the PR diff. In the
current code, deletion is never synchronous on the mojo thread: in `BACKGROUND`
mode (the default), directories are submitted to a background executor thread
immediately after the move; in `AT_END` mode they are queued and submitted to
the same executor at session end. The mojo thread never blocks waiting for
deletion to complete.
The purpose of the move is precisely to free the original path (e.g.
`target/`) atomically and instantly, so the new build can start writing there
while the background thread deletes the old content. That's also why
`failOnError` has no effect in fast mode — errors from the background thread
cannot be surfaced back to fail the build.
--
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]
