From e1ffab1cac55179011777b9a60bac447b0a62ccf Mon Sep 17 00:00:00 2001 From: Matthaus Owens Date: Fri, 29 Jul 2016 15:21:20 -0700 Subject: [PATCH] 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 --- .../eclipse/jgit/api/CleanCommandTest.java | 40 +++++++++ .../org/eclipse/jgit/api/CleanCommand.java | 84 ++++++++++++++++--- 2 files changed, 113 insertions(+), 11 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CleanCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CleanCommandTest.java index 7daa9957d..68f5dd1e0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CleanCommandTest.java +++ b/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 untrackedFolders = beforeCleanStatus.getUntrackedFolders(); + Set 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 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 forceCleanedFiles = git.clean().setCleanDirectories(true) + .setForce(true).call(); + + // The inner repository should be cleaned this time + assertTrue(forceCleanedFiles.contains(innerRepoName + "/")); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CleanCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CleanCommand.java index 596712811..7e331fd84 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CleanCommand.java +++ b/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> { private boolean ignore = true; + private boolean force = false; + /** * @param repo */ @@ -121,25 +125,69 @@ public class CleanCommand extends GitCommand> { 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$ - } + for (String dir : notIgnoredDirs) + if (paths.isEmpty() || paths.contains(dir)) { + files = cleanPath(dir, files); + } } catch (IOException e) { throw new JGitInternalException(e.getMessage(), e); } 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 cleanPath(String path, Set 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 filterIgnorePaths(Set inputPaths, Set ignoredNotInIndex, boolean exact) { if (ignore) { @@ -195,6 +243,20 @@ public class CleanCommand extends GitCommand> { 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. *