Configuration parameter for deleting read-only files

#250

Author: desruisseaux

src/it/read-only/pom.xml

@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>test</groupId>
+  <artifactId>read-only</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <name>Test for clean</name>
+  <description>Check for proper deletion (or not) of read-only files.</description>
+
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+  </properties>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-clean-plugin</artifactId>
+        <version>@pom.version@</version>
+        <configuration>
+          <force>true</force>
+          <retryOnError>false</retryOnError>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
+</project>

src/it/read-only/setup.bsh

@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+import java.io.File;
+
+if (!new File(basedir, "target/read-only-dir/read-only.properties").setWritable(false)) {
+    System.out.println("Cannot change file permission.");
+    return false;
+}
+if (File.separatorChar == '/') {
+    // Directory permission can be changed only on Unix, not on Windows.
+    if (!new File(basedir, "target/read-only-dir").setWritable(false)) {
+        System.out.println("Cannot change directory permission.");
+        return false;
+    }
+}
+if (!new File(basedir, "target/writable-dir/writable.properties").canWrite()) {
+    System.out.println("Expected a writable file.");
+    return false;
+}
+return true;

src/it/read-only/target/read-only-dir/read-only.properties

@@ -0,0 +1,16 @@
+# 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.

src/it/read-only/target/writable-dir/writable.properties

@@ -0,0 +1,16 @@
+# 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.

src/it/read-only/verify.bsh

@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+import java.io.File;
+
+if (new File(basedir, "target").exists()) {
+    System.out.println("target should have been deleted.");
+    return false;
+}
+return true;

src/main/java/org/apache/maven/plugins/clean/CleanMojo.java

@@ -128,6 +128,14 @@ public class CleanMojo extends AbstractMojo {
     @Parameter(property = "maven.clean.followSymLinks", defaultValue = "false")
     private boolean followSymLinks;
 
+    /**
+     * Whether to force the deletion of read-only files.
+     *
+     * @since 3.4.2
+     */
+    @Parameter(property = "maven.clean.force", defaultValue = "false")
+    private boolean force;
+
     /**
      * Disables the plugin execution. <br/>
      * Starting with <code>3.0.0</code> the property has been renamed from <code>clean.skip</code> to
@@ -249,7 +257,7 @@ public void execute() throws MojoExecutionException {
                     + FAST_MODE_BACKGROUND + "', '" + FAST_MODE_AT_END + "' and '" + FAST_MODE_DEFER + "'.");
         }
 
-        Cleaner cleaner = new Cleaner(session, getLog(), isVerbose(), fastDir, fastMode);
+        Cleaner cleaner = new Cleaner(session, getLog(), isVerbose(), fastDir, fastMode, force);
 
         try {
             for (Path directoryItem : getDirectories()) {

src/main/java/org/apache/maven/plugins/clean/Cleaner.java

@@ -23,14 +23,21 @@
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
+import java.nio.file.AccessDeniedException;
 import java.nio.file.DirectoryStream;
 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.nio.file.attribute.DosFileAttributeView;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.apache.maven.execution.ExecutionListener;
 import org.apache.maven.execution.MavenSession;
@@ -68,6 +75,13 @@ class Cleaner {
 
     private Log log;
 
+    /**
+     * Whether to force the deletion of read-only files. Note that on Linux,
+     * {@link Files#delete(Path)} and {@link Files#deleteIfExists(Path)} delete read-only files
+     * but throw {@link AccessDeniedException} if the directory containing the file is read-only.
+     */
+    private final boolean force;
+
     /**
      * Creates a new cleaner.
      *
@@ -76,15 +90,17 @@ class Cleaner {
      * @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 force    whether to force the deletion of read-only files
      */
-    Cleaner(MavenSession session, final Log log, boolean verbose, Path fastDir, String fastMode) {
+    Cleaner(MavenSession session, final Log log, boolean verbose, Path fastDir, String fastMode, boolean force) {
         this.session = session;
         // This can't be null as the Cleaner gets it from the CleanMojo which gets it from AbstractMojo class, where it
         // is never null.
         this.log = log;
         this.fastDir = fastDir;
         this.fastMode = fastMode;
         this.verbose = verbose;
+        this.force = force;
     }
 
     /**
@@ -262,6 +278,38 @@ private boolean isSymbolicLink(Path path) throws IOException {
                 || (attrs.isDirectory() && attrs.isOther());
     }
 
+    /**
+     * Makes the given file or directory writable.
+     * If the file is already writable, then this method tries to make the parent directory writable.
+     *
+     * @param file the path to the file or directory to make writable, or {@code null} if none
+     * @return the root path which has been made writable, or {@code null} if none
+     */
+    private static Path setWritable(Path file) throws IOException {
+        while (file != null) {
+            PosixFileAttributeView posix = Files.getFileAttributeView(file, PosixFileAttributeView.class);
+            if (posix != null) {
+                EnumSet<PosixFilePermission> permissions =
+                        EnumSet.copyOf(posix.readAttributes().permissions());
+                if (permissions.add(PosixFilePermission.OWNER_WRITE)) {
+                    posix.setPermissions(permissions);
+                    return file;
+                }
+            } else {
+                DosFileAttributeView dos = Files.getFileAttributeView(file, DosFileAttributeView.class);
+                if (dos == null) {
+                    return null; // Unknown type of file attributes.
+                }
+                // No need to update the parent directory because DOS read-only attribute does not apply to folders.
+                dos.setReadOnly(false);
+                return file;
+            }
+            // File was already writable. Maybe it is the parent directory which was not writable.
+            file = file.getParent();
+        }
+        return null;
+    }
+
     /**
      * Deletes the specified file, directory. If the path denotes a symlink, only the link is removed, its target is
      * left untouched.
@@ -276,21 +324,35 @@ private int delete(Path file, boolean failOnError, boolean retryOnError) throws
         IOException failure = delete(file);
         if (failure != null) {
             boolean deleted = false;
-
-            if (retryOnError) {
-                if (ON_WINDOWS) {
-                    // try to release any locks held by non-closed files
-                    System.gc();
+            boolean tryWritable = force && failure instanceof AccessDeniedException;
+            if (tryWritable || retryOnError) {
+                final Set<Path> madeWritable; // Safety against never-ending loops.
+                if (force) {
+                    madeWritable = new HashSet<>();
+                    madeWritable.add(null); // For having `add(null)` to return `false`.
+                } else {
+                    madeWritable = null;
                 }
-
                 final int[] delays = {50, 250, 750};
-                for (int i = 0; !deleted && i < delays.length; i++) {
-                    try {
-                        Thread.sleep(delays[i]);
-                    } catch (InterruptedException e) {
-                        // ignore
+                int delayIndex = 0;
+                while (!deleted && delayIndex < delays.length) {
+                    if (tryWritable) {
+                        tryWritable = madeWritable.add(setWritable(file));
+                        // `true` if we successfully changed permission, in which case we will skip the delay.
+                    }
+                    if (!tryWritable) {
+                        if (ON_WINDOWS) {
+                            // Try to release any locks held by non-closed files.
+                            System.gc();
+                        }
+                        try {
+                            Thread.sleep(delays[delayIndex++]);
+                        } catch (InterruptedException e) {
+                            // ignore
+                        }
                     }
                     deleted = delete(file) == null || !exists(file);
+                    tryWritable = !deleted && force && failure instanceof AccessDeniedException;
                 }
             } else {
                 deleted = !exists(file);

src/test/java/org/apache/maven/plugins/clean/CleanMojoTest.java

@@ -250,7 +250,7 @@ void testFollowLinksWithSymLinkOnPosix() throws Exception {
 
     private void testSymlink(LinkCreator linkCreator) throws Exception {
         // We use the SystemStreamLog() as the AbstractMojo class, because from there the Log is always provided
-        Cleaner cleaner = new Cleaner(null, new SystemStreamLog(), false, null, null);
+        Cleaner cleaner = new Cleaner(null, new SystemStreamLog(), false, null, null, false);
         Path testDir = Paths.get("target/test-classes/unit/test-dir").toAbsolutePath();
         Path dirWithLnk = testDir.resolve("dir");
         Path orgDir = testDir.resolve("org-dir");

src/test/java/org/apache/maven/plugins/clean/CleanerTest.java

@@ -61,7 +61,7 @@ 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);
+        final Cleaner cleaner = new Cleaner(null, log, false, null, null, false);
         cleaner.delete(basedir, null, false, true, false);
         assertFalse(exists(basedir));
         assertFalse(exists(file));
@@ -76,7 +76,7 @@ void deleteFailsWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws Excep
         // Remove the executable flag to prevent directory listing, which will result in a DirectoryNotEmptyException.
         final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString("rw-rw-r--");
         setPosixFilePermissions(basedir, permissions);
-        final Cleaner cleaner = new Cleaner(null, log, false, null, null);
+        final Cleaner cleaner = new Cleaner(null, log, false, null, null, false);
         final IOException exception =
                 assertThrows(IOException.class, () -> cleaner.delete(basedir, null, false, true, false));
         verify(log, never()).warn(any(CharSequence.class), any(Throwable.class));
@@ -94,7 +94,7 @@ void deleteFailsAfterRetryWhenNoPermission(@TempDir Path tempDir) throws Excepti
         // Remove the executable flag to prevent directory listing, which will result in a DirectoryNotEmptyException.
         final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString("rw-rw-r--");
         setPosixFilePermissions(basedir, permissions);
-        final Cleaner cleaner = new Cleaner(null, log, false, null, null);
+        final Cleaner cleaner = new Cleaner(null, log, false, null, null, false);
         final IOException exception =
                 assertThrows(IOException.class, () -> cleaner.delete(basedir, null, false, true, true));
         assertEquals("Failed to delete " + basedir, exception.getMessage());
@@ -112,7 +112,7 @@ void deleteLogsWarningWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws
         // Remove the writable flag to prevent deletion of the file, which will result in an AccessDeniedException.
         final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString("r-xr-xr-x");
         setPosixFilePermissions(basedir, permissions);
-        final Cleaner cleaner = new Cleaner(null, log, false, null, null);
+        final Cleaner cleaner = new Cleaner(null, log, false, null, null, false);
         assertDoesNotThrow(() -> cleaner.delete(basedir, null, false, false, false));
         verify(log, times(2)).warn(any(CharSequence.class), any(Throwable.class));
         InOrder inOrder = inOrder(log);
@@ -133,7 +133,7 @@ void deleteDoesNotLogAnythingWhenNoPermissionAndWarnDisabled(@TempDir Path tempD
         // Remove the writable flag to prevent deletion of the file, which will result in an AccessDeniedException.
         final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString("r-xr-xr-x");
         setPosixFilePermissions(basedir, permissions);
-        final Cleaner cleaner = new Cleaner(null, log, false, null, null);
+        final Cleaner cleaner = new Cleaner(null, log, false, null, null, false);
         assertDoesNotThrow(() -> cleaner.delete(basedir, null, false, false, false));
         verify(log, never()).warn(any(CharSequence.class), any(Throwable.class));
     }