Skip cleaning inner repositories by default in CleanCommand

Previously jgit would attempt to clean git repositories that had not
been committed by calling a non-recursive delete on them, which would
fail as they are directories. This commit addresses that issue in the
following ways.
Repositories are skipped in a default clean, similarly to cgit and only
cleaned when the force flag is applied. When the force flag is applied
repositories are deleted using a recursive delete call. The force flag
and setForce method are added here to CleanCommand to support this
change.

Bug: 498367
Change-Id: Ib6cfff65a033d0d0f76395060bf76719e13fc467
Signed-off-by: Matthaus Owens <matthaus@puppetlabs.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CleanCommandTest.java

@@ -47,6 +47,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.eclipse.jgit.lib.Constants.DOT_GIT_MODULES;
 
+import java.io.File;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -255,4 +256,43 @@ public class CleanCommandTest extends RepositoryTestCase {
 		assertTrue(cleanedFiles.contains("sub-clean/"));
 		assertTrue(cleanedFiles.size() == 4);
 	}
+
+	@Test
+	public void testCleanDirsWithRepository() throws Exception {
+		// Set up a repository inside the outer repository
+		String innerRepoName = "inner-repo";
+		File innerDir = new File(trash, innerRepoName);
+		innerDir.mkdir();
+		InitCommand initRepoCommand = new InitCommand();
+		initRepoCommand.setDirectory(innerDir);
+		initRepoCommand.call();
+
+		Status beforeCleanStatus = git.status().call();
+		Set<String> untrackedFolders = beforeCleanStatus.getUntrackedFolders();
+		Set<String> untrackedFiles = beforeCleanStatus.getUntracked();
+
+		// The inner repository should be listed as an untracked file
+		assertTrue(untrackedFiles.contains(innerRepoName));
+
+		// The inner repository should not be listed as an untracked folder
+		assertTrue(!untrackedFolders.contains(innerRepoName));
+
+		Set<String> cleanedFiles = git.clean().setCleanDirectories(true).call();
+
+		// The inner repository should not be cleaned.
+		assertTrue(!cleanedFiles.contains(innerRepoName + "/"));
+
+		assertTrue(cleanedFiles.contains("File2.txt"));
+		assertTrue(cleanedFiles.contains("File3.txt"));
+		assertTrue(!cleanedFiles.contains("sub-noclean/File1.txt"));
+		assertTrue(cleanedFiles.contains("sub-noclean/File2.txt"));
+		assertTrue(cleanedFiles.contains("sub-clean/"));
+		assertTrue(cleanedFiles.size() == 4);
+
+		Set<String> forceCleanedFiles = git.clean().setCleanDirectories(true)
+				.setForce(true).call();
+
+		// The inner repository should be cleaned this time
+		assertTrue(forceCleanedFiles.contains(innerRepoName + "/"));
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/api/CleanCommand.java

@@ -43,6 +43,8 @@
  */
 package org.eclipse.jgit.api;
 
+import static org.eclipse.jgit.lib.Constants.DOT_GIT;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
@@ -73,6 +75,8 @@ public class CleanCommand extends GitCommand<Set<String>> {
 
 	private boolean ignore = true;
 
+	private boolean force = false;
+
 	/**
 	 * @param repo
 	 */
@@ -121,18 +125,12 @@ public class CleanCommand extends GitCommand<Set<String>> {
 
 			for (String file : notIgnoredFiles)
 				if (paths.isEmpty() || paths.contains(file)) {
-					if (!dryRun)
-						FileUtils.delete(new File(repo.getWorkTree(), file));
-					files.add(file);
+					files = cleanPath(file, files);
 				}
 
-			if (directories)
 			for (String dir : notIgnoredDirs)
 				if (paths.isEmpty() || paths.contains(dir)) {
-						if (!dryRun)
-							FileUtils.delete(new File(repo.getWorkTree(), dir),
-									FileUtils.RECURSIVE);
-						files.add(dir + "/"); //$NON-NLS-1$
+					files = cleanPath(dir, files);
 				}
 		} catch (IOException e) {
 			throw new JGitInternalException(e.getMessage(), e);
@@ -140,6 +138,56 @@ public class CleanCommand extends GitCommand<Set<String>> {
 		return files;
 	}
 
+	/**
+	 * When dryRun is false, deletes the specified path from disk. If dryRun
+	 * is true, no paths are actually deleted. In both cases, the paths that
+	 * would have been deleted are added to inFiles and returned.
+	 *
+	 * Paths that are directories are recursively deleted when
+	 * {@link #directories} is true.
+	 * Paths that are git repositories are recursively deleted when
+	 * {@link #directories} and {@link #force} are both true.
+	 *
+	 * @param path
+	 * 			The path to be cleaned
+	 * @param inFiles
+	 * 			A set of strings representing the files that have been cleaned
+	 * 			already, the path to be cleaned will be added to this set
+	 * 			before being returned.
+	 *
+	 * @return a set of strings with the cleaned path added to it
+	 * @throws IOException
+	 */
+	private Set<String> cleanPath(String path, Set<String> inFiles)
+			throws IOException {
+		File curFile = new File(repo.getWorkTree(), path);
+		if (curFile.isDirectory()) {
+			if (directories) {
+				// Is this directory a git repository?
+				if (new File(curFile, DOT_GIT).exists()) {
+					if (force) {
+						if (!dryRun) {
+							FileUtils.delete(curFile, FileUtils.RECURSIVE);
+						}
+						inFiles.add(path + "/"); //$NON-NLS-1$
+					}
+				} else {
+					if (!dryRun) {
+						FileUtils.delete(curFile, FileUtils.RECURSIVE);
+					}
+					inFiles.add(path + "/"); //$NON-NLS-1$
+				}
+			}
+		} else {
+			if (!dryRun) {
+				FileUtils.delete(curFile, FileUtils.NONE);
+			}
+			inFiles.add(path);
+		}
+
+		return inFiles;
+	}
+
 	private Set<String> filterIgnorePaths(Set<String> inputPaths,
 			Set<String> ignoredNotInIndex, boolean exact) {
 		if (ignore) {
@@ -195,6 +243,20 @@ public class CleanCommand extends GitCommand<Set<String>> {
 		return this;
 	}
 
+	/**
+	 * If force is set, directories that are git repositories will also be
+	 * deleted.
+	 *
+	 * @param force
+	 *            whether or not to delete git repositories
+	 * @return {@code this}
+	 * @since 4.5
+	 */
+	public CleanCommand setForce(boolean force) {
+		this.force = force;
+		return this;
+	}
+
 	/**
 	 * If dirs is set, in addition to files, also clean directories.
 	 *