From 9085e2b95ba845eee9711215ce6d4ee13f641199 Mon Sep 17 00:00:00 2001 From: Martin Desruisseaux Date: Sun, 30 Mar 2025 16:06:54 +0200 Subject: [PATCH 1/3] Remove the dependency to Plexus, replaced by more reliance on `java.nio`. This commit contains the following work items: * Replacement of Plexus includes/excludes filters by `java.nio.file.PathMatcher`. One benefit is the support of different syntax, at least "glob" and "regex". If no syntax is specified, default to the "glob" syntax with modifications for reproducing the behavior of Plexus filters when it differs from "glob". * Use `java.nio.file.FileVisitor` for walking over files and directory trees. Consequently, the following of symbolic links is now handled by `FileVisitor` instead of by `maven-clean-plugin` itself. An advantage is that `FileVisitor` is safe against infinite loops when there is cycles in the symbolic links. Also, file attributes (whether the file is regular, a directory or a link) are queried only once per file. * Changes in some logging messages and exceptions. "Deleting XYZ" is replaced by either "Deleted XYZ" if the deletion has been successful, or replaced by "Failed to delete XYZ" in case of failure (i.e., "XYZ" is not logged twice in case of failure). * The `IOException` throws by Java is no longer wrapped in another `IOException`, so that the callers can catch an exception of the specific sub-type if desired. The exception is also thrown earlier, before it causes another exception. The difference can be seen in the tests: a deletion fails because unauthorized, but the error that was reported to the user was not the `AccessDeniedException`. Instead, it was a `DirectoryNotEmptyException` thrown when the plugin tried to delete the directory that contains the file that the plugin failed to delete. After this commit, the exception throws is the original `AccessDeniedException`. --- .../apache/maven/plugins/clean/CleanMojo.java | 33 +- .../apache/maven/plugins/clean/Cleaner.java | 456 ++++++++++-------- .../apache/maven/plugins/clean/Fileset.java | 61 ++- .../maven/plugins/clean/GlobSelector.java | 122 ----- .../apache/maven/plugins/clean/Selector.java | 320 +++++++++++- .../maven/plugins/clean/CleanMojoTest.java | 11 +- .../maven/plugins/clean/CleanerTest.java | 43 +- 7 files changed, 637 insertions(+), 409 deletions(-) delete mode 100644 src/main/java/org/apache/maven/plugins/clean/GlobSelector.java 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 67d3fe7..7f60747 100644 --- a/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java +++ b/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java @@ -222,7 +222,7 @@ public class CleanMojo implements org.apache.maven.api.plugin.Mojo { @Override public void execute() { if (skip) { - getLog().info("Clean is skipped."); + logger.info("Clean is skipped."); return; } @@ -236,7 +236,7 @@ public void execute() { } else { fastDir = null; if (fast) { - getLog().warn("Fast clean requires maven 3.3.1 or newer, " + logger.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' user property to be set."); } @@ -248,37 +248,22 @@ public void execute() { 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); - + final var cleaner = + new Cleaner(session, logger, isVerbose(), fastDir, fastMode, followSymLinks, failOnError, retryOnError); try { for (Path directoryItem : getDirectories()) { if (directoryItem != null) { - cleaner.delete(directoryItem, null, followSymLinks, failOnError, retryOnError); + cleaner.delete(directoryItem); } } - if (filesets != null) { for (Fileset fileset : filesets) { if (fileset.getDirectory() == null) { throw new MojoException("Missing base directory for " + fileset); } - final String[] includes = fileset.getIncludes(); - final String[] excludes = fileset.getExcludes(); - final boolean useDefaultExcludes = fileset.isUseDefaultExcludes(); - final GlobSelector selector; - if ((includes != null && includes.length != 0) - || (excludes != null && excludes.length != 0) - || useDefaultExcludes) { - selector = new GlobSelector(includes, excludes, useDefaultExcludes); - } else { - selector = null; - } - cleaner.delete( - fileset.getDirectory(), selector, fileset.isFollowSymlinks(), failOnError, retryOnError); + cleaner.delete(fileset); } } - } catch (IOException e) { throw new MojoException("Failed to clean project: " + e.getMessage(), e); } @@ -290,7 +275,7 @@ public void execute() { * @return true if verbose output is enabled, false otherwise. */ private boolean isVerbose() { - return (verbose != null) ? verbose : getLog().isDebugEnabled(); + return (verbose != null) ? verbose : logger.isDebugEnabled(); } /** @@ -307,8 +292,4 @@ private Path[] getDirectories() { } return directories; } - - private Log getLog() { - return logger; - } } 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 f19a9b5..ffac936 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Cleaner.java +++ b/src/main/java/org/apache/maven/plugins/clean/Cleaner.java @@ -20,15 +20,17 @@ import java.io.File; import java.io.IOException; +import java.nio.file.FileVisitOption; +import java.nio.file.FileVisitResult; +import java.nio.file.FileVisitor; import java.nio.file.Files; -import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayDeque; +import java.util.BitSet; import java.util.Deque; -import java.util.function.BiConsumer; -import java.util.function.Consumer; +import java.util.EnumSet; import java.util.stream.Stream; import org.apache.maven.api.Event; @@ -36,121 +38,173 @@ import org.apache.maven.api.Listener; import org.apache.maven.api.Session; import org.apache.maven.api.SessionData; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; import org.apache.maven.api.plugin.Log; -import org.codehaus.plexus.util.Os; - -import static org.apache.maven.plugins.clean.CleanMojo.FAST_MODE_BACKGROUND; -import static org.apache.maven.plugins.clean.CleanMojo.FAST_MODE_DEFER; /** * Cleans directories. * * @author Benjamin Bentmann + * @author Martin Desruisseaux */ -class Cleaner { +final class Cleaner implements FileVisitor { - private static final boolean ON_WINDOWS = Os.isFamily(Os.FAMILY_WINDOWS); + private static final boolean ON_WINDOWS = (File.separatorChar == '\\'); private static final SessionData.Key LAST_DIRECTORY_TO_DELETE = SessionData.key(Path.class, Cleaner.class.getName() + ".lastDirectoryToDelete"); /** - * The maven session. This is typically non-null in a real run, but it can be during unit tests. + * The maven session. This is typically non-null in a real run, but it can be during unit tests. */ + @Nonnull private final Session session; - private final Logger logDebug; - - private final Logger logInfo; + /** + * The logger where to send information or warning messages. + */ + @Nonnull + private final Log logger; - private final Logger logVerbose; + /** + * Whether to send to the logger some information that would normally be at the "debug" level. + * Those information include the files or directories that are deleted. + */ + private final boolean verbose; - private final Logger logWarn; + /** + * Whether to list the files or directories that are deleted. This is a combination of {@link #verbose} + * with {@link #logger} configuration, and is stored because frequently requested. + */ + private final boolean listDeletedFiles; + @Nonnull private final Path fastDir; + @Nonnull private final String fastMode; + @Nullable + private Selector selector; + + /** + * Whether the base directory is excluded from the set of directories to delete. + * This is usually {@code false}, unless explicitly excluded by specifying an + * empty string in the excludes. + */ + private boolean isBaseDirectoryExcluded; + + private boolean followSymlinks; + + private final boolean failOnError; + + private final boolean retryOnError; + + /** + * Number of files that we failed to delete. + * This is incremented only if {@link #failOnError} is {@code false}, otherwise exceptions are thrown. + */ + private int failureCount; + + /** + * Whether each directory level contains at least one excluded file. + * This is used for determining whether the directory can be deleted. + * A bit is used for each directory level, with {@link #currentDepth} + * telling which bit is for the current directory. + */ + private final BitSet nonEmptyDirectoryLevels; + + /** + * 0 for the base directory, and incremented for each subdirectory. + */ + private int currentDepth; + /** * Creates a new cleaner. * * @param session the Maven session to be used - * @param log the logger to use, may be null to disable logging + * @param logger the logger to use * @param verbose whether to perform verbose logging * @param fastDir the explicit configured directory or to be deleted in fast mode * @param fastMode the fast deletion mode + * @param followSymlinks whether to follow symlinks + * @param failOnError whether to abort with an exception in case a selected file/directory could not be deleted + * @param retryOnError whether to undertake additional delete attempts in case the first attempt failed */ - Cleaner(Session session, Log log, boolean verbose, Path fastDir, String fastMode) { - logDebug = (log == null || !log.isDebugEnabled()) ? null : logger(log::debug, log::debug); - - logInfo = (log == null || !log.isInfoEnabled()) ? null : logger(log::info, log::info); - - logWarn = (log == null || !log.isWarnEnabled()) ? null : logger(log::warn, log::warn); - - logVerbose = verbose ? logInfo : logDebug; - + @SuppressWarnings("checkstyle:ParameterNumber") + Cleaner( + @Nonnull Session session, + @Nonnull Log logger, + boolean verbose, + @Nonnull Path fastDir, + @Nonnull String fastMode, + boolean followSymlinks, + boolean failOnError, + boolean retryOnError) { this.session = session; + this.logger = logger; + this.verbose = verbose; this.fastDir = fastDir; this.fastMode = fastMode; + this.followSymlinks = followSymlinks; + this.failOnError = failOnError; + this.retryOnError = retryOnError; + listDeletedFiles = verbose ? logger.isInfoEnabled() : logger.isDebugEnabled(); + nonEmptyDirectoryLevels = new BitSet(); } - private Logger logger(Consumer l1, BiConsumer l2) { - return new Logger() { - @Override - public void log(CharSequence message) { - l1.accept(message); - } - - @Override - public void log(CharSequence message, Throwable t) { - l2.accept(message, t); - } - }; + /** + * Deletes the specified fileset. + * + * @param fileset the fileset to delete, must not be {@code null} + * @throws IOException if a file/directory could not be deleted and {@code failOnError} is {@code true} + */ + public void delete(@Nonnull Fileset fileset) throws IOException { + selector = new Selector(fileset); + if (selector.isEmpty()) { + selector = null; + } + isBaseDirectoryExcluded = fileset.isBaseDirectoryExcluded(); + followSymlinks = fileset.isFollowSymlinks(); + delete(fileset.getDirectory()); } /** * Deletes the specified directories and its contents. + * Non-existing directories will be silently ignored. * - * @param basedir the directory to delete, must not be null. Non-existing directories will be silently - * ignored - * @param selector the selector used to determine what contents to delete, may be null to delete - * everything - * @param followSymlinks whether to follow symlinks - * @param failOnError whether to abort with an exception in case a selected file/directory could not be deleted - * @param retryOnError whether to undertake additional delete attempts in case the first attempt failed - * @throws IOException if a file/directory could not be deleted and failOnError is true + * @param basedir the directory to delete, must not be {@code null} + * @throws IOException if a file/directory could not be deleted and {@code failOnError} is {@code true} */ - public void delete( - Path basedir, Selector selector, boolean followSymlinks, boolean failOnError, boolean retryOnError) - throws IOException { + public void delete(Path basedir) throws IOException { if (!Files.isDirectory(basedir)) { - if (!Files.exists(basedir)) { - if (logDebug != null) { - logDebug.log("Skipping non-existing directory " + basedir); + if (Files.notExists(basedir)) { + if (logger.isDebugEnabled()) { + logger.debug("Skipping non-existing directory " + basedir); } return; } throw new IOException("Invalid base directory " + basedir); } - - if (logInfo != null) { - logInfo.log("Deleting " + basedir + (selector != null ? " (" + selector + ")" : "")); + if (logger.isInfoEnabled()) { + logger.info("Deleting " + basedir + (selector != null ? " (" + selector + ")" : "")); + } + var options = EnumSet.noneOf(FileVisitOption.class); + if (followSymlinks) { + options.add(FileVisitOption.FOLLOW_LINKS); + basedir = getCanonicalPath(basedir); } - - Path file = followSymlinks ? basedir : getCanonicalPath(basedir); - if (selector == null && !followSymlinks && fastDir != null && session != null) { // If anything wrong happens, we'll just use the usual deletion mechanism - if (fastDelete(file)) { + if (fastDelete(basedir)) { return; } } - - delete(file, "", selector, followSymlinks, failOnError, retryOnError); + Files.walkFileTree(basedir, options, Integer.MAX_VALUE, this); } private boolean fastDelete(Path baseDir) { - Path fastDir = this.fastDir; // Handle the case where we use ${maven.multiModuleProjectDirectory}/target/.clean for example if (fastDir.toAbsolutePath().startsWith(baseDir.toAbsolutePath())) { try { @@ -167,9 +221,7 @@ private boolean fastDelete(Path baseDir) { throw e; } } catch (IOException e) { - if (logDebug != null) { - logDebug.log("Unable to fast delete directory", e); - } + logger.debug("Unable to fast delete directory", e); return false; } } @@ -179,15 +231,12 @@ private boolean fastDelete(Path baseDir) { Files.createDirectories(fastDir); } } catch (IOException e) { - if (logDebug != null) { - logDebug.log( - "Unable to fast delete directory as the path " + fastDir - + " does not point to a directory or cannot be created", - e); - } + logger.debug( + "Unable to fast delete directory as the path " + fastDir + + " does not point to a directory or cannot be created", + e); return false; } - try { Path tmpDir = Files.createTempDirectory(fastDir, ""); Path dstDir = tmpDir.resolve(baseDir.getFileName()); @@ -199,81 +248,89 @@ private boolean fastDelete(Path baseDir) { BackgroundCleaner.delete(this, tmpDir, fastMode); return true; } catch (IOException e) { - if (logDebug != null) { - logDebug.log("Unable to fast delete directory", e); - } + logger.debug("Unable to fast delete directory", e); return false; } } /** - * Deletes the specified file or directory. - * - * @param file the file/directory to delete, must not be null. If followSymlinks is - * false, it is assumed that the parent file is canonical - * @param pathname the relative pathname of the file, using {@link File#separatorChar}, must not be - * null - * @param selector the selector used to determine what contents to delete, may be null to delete - * everything - * @param followSymlinks whether to follow symlinks - * @param failOnError whether to abort with an exception in case a selected file/directory could not be deleted - * @param retryOnError whether to undertake additional delete attempts in case the first attempt failed - * @return The result of the cleaning, never null - * @throws IOException if a file/directory could not be deleted and failOnError is true + * Invoked for a directory before entries in the directory are visited. + * Determines if the given directory should be scanned for files to delete. */ - private Result delete( - Path file, - String pathname, - Selector selector, - boolean followSymlinks, - boolean failOnError, - boolean retryOnError) - throws IOException { - Result result = new Result(); - - boolean isDirectory = Files.isDirectory(file); - - if (isDirectory) { - if (selector == null || selector.couldHoldSelected(pathname)) { - final boolean isSymlink = isSymbolicLink(file); - Path canonical = followSymlinks ? file : getCanonicalPath(file); - if (followSymlinks || !isSymlink) { - String prefix = !pathname.isEmpty() ? pathname + File.separatorChar : ""; - try (Stream children = Files.list(canonical)) { - for (Path child : children.toList()) { - result.update(delete( - child, - prefix + child.getFileName(), - selector, - followSymlinks, - failOnError, - retryOnError)); - } - } - } else if (logDebug != null) { - logDebug.log("Not recursing into symlink " + file); - } - } else if (logDebug != null) { - logDebug.log("Not recursing into directory without included files " + file); - } + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + if (selector == null || selector.couldHoldSelected(dir)) { + nonEmptyDirectoryLevels.clear(++currentDepth); + return FileVisitResult.CONTINUE; + } else { + nonEmptyDirectoryLevels.set(currentDepth); // Remember that this directory is not empty. + logger.debug("Not recursing into directory without included files " + dir); + return FileVisitResult.SKIP_SUBTREE; } + } - if (!result.excluded && (selector == null || selector.isSelected(pathname))) { - if (logVerbose != null) { - if (isDirectory) { - logVerbose.log("Deleting directory " + file); - } else if (Files.exists(file)) { - logVerbose.log("Deleting file " + file); - } else { - logVerbose.log("Deleting dangling symlink " + file); - } + /** + * Invoked for a file in a directory. + * Deletes that file, unless the file is excluded by the selector. + */ + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + if ((selector == null || selector.matches(file)) && tryDelete(file)) { + if (listDeletedFiles) { + logDelete(file, attrs); } - result.failures += delete(file, failOnError, retryOnError); } else { - result.excluded = true; + nonEmptyDirectoryLevels.set(currentDepth); // Remember that the directory will not be empty. + } + return FileVisitResult.CONTINUE; + } + + /** + * Invoked for a file that could not be visited. + */ + @Override + public FileVisitResult visitFileFailed(Path file, IOException failure) throws IOException { + if (logger.isWarnEnabled()) { + logger.warn("Failed to visit " + file, failure); + } + failureCount++; + nonEmptyDirectoryLevels.set(currentDepth); // Remember that the directory will not be empty. + if (failOnError) { + throw failure; } + return FileVisitResult.CONTINUE; + } - return result; + /** + * Invoked for a directory after all files and sub-trees have been visited. + * If the directory is not empty, then this method deletes it. + */ + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException failure) throws IOException { + boolean canDelete = !nonEmptyDirectoryLevels.get(currentDepth--); // False if the directory is not empty. + if (failure != null) { + if (logger.isWarnEnabled()) { + logger.warn("Error in directory " + dir, failure); + } + failureCount++; + canDelete = false; + } else { + canDelete &= (currentDepth != 0 || !isBaseDirectoryExcluded); + if (canDelete && selector != null) { + canDelete = selector.matches(dir); + } + } + if (canDelete) { + if (tryDelete(dir) && listDeletedFiles) { + logDelete(dir, null); + } + } else { + nonEmptyDirectoryLevels.set(currentDepth); // Remember that the parent of `dir` is not empty. + } + if (failure != null && failOnError) { + throw failure; + } + return FileVisitResult.CONTINUE; } private static Path getCanonicalPath(Path path) { @@ -284,90 +341,78 @@ private static Path getCanonicalPath(Path path) { } } - private boolean isSymbolicLink(Path path) throws IOException { - BasicFileAttributes attrs = Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); - return attrs.isSymbolicLink() - // MCLEAN-93: NTFS junctions have isDirectory() and isOther() attributes set - || (attrs.isDirectory() && attrs.isOther()); - } - /** - * Deletes the specified file or directory. If the path denotes a symlink, only the link is removed. Its target is - * left untouched. + * Deletes the specified file or directory. + * If the path denotes a symlink, only the link is removed. Its target is left untouched. + * This method returns {@code true} if the file has been deleted, or {@code false} if the + * file does not exist or if an {@link IOException} occurred but {@link #failOnError} is + * {@code false}. * - * @param file the file/directory to delete, must not be null - * @param failOnError whether to abort with an exception if the file/directory could not be deleted - * @param retryOnError whether to undertake additional delete attempts if the first attempt failed - * @return 0 if the file was deleted, 1 otherwise - * @throws IOException if a file/directory could not be deleted and failOnError is true + * @param file the file/directory to delete, must not be {@code null} + * @return whether the file has been deleted + * @throws IOException if a file/directory could not be deleted and {@code failOnError} is {@code true} */ - private int delete(Path file, boolean failOnError, boolean retryOnError) throws IOException { - IOException failure = delete(file); - if (failure != null) { - + @SuppressWarnings("SleepWhileInLoop") + private boolean tryDelete(final Path file) throws IOException { + try { + return Files.deleteIfExists(file); + } catch (IOException failure) { if (retryOnError) { if (ON_WINDOWS) { // try to release any locks held by non-closed files System.gc(); } - - final int[] delays = {50, 250, 750}; - for (int delay : delays) { + for (int delay : new int[] {50, 250, 750}) { try { Thread.sleep(delay); } catch (InterruptedException e) { - throw new IOException(e); + failure.addSuppressed(e); + throw failure; } - failure = delete(file); - if (failure == null) { - break; + try { + return Files.deleteIfExists(file); + } catch (IOException again) { + again.addSuppressed(failure); + failure = again; } } } - - if (Files.exists(file)) { - if (failOnError) { - throw new IOException("Failed to delete " + file, failure); - } else { - if (logWarn != null) { - logWarn.log("Failed to delete " + file, failure); - } - return 1; - } + if (logger.isWarnEnabled()) { + logger.warn("Failed to delete " + file, failure); + } + failureCount++; + if (failOnError) { + throw failure; } + return false; } - - return 0; } - private static IOException delete(Path file) { - try { - Files.deleteIfExists(file); - } catch (IOException e) { - return e; + /** + * Reports that a file, directory or symbolic link has been deleted. This method should be invoked only + * when {@link #tryDelete(Path)} returned {@code true} and {@link #listDeletedFiles} is {@code true}. + * + *

If {@code attrs} is {@code null}, then the file is assumed a directory. This arbitrary rule + * is an implementation convenience specific to the context in which we invoke this method.

+ */ + private void logDelete(final Path file, final BasicFileAttributes attrs) { + String message; + if (attrs == null || attrs.isDirectory()) { + message = "Deleted directory " + file; + } else if (attrs.isRegularFile()) { + message = "Deleted file " + file; + } else if (attrs.isSymbolicLink()) { + message = "Deleted dangling symlink " + file; + } else { + message = "Deleted " + file; } - return null; - } - - private static class Result { - - private int failures; - - private boolean excluded; - - public void update(Result result) { - failures += result.failures; - excluded |= result.excluded; + if (verbose) { + logger.info(message); + } else { + logger.debug(message); } } - private interface Logger { - - void log(CharSequence message); - - void log(CharSequence message, Throwable t); - } - private static class BackgroundCleaner extends Thread { private static BackgroundCleaner instance; @@ -409,13 +454,14 @@ private BackgroundCleaner(Cleaner cleaner, Path dir, String fastMode) { @Override public void run() { - while (true) { - Path basedir = pollNext(); - if (basedir == null) { - break; - } + var options = EnumSet.noneOf(FileVisitOption.class); + if (cleaner.followSymlinks) { + options.add(FileVisitOption.FOLLOW_LINKS); + } + Path basedir; + while ((basedir = pollNext()) != null) { try { - cleaner.delete(basedir, "", null, false, false, true); + Files.walkFileTree(basedir, options, Integer.MAX_VALUE, cleaner); } catch (IOException e) { // do not display errors } @@ -457,7 +503,7 @@ synchronized boolean doDelete(Path dir) { return false; } filesToDelete.add(dir); - if (status == NEW && FAST_MODE_BACKGROUND.equals(fastMode)) { + if (status == NEW && CleanMojo.FAST_MODE_BACKGROUND.equals(fastMode)) { status = RUNNING; notifyAll(); start(); @@ -486,11 +532,9 @@ synchronized void doSessionEnd() { if (status == NEW) { start(); } - if (!FAST_MODE_DEFER.equals(fastMode)) { + if (!CleanMojo.FAST_MODE_DEFER.equals(fastMode)) { try { - if (cleaner.logInfo != null) { - cleaner.logInfo.log("Waiting for background file deletion"); - } + cleaner.logger.info("Waiting for background file deletion"); while (status != STOPPED) { wait(); } diff --git a/src/main/java/org/apache/maven/plugins/clean/Fileset.java b/src/main/java/org/apache/maven/plugins/clean/Fileset.java index 90d15c5..2842ec6 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Fileset.java +++ b/src/main/java/org/apache/maven/plugins/clean/Fileset.java @@ -19,11 +19,10 @@ package org.apache.maven.plugins.clean; import java.nio.file.Path; -import java.util.Arrays; /** * Customizes the string representation of - * org.apache.maven.shared.model.fileset.FileSet to return the + * {@code org.apache.maven.shared.model.fileset.FileSet} to return the * included and excluded files from the file-set's directory. Specifically, * "file-set: [directory] (included: [included files], * excluded: [excluded files])" @@ -43,53 +42,87 @@ public class Fileset { private boolean useDefaultExcludes; /** - * @return {@link #directory} + * {@return the base directory}. */ public Path getDirectory() { return directory; } /** - * @return {@link #includes} + * {@return the patterns of the file to include, or an empty array if unspecified}. */ public String[] getIncludes() { return (includes != null) ? includes : new String[0]; } /** - * @return {@link #excludes} + * {@return the patterns of the file to exclude, or an empty array if unspecified}. */ public String[] getExcludes() { return (excludes != null) ? excludes : new String[0]; } /** - * @return {@link #followSymlinks} + * {@return whether the base directory is excluded from the fileset}. + * This is {@code false} by default. The base directory can be excluded + * explicitly if the exclude patterns contains an empty string. + */ + public boolean isBaseDirectoryExcluded() { + if (excludes != null) { + for (String pattern : excludes) { + if (pattern == null || pattern.isEmpty()) { + return true; + } + } + } + return false; + } + + /** + * {@return whether to follow symbolic links}. */ public boolean isFollowSymlinks() { return followSymlinks; } /** - * @return {@link #useDefaultExcludes} + * {@return whether to use a default set of excludes}. */ public boolean isUseDefaultExcludes() { return useDefaultExcludes; } /** - * Retrieves the included and excluded files from this file-set's directory. - * Specifically, "file-set: [directory] (included: - * [included files], excluded: [excluded files])" + * Appends the elements of the given array in the given buffer. + * This is a helper method for {@link #toString()} implementations. * - * @return The included and excluded files from this file-set's directory. + * @param buffer the buffer where to add the elements + * @param label label identifying the array of elements to add + * @param patterns the elements to append, or {@code null} if none + */ + static void append(StringBuilder buffer, String label, String[] patterns) { + buffer.append(label).append(": ["); + if (patterns != null) { + for (int i = 0; i < patterns.length; i++) { + if (i != 0) { + buffer.append(", "); + } + buffer.append(patterns[i]); + } + } + buffer.append(']'); + } + + /** + * {@return a string representation of the included and excluded files from this file-set's directory}. * Specifically, "file-set: [directory] (included: * [included files], excluded: [excluded files])" - * @see java.lang.Object#toString() */ @Override public String toString() { - return "file set: " + getDirectory() + " (included: " + Arrays.asList(getIncludes()) + ", excluded: " - + Arrays.asList(getExcludes()) + ")"; + var buffer = new StringBuilder("file set: ").append(getDirectory()); + append(buffer.append(" ("), "included", getIncludes()); + append(buffer.append(", "), "excluded", getExcludes()); + return buffer.append(')').toString(); } } diff --git a/src/main/java/org/apache/maven/plugins/clean/GlobSelector.java b/src/main/java/org/apache/maven/plugins/clean/GlobSelector.java deleted file mode 100644 index 3d61071..0000000 --- a/src/main/java/org/apache/maven/plugins/clean/GlobSelector.java +++ /dev/null @@ -1,122 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.maven.plugins.clean; - -import java.io.File; -import java.util.Arrays; - -import org.codehaus.plexus.util.DirectoryScanner; -import org.codehaus.plexus.util.SelectorUtils; - -/** - * Selects paths based on Ant-like glob patterns. - * - * @author Benjamin Bentmann - */ -class GlobSelector implements Selector { - - private final String[] includes; - - private final String[] excludes; - - private final String str; - - GlobSelector(String[] includes, String[] excludes, boolean useDefaultExcludes) { - this.str = "includes = " + toString(includes) + ", excludes = " + toString(excludes); - this.includes = normalizePatterns(includes); - this.excludes = normalizePatterns(addDefaultExcludes(excludes, useDefaultExcludes)); - } - - private static String toString(String[] patterns) { - return (patterns == null) ? "[]" : Arrays.asList(patterns).toString(); - } - - private static String[] addDefaultExcludes(String[] excludes, boolean useDefaultExcludes) { - String[] defaults = DirectoryScanner.DEFAULTEXCLUDES; - if (!useDefaultExcludes) { - return excludes; - } else if (excludes == null || excludes.length <= 0) { - return defaults; - } else { - String[] patterns = new String[excludes.length + defaults.length]; - System.arraycopy(excludes, 0, patterns, 0, excludes.length); - System.arraycopy(defaults, 0, patterns, excludes.length, defaults.length); - return patterns; - } - } - - private static String[] normalizePatterns(String[] patterns) { - String[] normalized; - - if (patterns != null) { - normalized = new String[patterns.length]; - for (int i = patterns.length - 1; i >= 0; i--) { - normalized[i] = normalizePattern(patterns[i]); - } - } else { - normalized = new String[0]; - } - - return normalized; - } - - private static String normalizePattern(String pattern) { - if (pattern == null) { - return ""; - } - - String normalized = pattern.replace((File.separatorChar == '/') ? '\\' : '/', File.separatorChar); - - if (normalized.endsWith(File.separator)) { - normalized += "**"; - } - - return normalized; - } - - @Override - public boolean isSelected(String pathname) { - return (includes.length <= 0 || isMatched(pathname, includes)) - && (excludes.length <= 0 || !isMatched(pathname, excludes)); - } - - private static boolean isMatched(String pathname, String[] patterns) { - for (String pattern : patterns) { - if (SelectorUtils.matchPath(pattern, pathname)) { - return true; - } - } - return false; - } - - @Override - public boolean couldHoldSelected(String pathname) { - for (String include : includes) { - if (SelectorUtils.matchPatternStart(include, pathname)) { - return true; - } - } - return includes.length <= 0; - } - - @Override - public String toString() { - return str; - } -} diff --git a/src/main/java/org/apache/maven/plugins/clean/Selector.java b/src/main/java/org/apache/maven/plugins/clean/Selector.java index 93a0695..3810506 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Selector.java +++ b/src/main/java/org/apache/maven/plugins/clean/Selector.java @@ -18,28 +18,328 @@ */ package org.apache.maven.plugins.clean; +import java.io.File; +import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.util.LinkedHashSet; +import java.util.Set; + +import org.codehaus.plexus.util.DirectoryScanner; + /** - * Determines whether a path is selected for deletion. The pathnames used for method parameters will be relative to some - * base directory and use {@link java.io.File#separatorChar} as separator. + * Determines whether a path is selected for deletion. + * The pathnames used for method parameters will be relative to some base directory + * and use {@code '/'} as separator, regardless the hosting operating system. + * + *

Syntax

+ * If a pattern contains the {@code ':'} character, then that pattern is given verbatim to + * {@link FileSystem#getPathMatcher(String)}, which will interpret the part before {@code ':'} + * as the syntax (usually {@code "glob"} or {@code "regex"}). If a pattern does not contain the + * {@code ':'} character, then the syntax defaults to a reproduction of the Maven 3 behavior. + * This is implemented as the {@code "glob"} syntax with the following modifications: + * + *
    + *
  • The platform-specific separator ({@code '\\'} on Windows) is replaced by {@code '/'}. + * Note that it means that the backslash cannot be used for escaping characters.
    • + *
  • Trailing {@code "/"} is completed as {@code "/**"}.
    • + *
  • The {@code "**"} wildcard means "0 or more directories" instead of "1 or more directories". + * This is implemented by adding variants of the pattern without the {@code "**"} wildcard.
    • + *
+ * + * If above changes are not desired, put an explicit {@code "glob:"} prefix before the patterns. + * Note that putting such prefix is recommended anyway for better performances. * * @author Benjamin Bentmann + * @author Martin Desruisseaux */ -interface Selector { +final class Selector implements PathMatcher { + /** + * String representation of the normalized include filters. + * This is kept only for {@link #toString()} implementation. + */ + private final String[] includePatterns; + + /** + * String representation of the normalized exclude filters. + * This is kept only for {@link #toString()} implementation. + */ + private final String[] excludePatterns; + + /** + * The matcher for includes. The length of this array is equal to {@link #includePatterns} array length. + */ + private final PathMatcher[] includes; + + /** + * The matcher for excludes. The length of this array is equal to {@link #excludePatterns} array length. + */ + private final PathMatcher[] excludes; + + /** + * The matcher for all directories to include. This array includes the parents of all those directories, + * because they need to be accepted before we can walk to the sub-directories. + * This is an optimization for skipping whole directories when possible. + */ + private final PathMatcher[] dirIncludes; + + /** + * The matcher for directories to exclude. This array does not include the parent directories, + * since they may contain other sub-trees that need to be included. + * This is an optimization for skipping whole directories when possible. + */ + private final PathMatcher[] dirExcludes; + + /** + * The base directory. All files will be relativized to that directory before to be matched. + */ + private final Path baseDirectory; + + /** + * Creates a new selector from the given file seT. + * + * @param fs the user-specified configuration + */ + Selector(Fileset fs) { + includePatterns = normalizePatterns(fs.getIncludes(), false); + excludePatterns = normalizePatterns(addDefaultExcludes(fs.getExcludes(), fs.isUseDefaultExcludes()), true); + baseDirectory = fs.getDirectory(); + FileSystem system = baseDirectory.getFileSystem(); + includes = matchers(system, includePatterns); + excludes = matchers(system, excludePatterns); + dirIncludes = matchers(system, directoryPatterns(includePatterns, false)); + dirExcludes = matchers(system, directoryPatterns(excludePatterns, true)); + } + + /** + * Returns the given array of excludes, optionally expanded with a default set of excludes. + * + * @param excludes the user-specified excludes. + * @param useDefaultExcludes whether to expand user exclude with the set of default excludes + * @return the potentially expanded set of excludes to use + */ + private static String[] addDefaultExcludes(final String[] excludes, final boolean useDefaultExcludes) { + if (!useDefaultExcludes) { + return excludes; + } + String[] defaults = DirectoryScanner.DEFAULTEXCLUDES; + if (excludes == null || excludes.length == 0) { + return defaults; + } else { + String[] patterns = new String[excludes.length + defaults.length]; + System.arraycopy(excludes, 0, patterns, 0, excludes.length); + System.arraycopy(defaults, 0, patterns, excludes.length, defaults.length); + return patterns; + } + } + + /** + * Returns the given array of patterns with path separator normalized to {@code '/'}. + * Null or empty patterns are ignored, and duplications are removed. + * + * @param patterns the patterns to normalize + * @param excludes whether the patterns are exclude patterns + * @return normalized patterns without null, empty or duplicated patterns + */ + private static String[] normalizePatterns(final String[] patterns, final boolean excludes) { + if (patterns == null) { + return new String[0]; + } + // TODO: use `LinkedHashSet.newLinkedHashSet(int)` instead with JDK19. + final var normalized = new LinkedHashSet(patterns.length); + for (String pattern : patterns) { + if (pattern != null && !pattern.isEmpty()) { + final boolean useMavenSyntax = pattern.indexOf(':') < 0; + if (useMavenSyntax) { + pattern = pattern.replace(File.separatorChar, '/'); + if (pattern.endsWith("/")) { + pattern += "**"; + } + // Following are okay only when "**" means "0 or more directories". + while (pattern.endsWith("/**/**")) { + pattern = pattern.substring(0, pattern.length() - 3); + } + while (pattern.startsWith("**/**/")) { + pattern = pattern.substring(3); + } + pattern = pattern.replace("/**/**/", "/**/"); + } + normalized.add(pattern); + /* + * If the pattern starts or ends with "**", Java GLOB expects a directory level at + * that location while Maven seems to consider that "**" can mean "no directory". + * Add another pattern for reproducing this effect. + */ + if (useMavenSyntax) { + addPatternsWithOneDirRemoved(normalized, pattern, 0); + } + } + } + return simplify(normalized, excludes); + } + + /** + * Adds all variants of the given pattern with {@code **} removed. + * This is used for simulating the Maven behavior where {@code "**} may match zero directory. + * Tests suggest that we need an explicit GLOB pattern with no {@code "**"} for matching an absence of directory. + * + * @param patterns where to add the derived patterns + * @param pattern the pattern for which to add derived forms + * @param end should be 0 (reserved for recursive invocations of this method) + */ + private static void addPatternsWithOneDirRemoved(final Set patterns, final String pattern, int end) { + final int length = pattern.length(); + int start; + while ((start = pattern.indexOf("**", end)) >= 0) { + end = start + 2; // 2 is the length of "**". + if (end < length) { + if (pattern.charAt(end) != '/') { + continue; + } + if (start == 0) { + end++; // Ommit the leading slash if there is nothing before it. + } + } + if (start > 0) { + if (pattern.charAt(--start) != '/') { + continue; + } + } + String reduced = pattern.substring(0, start) + pattern.substring(end); + patterns.add(reduced); + addPatternsWithOneDirRemoved(patterns, reduced, start); + } + } + + /** + * Applies some heuristic rules for simplifying the set of patterns, + * then returns the patterns as an array. + * + * @param patterns the patterns to simplify and return asarray + * @param excludes whether the patterns are exclude patterns + * @return the set content as an array, after simplification + */ + private static String[] simplify(Set patterns, boolean excludes) { + /* + * If the "**" pattern is present, it makes all other patterns useless. + * In the case of include patterns, an empty set means to include everything. + */ + if (patterns.remove("**")) { + patterns.clear(); + if (excludes) { + patterns.add("**"); + } + } + return patterns.toArray(String[]::new); + } + + /** + * Eventually adds the parent directory of the given patterns, without duplicated values. + * The patterns given to this method should have been normalized. + * + * @param patterns the normalized include or exclude patterns + * @param excludes whether the patterns are exclude patterns + * @return pattens of directories to include or exclude + */ + private static String[] directoryPatterns(final String[] patterns, final boolean excludes) { + // TODO: use `LinkedHashSet.newLinkedHashSet(int)` instead with JDK19. + final var directories = new LinkedHashSet(patterns.length); + for (String pattern : patterns) { + int s = pattern.indexOf(':'); + if (s < 0 || pattern.startsWith("glob:")) { + if (excludes) { + if (pattern.endsWith("/**")) { + directories.add(pattern.substring(0, pattern.length() - 3)); + } + } else { + if (pattern.regionMatches(++s, "**/", 0, 3)) { + s = pattern.indexOf('/', s + 3); + if (s < 0) { + return new String[0]; // Pattern is "**", so we need to accept everything. + } + directories.add(pattern.substring(0, s)); + } + } + } + } + return simplify(directories, excludes); + } + + /** + * Creates the path matchers for the given patterns. + * If no syntax is specified, the default is {@code glob}. + */ + private static PathMatcher[] matchers(final FileSystem fs, final String[] patterns) { + final var matchers = new PathMatcher[patterns.length]; + for (int i = 0; i < patterns.length; i++) { + String pattern = patterns[i]; + if (pattern.indexOf(':') < 0) { + pattern = "glob:" + pattern; + } + matchers[i] = fs.getPathMatcher(pattern); + } + return matchers; + } + + /** + * {@return whether there is no include or exclude filters}. + * In such case, this {@code Selector} instance should be ignored. + */ + boolean isEmpty() { + return (includes.length | excludes.length) == 0; + } /** * Determines whether a path is selected for deletion. + * This is true if the given file matches an include pattern and no exclude pattern. * - * @param pathname The pathname to test, must not be null. - * @return true if the given path is selected for deletion, false otherwise. + * @param pathname The pathname to test, must not be {@code null} + * @return {@code true} if the given path is selected for deletion, {@code false} otherwise + */ + @Override + public boolean matches(Path path) { + path = baseDirectory.relativize(path); + return (includes.length == 0 || isMatched(path, includes)) + && (excludes.length == 0 || !isMatched(path, excludes)); + } + + /** + * {@return whether the given file matches according one of the given matchers}. */ - boolean isSelected(String pathname); + private static boolean isMatched(Path path, PathMatcher[] matchers) { + for (PathMatcher matcher : matchers) { + if (matcher.matches(path)) { + return true; + } + } + return false; + } /** * Determines whether a directory could contain selected paths. * - * @param pathname The directory pathname to test, must not be null. - * @return true if the given directory might contain selected paths, false if the - * directory will definitively not contain selected paths.. + * @param directory the directory pathname to test, must not be {@code null} + * @return {@code true} if the given directory might contain selected paths, {@code false} if the + * directory will definitively not contain selected paths + */ + public boolean couldHoldSelected(Path directory) { + if (baseDirectory.equals(directory)) { + return true; + } + directory = baseDirectory.relativize(directory); + return (dirIncludes.length == 0 || isMatched(directory, dirIncludes)) + && (dirExcludes.length == 0 || !isMatched(directory, dirExcludes)); + } + + /** + * {@return a string representation for logging purposes}. + * This string representation is reported logged when {@link Cleaner} is executed. */ - boolean couldHoldSelected(String pathname); + @Override + public String toString() { + var buffer = new StringBuilder(); + Fileset.append(buffer, "includes", includePatterns); + Fileset.append(buffer.append(", "), "excludes", excludePatterns); + return buffer.toString(); + } } diff --git a/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java b/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java index 02e8bf6..ad6d64e 100644 --- a/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java @@ -29,6 +29,7 @@ import java.nio.file.Paths; import java.util.Collections; +import org.apache.maven.api.plugin.Log; import org.apache.maven.api.plugin.MojoException; import org.apache.maven.api.plugin.testing.Basedir; import org.apache.maven.api.plugin.testing.InjectMojo; @@ -45,6 +46,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; /** * Test the clean mojo. @@ -52,6 +54,8 @@ @MojoTest public class CleanMojoTest { + private final Log log = mock(Log.class); + /** * Tests the simple removal of directories * @@ -241,7 +245,7 @@ interface LinkCreator { } private void testSymlink(LinkCreator linkCreator) throws Exception { - Cleaner cleaner = new Cleaner(null, null, false, null, null); + Cleaner cleaner = new Cleaner(null, log, false, null, null, false, true, false); Path testDir = Paths.get("target/test-classes/unit/test-dir").toAbsolutePath(); Path dirWithLnk = testDir.resolve("dir"); Path orgDir = testDir.resolve("org-dir"); @@ -254,7 +258,7 @@ private void testSymlink(LinkCreator linkCreator) throws Exception { Files.write(file, Collections.singleton("Hello world")); linkCreator.createLink(jctDir, orgDir); // delete - cleaner.delete(dirWithLnk, null, false, true, false); + cleaner.delete(dirWithLnk); // verify assertTrue(Files.exists(file)); assertFalse(Files.exists(jctDir)); @@ -267,7 +271,8 @@ private void testSymlink(LinkCreator linkCreator) throws Exception { Files.write(file, Collections.singleton("Hello world")); linkCreator.createLink(jctDir, orgDir); // delete - cleaner.delete(dirWithLnk, null, true, true, false); + cleaner = new Cleaner(null, log, false, null, null, true, true, false); + cleaner.delete(dirWithLnk); // verify assertFalse(Files.exists(file)); assertFalse(Files.exists(jctDir)); diff --git a/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java b/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java index 798ee3a..c4e8567 100644 --- a/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java +++ b/src/test/java/org/apache/maven/plugins/clean/CleanerTest.java @@ -18,9 +18,7 @@ */ package org.apache.maven.plugins.clean; -import java.io.IOException; import java.nio.file.AccessDeniedException; -import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermission; import java.nio.file.attribute.PosixFilePermissions; @@ -41,8 +39,8 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.inOrder; @@ -61,8 +59,8 @@ class CleanerTest { void deleteSucceedsDeeply(@TempDir Path tempDir) throws Exception { final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); final Path file = createFile(basedir.resolve("file")); - final Cleaner cleaner = new Cleaner(null, log, false, null, null); - cleaner.delete(basedir, null, false, true, false); + final var cleaner = new Cleaner(null, log, false, null, null, false, true, false); + cleaner.delete(basedir); assertFalse(exists(basedir)); assertFalse(exists(file)); } @@ -76,14 +74,10 @@ void deleteFailsWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws Excep // Remove the executable flag to prevent directory listing, which will result in a DirectoryNotEmptyException. final Set permissions = PosixFilePermissions.fromString("rw-rw-r--"); setPosixFilePermissions(basedir, permissions); - final Cleaner cleaner = new Cleaner(null, log, false, null, null); - final IOException exception = - assertThrows(IOException.class, () -> cleaner.delete(basedir, null, false, true, false)); - verify(log, never()).warn(any(CharSequence.class), any(Throwable.class)); - assertEquals("Failed to delete " + basedir, exception.getMessage()); - final DirectoryNotEmptyException cause = - assertInstanceOf(DirectoryNotEmptyException.class, exception.getCause()); - assertEquals(basedir.toString(), cause.getMessage()); + final var cleaner = new Cleaner(null, log, false, null, null, false, true, false); + final var exception = assertThrows(AccessDeniedException.class, () -> cleaner.delete(basedir)); + verify(log, times(1)).warn(any(CharSequence.class), any(Throwable.class)); + assertTrue(exception.getMessage().contains(basedir.toString())); } @Test @@ -94,13 +88,9 @@ void deleteFailsAfterRetryWhenNoPermission(@TempDir Path tempDir) throws Excepti // Remove the executable flag to prevent directory listing, which will result in a DirectoryNotEmptyException. final Set permissions = PosixFilePermissions.fromString("rw-rw-r--"); setPosixFilePermissions(basedir, permissions); - final Cleaner cleaner = new Cleaner(null, log, false, null, null); - final IOException exception = - assertThrows(IOException.class, () -> cleaner.delete(basedir, null, false, true, true)); - assertEquals("Failed to delete " + basedir, exception.getMessage()); - final DirectoryNotEmptyException cause = - assertInstanceOf(DirectoryNotEmptyException.class, exception.getCause()); - assertEquals(basedir.toString(), cause.getMessage()); + final var cleaner = new Cleaner(null, log, false, null, null, false, true, true); + final var exception = assertThrows(AccessDeniedException.class, () -> cleaner.delete(basedir)); + assertTrue(exception.getMessage().contains(basedir.toString())); } @Test @@ -112,16 +102,13 @@ void deleteLogsWarningWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws // Remove the writable flag to prevent deletion of the file, which will result in an AccessDeniedException. final Set permissions = PosixFilePermissions.fromString("r-xr-xr-x"); setPosixFilePermissions(basedir, permissions); - final Cleaner cleaner = new Cleaner(null, log, false, null, null); - assertDoesNotThrow(() -> cleaner.delete(basedir, null, false, false, false)); - verify(log, times(2)).warn(any(CharSequence.class), any(Throwable.class)); + final var cleaner = new Cleaner(null, log, false, null, null, false, false, false); + assertDoesNotThrow(() -> cleaner.delete(basedir)); + verify(log, times(1)).warn(any(CharSequence.class), any(Throwable.class)); InOrder inOrder = inOrder(log); ArgumentCaptor cause1 = ArgumentCaptor.forClass(AccessDeniedException.class); inOrder.verify(log).warn(eq("Failed to delete " + file), cause1.capture()); assertEquals(file.toString(), cause1.getValue().getMessage()); - ArgumentCaptor cause2 = ArgumentCaptor.forClass(DirectoryNotEmptyException.class); - inOrder.verify(log).warn(eq("Failed to delete " + basedir), cause2.capture()); - assertEquals(basedir.toString(), cause2.getValue().getMessage()); } @Test @@ -133,8 +120,8 @@ void deleteDoesNotLogAnythingWhenNoPermissionAndWarnDisabled(@TempDir Path tempD // Remove the writable flag to prevent deletion of the file, which will result in an AccessDeniedException. final Set permissions = PosixFilePermissions.fromString("r-xr-xr-x"); setPosixFilePermissions(basedir, permissions); - final Cleaner cleaner = new Cleaner(null, log, false, null, null); - assertDoesNotThrow(() -> cleaner.delete(basedir, null, false, false, false)); + final var cleaner = new Cleaner(null, log, false, null, null, false, false, false); + assertDoesNotThrow(() -> cleaner.delete(basedir)); verify(log, never()).warn(any(CharSequence.class), any(Throwable.class)); } } From 26a0ca60ebbb84b1757975fd72eff56300b02573 Mon Sep 17 00:00:00 2001 From: Martin Desruisseaux Date: Mon, 31 Mar 2025 15:14:46 +0200 Subject: [PATCH 2/3] Remove the last dependency to Plexus by copying the content of the `DEFAULTEXCLUDES` array. --- pom.xml | 5 -- .../apache/maven/plugins/clean/Selector.java | 83 ++++++++++++++++++- .../maven/plugins/clean/CleanMojoTest.java | 5 +- 3 files changed, 82 insertions(+), 11 deletions(-) diff --git a/pom.xml b/pom.xml index aa6bda3..d1717a4 100644 --- a/pom.xml +++ b/pom.xml @@ -99,11 +99,6 @@ under the License. provided - - org.codehaus.plexus - plexus-utils - - org.apache.maven.plugin-testing diff --git a/src/main/java/org/apache/maven/plugins/clean/Selector.java b/src/main/java/org/apache/maven/plugins/clean/Selector.java index 3810506..ee84048 100644 --- a/src/main/java/org/apache/maven/plugins/clean/Selector.java +++ b/src/main/java/org/apache/maven/plugins/clean/Selector.java @@ -25,8 +25,6 @@ import java.util.LinkedHashSet; import java.util.Set; -import org.codehaus.plexus.util.DirectoryScanner; - /** * Determines whether a path is selected for deletion. * The pathnames used for method parameters will be relative to some base directory @@ -54,6 +52,85 @@ * @author Martin Desruisseaux */ final class Selector implements PathMatcher { + /** + * Patterns which should be excluded by default, like SCM files. + * + *

Source: this list is copied from {@code plexus-utils-4.0.2} (released in + * September 23, 2024), class {@code org.codehaus.plexus.util.AbstractScanner}.

+ */ + private static final String[] DEFAULT_EXCLUDES = { + // Miscellaneous typical temporary files + "**/*~", + "**/#*#", + "**/.#*", + "**/%*%", + "**/._*", + + // CVS + "**/CVS", + "**/CVS/**", + "**/.cvsignore", + + // RCS + "**/RCS", + "**/RCS/**", + + // SCCS + "**/SCCS", + "**/SCCS/**", + + // Visual SourceSafe + "**/vssver.scc", + + // MKS + "**/project.pj", + + // Subversion + "**/.svn", + "**/.svn/**", + + // Arch + "**/.arch-ids", + "**/.arch-ids/**", + + // Bazaar + "**/.bzr", + "**/.bzr/**", + + // SurroundSCM + "**/.MySCMServerInfo", + + // Mac + "**/.DS_Store", + + // Serena Dimensions Version 10 + "**/.metadata", + "**/.metadata/**", + + // Mercurial + "**/.hg", + "**/.hg/**", + + // git + "**/.git", + "**/.git/**", + "**/.gitignore", + + // BitKeeper + "**/BitKeeper", + "**/BitKeeper/**", + "**/ChangeSet", + "**/ChangeSet/**", + + // darcs + "**/_darcs", + "**/_darcs/**", + "**/.darcsrepo", + "**/.darcsrepo/**", + "**/-darcs-backup*", + "**/.darcs-temp-mail" + }; + /** * String representation of the normalized include filters. * This is kept only for {@link #toString()} implementation. @@ -122,7 +199,7 @@ private static String[] addDefaultExcludes(final String[] excludes, final boolea if (!useDefaultExcludes) { return excludes; } - String[] defaults = DirectoryScanner.DEFAULTEXCLUDES; + String[] defaults = DEFAULT_EXCLUDES; if (excludes == null || excludes.length == 0) { return defaults; } else { diff --git a/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java b/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java index ad6d64e..670133c 100644 --- a/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java @@ -41,7 +41,6 @@ import static org.apache.maven.api.plugin.testing.MojoExtension.getBasedir; import static org.apache.maven.api.plugin.testing.MojoExtension.setVariableValueToObject; -import static org.codehaus.plexus.util.IOUtil.copy; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -215,8 +214,8 @@ public void testFollowLinksWithWindowsJunction() throws Exception { .start(); process.waitFor(); ByteArrayOutputStream baos = new ByteArrayOutputStream(); - copy(process.getInputStream(), baos); - copy(process.getErrorStream(), baos); + process.getInputStream().transferTo(baos); + process.getErrorStream().transferTo(baos); if (!Files.exists(link)) { throw new IOException("Unable to create junction: " + baos); } From 2bdc6c585ca0f00fcac3fe54a2ec7a66d8ff981a Mon Sep 17 00:00:00 2001 From: Martin Desruisseaux Date: Tue, 1 Apr 2025 14:10:42 +0200 Subject: [PATCH 3/3] Try to port MCLEAN-93: Windows NTFS junctions support (second attempt). --- .../apache/maven/plugins/clean/Cleaner.java | 41 ++++++++++++++++--- 1 file changed, 35 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 ffac936..0a399ac 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,9 @@ * @author Martin Desruisseaux */ final class Cleaner implements FileVisitor { - + /** + * Whether the host operating system is from the Windows family. + */ private static final boolean ON_WINDOWS = (File.separatorChar == '\\'); private static final SessionData.Key LAST_DIRECTORY_TO_DELETE = @@ -171,13 +173,13 @@ public void delete(@Nonnull Fileset fileset) throws IOException { } /** - * Deletes the specified directories and its contents. + * Deletes the specified directory and its contents. * Non-existing directories will be silently ignored. * * @param basedir the directory to delete, must not be {@code null} * @throws IOException if a file/directory could not be deleted and {@code failOnError} is {@code true} */ - public void delete(Path basedir) throws IOException { + public void delete(@Nonnull Path basedir) throws IOException { if (!Files.isDirectory(basedir)) { if (Files.notExists(basedir)) { if (logger.isDebugEnabled()) { @@ -193,7 +195,7 @@ public void delete(Path basedir) throws IOException { var options = EnumSet.noneOf(FileVisitOption.class); if (followSymlinks) { options.add(FileVisitOption.FOLLOW_LINKS); - basedir = getCanonicalPath(basedir); + basedir = getCanonicalPath(basedir, null); } if (selector == null && !followSymlinks && fastDir != null && session != null) { // If anything wrong happens, we'll just use the usual deletion mechanism @@ -259,6 +261,14 @@ private boolean fastDelete(Path baseDir) { */ @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + if (ON_WINDOWS && !followSymlinks && attrs.isOther()) { + /* + * MCLEAN-93: NTFS junctions have isDirectory() and isOther() attributes set. + * If not following symbolic links, then it should be handled as a file. + */ + visitFile(dir, attrs); + return FileVisitResult.SKIP_SUBTREE; + } if (selector == null || selector.couldHoldSelected(dir)) { nonEmptyDirectoryLevels.clear(++currentDepth); return FileVisitResult.CONTINUE; @@ -333,11 +343,30 @@ public FileVisitResult postVisitDirectory(Path dir, IOException failure) throws return FileVisitResult.CONTINUE; } - private static Path getCanonicalPath(Path path) { + /** + * Returns the real path of the given file. If the real path cannot be obtained, + * this method tries to get the real path of the parent and to append the rest of + * the filename. + * + * @param path the path to get as a canonical path + * @param mainError should be {@code null} (reserved to recursive calls of this method) + * @return the real path of the given path + * @throws IOException if the canonical path cannot be obtained + */ + private static Path getCanonicalPath(final Path path, IOException mainError) throws IOException { try { return path.toRealPath(); } catch (IOException e) { - return getCanonicalPath(path.getParent()).resolve(path.getFileName()); + if (mainError == null) { + mainError = e; + } else { + mainError.addSuppressed(e); + } + final Path parent = path.getParent(); + if (parent != null) { + return getCanonicalPath(parent, mainError).resolve(path.getFileName()); + } + throw e; } }