summaryrefslogtreecommitdiff
path: root/t/t7300-clean.sh
AgeCommit message (Collapse)AuthorFilesLines
2021-05-13dir: traverse into untracked directories if they may have ignored subfilesLibravatar Elijah Newren1-1/+1
A directory that is untracked does not imply that all files under it should be categorized as untracked; in particular, if the caller is interested in ignored files, many files or directories underneath the untracked directory may be ignored. We previously partially handled this right with DIR_SHOW_IGNORED_TOO, but missed DIR_SHOW_IGNORED. It was not obvious, though, because the logic for untracked and excluded files had been fused together making it harder to reason about. The previous commit split that logic out, making it easier to notice that DIR_SHOW_IGNORED was missing. Add it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13dir: avoid unnecessary traversal into ignored directoryLibravatar Elijah Newren1-1/+1
The show_other_directories case in treat_directory() tried to handle both excludes and untracked files with the same logic, and mishandled both the excludes and the untracked files in the process, in different ways. Split that logic apart, and then focus on the logic for the excludes; a subsequent commit will address the logic for untracked files. For show_other_directories, an excluded directory means that every path underneath that directory will also be excluded. Given that the calling code requested to just show directories when everything under a directory had the same state (that's what the "DIR_SHOW_OTHER_DIRECTORIES" flag means), we generally do not need to traverse into such directories and can just immediately mark them as ignored (i.e. as path_excluded). The only reason we cannot just immediately return path_excluded is the DIR_HIDE_EMPTY_DIRECTORIES flag and the possibility that the ignored directory is an empty directory. The code previously treated DIR_SHOW_IGNORED_TOO in most cases as an exception as well, which was wrong. It can sometimes reduce the number of cases where we need to recurse (namely if DIR_SHOW_IGNORED_TOO_MODE_MATCHING is also set), but should not be able to increase the number of cases where we need to recurse. Fix the logic accordingly. Some sidenotes about possible confusion with dir.c: * "ignored" often refers to an untracked ignore", i.e. a file which is not tracked which matches one of the ignore/exclusion rules. But you can also have a "tracked ignore", a tracked file that happens to match one of the ignore/exclusion rules and which dir.c has to worry about since "git ls-files -c -i" is supposed to list them. * The dir code often uses "ignored" and "excluded" interchangeably, which you need to keep in mind while reading the code. * "exclude" is used multiple ways in the code: * As noted above, "exclude" is often a synonym for "ignored". * The logic for parsing .gitignore files was re-used in .git/info/sparse-checkout, except there it is used to mark paths that the user wants to *keep*. This was mostly addressed by commit 65edd96aec ("treewide: rename 'exclude' methods to 'pattern'", 2019-09-03), but every once in a while you'll find a comment about "exclude" referring to these patterns that might in fact be in use by the sparse-checkout machinery for inclusion rules. * The word "EXCLUDE" is also used for pathspec negation, as in (pathspec->items[3].magic & PATHSPEC_EXCLUDE) Thus if a user had a .gitignore file containing *~ *.log !settings.log And then ran git add -- 'settings.*' ':^settings.log' Then :^settings.log is a pathspec negation making settings.log not be requested to be added even though all other settings.* files are being added. Also, !settings.log in the gitignore file is a negative exclude pattern meaning that settings.log is normally a file we want to track even though all other *.log files are ignored. Sometimes it feels like dir.c needs its own glossary with its many definitions, including the multiply-defined terms. Reported-by: Jason Gore <Jason.Gore@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13t3001, t7300: add testcase showcasing missed directory traversalLibravatar Elijah Newren1-0/+19
In the last commit, we added a testcase showing that the directory traversal machinery sometimes traverses into directories unnecessarily. Here we show that there are cases where it does the opposite: it does not traverse into directories, despite those directories having important files that need to be flagged. Add a testcase showing that `git ls-files -o -i --directory` can omit some of the files it should be listing, and another showing that `git clean -fX` can fail to clean out some of the expected files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13t7300: add testcase showing unnecessary traversal into ignored directoryLibravatar Elijah Newren1-0/+23
The PNPM package manager is apparently creating deeply nested (but ignored) directory structures; traversing them is costly performance-wise, unnecessary, and in some cases is even throwing warnings/errors because the paths are too long to handle on various platforms. Add a testcase that checks for such unnecessary directory traversal. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10tests: remove most uses of C_LOCALE_OUTPUTLibravatar Ævar Arnfjörð Bjarmason1-3/+3
As a follow-up to d162b25f956 (tests: remove support for GIT_TEST_GETTEXT_POISON, 2021-01-20) remove those uses of the now always true C_LOCALE_OUTPUT prerequisite from those tests which declare it as an argument to test_expect_{success,failure}. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16dir: treat_leading_path() and read_directory_recursive(), round 2Libravatar Elijah Newren1-1/+1
I was going to title this "dir: more synchronizing of treat_leading_path() and read_directory_recursive()", a nod to commit 777b42034764 ("dir: synchronize treat_leading_path() and read_directory_recursive()", 2019-12-19), but the title was too long. Anyway, first the backstory... fill_directory() has always had a slightly error-prone interface: it returns a subset of paths which *might* match the specified pathspec; it was intended to prune away some paths which didn't match the specified pathspec and keep at least all the ones that did match it. Given this interface, callers were responsible to post-process the results and check whether each actually matched the pathspec. builtin/clean.c did this. It would first prune out duplicates (e.g. if "dir" was returned as well as all files under "dir/", then it would simplify this to just "dir"), and after pruning duplicates it would compare the remaining paths to the specified pathspec(s). This post-processing itself could run into problems, though, as noted in commit 404ebceda01c ("dir: also check directories for matching pathspecs", 2019-09-17): For the case of git-clean and a set of pathspecs of "dir/file" and "more", this caused a problem because we'd end up with dir entries for both of "dir" "dir/file" Then correct_untracked_entries() would try to helpfully prune duplicates for us by removing "dir/file" since it's under "dir", leaving us with "dir" Since the original pathspec only had "dir/file", the only entry left doesn't match and leaves nothing to be removed. (Note that if only one pathspec was specified, e.g. only "dir/file", then the common_prefix_len optimizations in fill_directory would cause us to bypass this problem, making it appear in simple tests that we could correctly remove manually specified pathspecs.) That commit fixed the issue -- when multiple pathspecs were specified -- by making sure fill_directory() wouldn't return both "dir" and "dir/file" outside the common_prefix_len optimization path. This is where it starts to get fun. In commit b9670c1f5e6b ("dir: fix checks on common prefix directory", 2019-12-19), we noticed that the common_prefix_len wasn't doing appropriate checks and letting all kinds of stuff through, resulting in recursing into .git/ directories and other craziness. So it started locking down and doing checks on pathnames within that code path. That continued with commit 777b42034764 ("dir: synchronize treat_leading_path() and read_directory_recursive()", 2019-12-19), which noted the following: Our optimization to avoid calling into read_directory_recursive() when all pathspecs have a common leading directory mean that we need to match the logic that read_directory_recursive() would use if we had just called it from the root. Since it does more than call treat_path() we need to copy that same logic. ...and then it more forcefully addressed the issue with this wonderfully ironic statement: Needing to duplicate logic like this means it is guaranteed someone will eventually need to make further changes and forget to update both locations. It is tempting to just nuke the leading_directory special casing to avoid such bugs and simplify the code, but unpack_trees' verify_clean_subdirectory() also calls read_directory() and does so with a non-empty leading path, so I'm hesitant to try to restructure further. Add obnoxious warnings to treat_leading_path() and read_directory_recursive() to try to warn people of such problems. You would think that with such a strongly worded description, that its author would have actually ensured that the logic in treat_leading_path() and read_directory_recursive() did actually match and that *everything* that was needed had at least been copied over at the time that this paragraph was written. But you'd be wrong, I messed it up by missing part of the logic. Copy the missing bits to fix the new final test in t7300. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16clean: demonstrate a bug with pathspecsLibravatar Derrick Stolee1-0/+9
b9670c1f5e (dir: fix checks on common prefix directory, 2019-12-19) modified the way pathspecs are handled when handling a directory during "git clean -f <path>". While this improved the behavior for known test breakages, it also regressed in how the clean command handles cleaning a specified file. Add a test case that demonstrates this behavior. This test passes before b9670c1f5e then fails after. Helped-by: Kevin Willford <Kevin.Willford@microsoft.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17clean: avoid removing untracked files in a nested git repositoryLibravatar Elijah Newren1-5/+5
Users expect files in a nested git repository to be left alone unless sufficiently forced (with two -f's). Unfortunately, in certain circumstances, git would delete both tracked (and possibly dirty) files and untracked files within a nested repository. To explain how this happens, let's contrast a couple cases. First, take the following example setup (which assumes we are already within a git repo): git init nested cd nested >tracked git add tracked git commit -m init >untracked cd .. In this setup, everything works as expected; running 'git clean -fd' will result in fill_directory() returning the following paths: nested/ nested/tracked nested/untracked and then correct_untracked_entries() would notice this can be compressed to nested/ and then since "nested/" is a directory, we would call remove_dirs("nested/", ...), which would check is_nonbare_repository_dir() and then decide to skip it. However, if someone also creates an ignored file: >nested/ignored then running 'git clean -fd' would result in fill_directory() returning the same paths: nested/ nested/tracked nested/untracked but correct_untracked_entries() will notice that we had ignored entries under nested/ and thus simplify this list to nested/tracked nested/untracked Since these are not directories, we do not call remove_dirs() which was the only place that had the is_nonbare_repository_dir() safety check -- resulting in us deleting both the untracked file and the tracked (and possibly dirty) file. One possible fix for this issue would be walking the parent directories of each path and checking if they represent nonbare repositories, but that would be wasteful. Even if we added caching of some sort, it's still a waste because we should have been able to check that "nested/" represented a nonbare repository before even descending into it in the first place. Add a DIR_SKIP_NESTED_GIT flag to dir_struct.flags and use it to prevent fill_directory() and friends from descending into nested git repos. With this change, we also modify two regression tests added in commit 91479b9c72f1 ("t7300: add tests to document behavior of clean and nested git", 2015-06-15). That commit, nor its series, nor the six previous iterations of that series on the mailing list discussed why those tests coded the expectation they did. In fact, it appears their purpose was simply to test _existing_ behavior to make sure that the performance changes didn't change the behavior. However, these two tests directly contradicted the manpage's claims that two -f's were required to delete files/directories under a nested git repository. While one could argue that the user gave an explicit path which matched files/directories that were within a nested repository, there's a slippery slope that becomes very difficult for users to understand once you go down that route (e.g. what if they specified "git clean -f -d '*.c'"?) It would also be hard to explain what the exact behavior was; avoid such problems by making it really simple. Also, clean up some grammar errors describing this functionality in the git-clean manpage. Finally, there are still a couple bugs with -ffd not cleaning out enough (e.g. missing the nested .git) and with -ffdX possibly cleaning out the wrong files (paying attention to outer .gitignore instead of inner). This patch does not address these cases at all (and does not change the behavior relative to those flags), it only fixes the handling when given a single -f. See https://public-inbox.org/git/20190905212043.GC32087@szeder.dev/ for more discussion of the -ffd[X?] bugs. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17clean: disambiguate the definition of -dLibravatar Elijah Newren1-0/+2
The -d flag pre-dated git-clean's ability to have paths specified. As such, the default for git-clean was to only remove untracked files in the current directory, and -d existed to allow it to recurse into subdirectories. The interaction of paths and the -d option appears to not have been carefully considered, as evidenced by numerous bugs and a dearth of tests covering such pairings in the testsuite. The definition turns out to be important, so let's look at some of the various ways one could interpret the -d option: A) Without -d, only look in subdirectories which contain tracked files under them; with -d, also look in subdirectories which are untracked for files to clean. B) Without specified paths from the user for us to delete, we need to have some kind of default, so...without -d, only look in subdirectories which contain tracked files under them; with -d, also look in subdirectories which are untracked for files to clean. The important distinction here is that choice B says that the presence or absence of '-d' is irrelevant if paths are specified. The logic behind option B is that if a user explicitly asked us to clean a specified pathspec, then we should clean anything that matches that pathspec. Some examples may clarify. Should git clean -f untracked_dir/file remove untracked_dir/file or not? It seems crazy not to, but a strict reading of option A says it shouldn't be removed. How about git clean -f untracked_dir/file1 tracked_dir/file2 or git clean -f untracked_dir_1/file1 untracked_dir_2/file2 ? Should it remove either or both of these files? Should it require multiple runs to remove both the files listed? (If this sounds like a crazy question to even ask, see the commit message of "t7300: Add some testcases showing failure to clean specified pathspecs" added earlier in this patch series.) What if -ffd were used instead of -f -- should that allow these to be removed? Should it take multiple invocations with -ffd? What if a glob (such as '*tracked*') were used instead of spelling out the directory names? What if the filenames involved globs, such as git clean -f '*.o' or git clean -f '*/*.o' ? The current documentation actually suggests a definition that is slightly different than choice A, and the implementation prior to this series provided something radically different than either choices A or B. (The implementation, though, was clearly just buggy). There may be other choices as well. However, for almost any given choice of definition for -d that I can think of, some of the examples above will appear buggy to the user. The only case that doesn't have negative surprises is choice B: treat a user-specified path as a request to clean all untracked files which match that path specification, including recursing into any untracked directories. Change the documentation and basic implementation to use this definition. There were two regression tests that indirectly depended on the current implementation, but neither was about subdirectory handling. These two tests were introduced in commit 5b7570cfb41c ("git-clean: add tests for relative path", 2008-03-07) which was solely created to add coverage for the changes in commit fb328947c8e ("git-clean: correct printing relative path", 2008-03-07). Both tests specified a directory that happened to have an untracked subdirectory, but both were only checking that the resulting printout of a file that was removed was shown with a relative path. Update these tests appropriately. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17dir: if our pathspec might match files under a dir, recurse into itLibravatar Elijah Newren1-2/+2
For git clean, if a directory is entirely untracked and the user did not specify -d (corresponding to DIR_SHOW_IGNORED_TOO), then we usually do not want to remove that directory and thus do not recurse into it. However, if the user manually specified specific (or even globbed) paths somewhere under that directory to remove, then we need to recurse into the directory to make sure we remove the relevant paths under that directory as the user requested. Note that this does not mean that the recursed-into directory will be added to dir->entries for later removal; as of a few commits earlier in this series, there is another more strict match check that is run after returning from a recursed-into directory before deciding to add it to the list of entries. Therefore, this will only result in files underneath the given directory which match one of the pathspecs being added to the entries list. Two notes of potential interest to future readers: * If we wanted to only recurse into a directory when it is specifically matched rather than matched-via-glob (e.g. '*.c'), then we could do so via making the final non-zero return in match_pathspec_item be MATCHED_RECURSIVELY instead of MATCHED_RECURSIVELY_LEADING_PATHSPEC. (Note that the relative order of MATCHED_RECURSIVELY_LEADING_PATHSPEC and MATCHED_RECURSIVELY are important for such a change.) I was leaving open that possibility while writing an RFC asking for the behavior we want, but even though we don't want it, that knowledge might help you understand the code flow better. * There is a growing amount of logic in read_directory_recursive() for deciding whether to recurse into a subdirectory. However, there is a comment immediately preceding this logic that says to recurse if instructed by treat_path(). It may be better for the logic in read_directory_recursive() to ultimately be moved to treat_path() (or another function it calls, such as treat_directory()), but I have left that for someone else to tackle in the future. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17dir: also check directories for matching pathspecsLibravatar Elijah Newren1-2/+2
Even if a directory doesn't match a pathspec, it is possible, depending on the precise pathspecs, that some file underneath it might. So we special case and recurse into the directory for such situations. However, we previously always added any untracked directory that we recursed into to the list of untracked paths, regardless of whether the directory itself matched the pathspec. For the case of git-clean and a set of pathspecs of "dir/file" and "more", this caused a problem because we'd end up with dir entries for both of "dir" "dir/file" Then correct_untracked_entries() would try to helpfully prune duplicates for us by removing "dir/file" since it's under "dir", leaving us with "dir" Since the original pathspec only had "dir/file", the only entry left doesn't match and leaves nothing to be removed. (Note that if only one pathspec was specified, e.g. only "dir/file", then the common_prefix_len optimizations in fill_directory would cause us to bypass this problem, making it appear in simple tests that we could correctly remove manually specified pathspecs.) Fix this by actually checking whether the directory we are about to add to the list of dir entries actually matches the pathspec; only do this matching check after we have already returned from recursing into the directory. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-17t7300: add testcases showing failure to clean specified pathspecsLibravatar Elijah Newren1-0/+32
Someone brought me a testcase where multiple git-clean invocations were required to clean out unwanted files: mkdir d{1,2} touch d{1,2}/ut touch d1/t && git add d1/t With this setup, the user would need to run git clean -ffd */ut twice to delete both ut files. A little testing showed some interesting variants: * If only one of those two ut files existed (either one), then only one clean command would be necessary. * If both directories had tracked files, then only one git clean would be necessary to clean both files. * If both directories had no tracked files then the clean command above would never clean either of the untracked files despite the pathspec explicitly calling both of them out. A bisect showed that the failure to clean out the files started with commit cf424f5fd89b ("clean: respect pathspecs with "-d", 2014-03-10). However, that pointed to a separate issue: while the "-d" flag was used by the original user who showed me this problem, that flag should have been irrelevant to this problem. Testing again without the "-d" flag showed that the same buggy behavior exists without using that flag, and has in fact existed since before cf424f5fd89b. Although these problems at first are perceived to be different (e.g. never clearing out the requested files vs. taking multiple invocations to get everything cleared out), they are actually just different manifestations of the same problem. The case with multiple directories that have no tracked files is the more general case; solving it will solve all the others. So, I concentrate on it. Add testcases showing that multiple untracked files within entirely untracked directories cannot be cleaned when specifying these files to git clean via pathspecs. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-26t7300-clean: demonstrate deleting nested repo with an ignored file breakageLibravatar SZEDER Gábor1-0/+22
'git clean -fd' must not delete an untracked directory if it belongs to a different Git repository or worktree. Unfortunately, if a '.gitignore' rule in the outer repository happens to match a file in a nested repository or worktree, then something goes awry and 'git clean -fd' does delete the content of the nested repository's worktree except that ignored file, potentially leading to data loss. Add a test to 't7300-clean.sh' to demonstrate this breakage. This issue is a regression introduced in 6b1db43109 (clean: teach clean -d to preserve ignored paths, 2017-05-23). Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-19clean: show an error message when the path is too longLibravatar Johannes Schindelin1-0/+12
When `lstat()` failed, `git clean` would abort without an error message, leaving the user quite puzzled. In particular on Windows, where the default maximum path length is quite small (yet there are ways to circumvent that limit in many cases), it is very important that users be given an indication why their command failed because of too long paths when it did. This test case makes sure that a warning is issued that would have helped the user who reported this issue: https://github.com/git-for-windows/git/issues/521 Note that we temporarily set `core.longpaths = false` in the regression test; this ensures forward-compatibility with the `core.longpaths` feature that has not yet been upstreamed from Git for Windows. Helped-by: René Scharfe <l.s.r@web.de> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-24clean: teach clean -d to preserve ignored pathsLibravatar Samuel Lijin1-1/+1
There is an implicit assumption that a directory containing only untracked and ignored paths should itself be considered untracked. This makes sense in use cases where we're asking if a directory should be added to the git database, but not when we're asking if a directory can be safely removed from the working tree; as a result, clean -d would assume that an "untracked" directory containing ignored paths could be deleted, even though doing so would also remove the ignored paths. To get around this, we teach clean -d to collect ignored paths and skip an untracked directory if it contained an ignored path, instead just removing the untracked contents thereof. To achieve this, cmd_clean() has to collect all untracked contents of untracked directories, in addition to all ignored paths, to determine which untracked dirs must be skipped (because they contain ignored paths) and which ones should *not* be skipped. For this purpose, correct_untracked_entries() is introduced to prune a given dir_struct of untracked entries containing ignored paths and those untracked entries encompassed by the untracked entries which are not pruned away. A memory leak is also fixed in cmd_clean(). This also fixes the known breakage in t7300, since clean -d now skips untracked directories containing ignored paths. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-22t7300: clean -d should skip dirs with ignored filesLibravatar Samuel Lijin1-0/+16
If git sees a directory which contains only untracked and ignored files, clean -d should not remove that directory. It was recently discovered that this is *not* true of git clean -d, and it's possible that this has never worked correctly; this test and its accompanying patch series aims to fix that. Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-03t7300: mark test with SANITYLibravatar Stefan Beller1-1/+1
The test runs `chmod 0` on a file to test a case where Git fails to read it, but that would not work if it is run as root. Reported-by: Jan Keromnes <janx@linux.com> Fix-proposed-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-02Merge branch 'ee/clean-test-fixes'Libravatar Junio C Hamano1-12/+4
* ee/clean-test-fixes: t7300: fix broken && chains
2015-08-31t7300: fix broken && chainsLibravatar Erik Elfström1-12/+4
While we are here, remove some boilerplate by using test_commit. Signed-off-by: Erik Elfström <erik.elfstrom@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11t7300-clean: require POSIXPERM for chmod 0 testLibravatar Johannes Sixt1-1/+1
A test case introduced by 91479b9c (t7300: add tests to document behavior of clean and nested git) uses 'chmod 0' to verify that a subdirectory that has an unreadable .git file is not removed. This can work only when the system pays attention to the permissions set with 'chmod'. Therefore, set the POSIXPERM prerequisite on the test case. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-15clean: improve performance when removing lots of directoriesLibravatar Erik Elfström1-6/+4
"git clean" uses resolve_gitlink_ref() to check for the presence of nested git repositories, but it has the drawback of creating a ref_cache entry for every directory that should potentially be cleaned. The linear search through the ref_cache list causes a massive performance hit for large number of directories. Modify clean.c:remove_dirs to use setup.c:is_git_directory and setup.c:read_gitfile_gently instead. Both these functions will open files and parse contents when they find something that looks like a git repository. This is ok from a performance standpoint since finding repository candidates should be comparatively rare. Using is_git_directory and read_gitfile_gently should give a more standardized check for what is and what isn't a git repository but also gives three behavioral changes. The first change is that we will now detect and avoid cleaning empty nested git repositories (only init run). This is desirable. Second, we will no longer die when cleaning a file named ".git" with garbage content (it will be cleaned instead). This is also desirable. The last change is that we will detect and avoid cleaning empty bare repositories that have been placed in a directory named ".git". This is not desirable but should have no real user impact since we already fail to clean non-empty bare repositories in the same scenario. This is thus deemed acceptable. On top of this we add some extra precautions. If read_gitfile_gently fails to open the git file, read the git file or verify the path in the git file we assume that the path with the git file is a valid repository and avoid cleaning. Update t7300 to reflect these changes in behavior. The time to clean an untracked directory containing 100000 sub directories went from 61s to 1.7s after this change. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Erik Elfström <erik.elfstrom@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-15t7300: add tests to document behavior of clean and nested gitLibravatar Erik Elfström1-0/+142
Signed-off-by: Erik Elfström <erik.elfstrom@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-20t: use verbose instead of hand-rolled errorsLibravatar Jeff King1-8/+2
Many tests that predate the "verbose" helper function use a pattern like: test ... || { echo ... false } to give more verbose output. Using the helper, we can do this with a single line, and avoid a || which interacts badly with &&-chaining (besides fooling --chain-lint, we hit the error block no matter which command in the chain failed, so we may often show useless results). In most cases, the messages printed by "verbose" are equally good (in some cases better; t6006 accidentally redirects the message to a file!). The exception is t7001, whose output suffers slightly. However, it's still enough to show the user which part failed, given that we will have just printed the test script to stderr. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-02Merge branch 'maint-1.8.5' into maintLibravatar Junio C Hamano1-2/+2
* maint-1.8.5: t7300: repair filesystem permissions with test_when_finished enums: remove trailing ',' after last item in enum
2014-07-02t7300: repair filesystem permissions with test_when_finishedLibravatar Jeff King1-2/+2
We create a directory that cannot be removed, confirm that it cannot be removed, and then fix it like: chmod 0 foo && test_must_fail git clean -d -f && chmod 755 foo If the middle step fails but leaves the directory (e.g., the bug is that clean does not notice the failure), this pollutes the test repo with an unremovable directory. Not only does this cause further tests to fail, but it means that "rm -rf" fails on the whole trash directory, and the user has to intervene manually to even re-run the test script. We can bump the "chmod 755" recovery to a test_when_finished block to be sure that it always runs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-11clean: respect pathspecs with "-d"Libravatar Jeff King1-0/+16
git-clean uses read_directory to fill in a `struct dir` with potential hits. However, read_directory does not actually check against our pathspec. It uses a simplified version that may turn up false positives. As a result, we need to check that any hits match our pathspec. We do so reliably for non-directories. For directories, if "-d" is not given we check that the pathspec matched exactly (i.e., we are even stricter, and require an explicit "git clean foo" to clean "foo/"). But if "-d" is given, rather than relaxing the exact match to allow a recursive match, we do not check the pathspec at all. This regression was introduced in 113f10f (Make git-clean a builtin, 2007-11-11). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-15dir.c: git-clean -d -X: don't delete tracked directoriesLibravatar Karsten Blees1-0/+34
The notion of "ignored tracked" directories introduced in 721ac4ed "dir.c: Make git-status --ignored more consistent" has a few unwanted side effects: - git-clean -d -X: deletes ignored tracked directories. git-clean should never delete tracked content. - git-ls-files --ignored --other --directory: lists ignored tracked directories instead of "other" directories. - git-status --ignored: lists ignored tracked directories while contained files may be listed as modified. Paths listed by git-status should be disjoint (except in long format where a path may be listed in both the staged and unstaged section). Additionally, the current behaviour violates documentation in gitignore(5) ("Specifies intentionally *untracked* files to ignore") and Documentation/ technical/api-directory-listing.txt ("DIR_SHOW_OTHER_DIRECTORIES: Include a directory that is *not tracked*."). In dir.c::treat_directory, remove the special handling of ignored tracked directories, so that the DIR_SHOW_OTHER_DIRECTORIES flag only affects "other" (i.e. untracked) directories. In dir.c::dir_add_name, check that added paths are untracked even if DIR_SHOW_IGNORED is set. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-03-15clean: preserve nested git worktree in subdirectoriesLibravatar Junio C Hamano1-5/+22
remove_dir_recursively() has a check to avoid removing the directory it was asked to remove without recursing into it and report success when the directory is the top level of a working tree of a nested git repository, to protect such a repository from "clean -f" (without double -f). If a working tree of a nested git repository is in a subdirectory of a toplevel project, however, this protection did not apply by mistake; we forgot to pass the REMOVE_DIR_KEEP_NESTED_GIT down to the recursive removal codepath. This requires us to also teach the higher level not to remove the directory it is asked to remove, when the recursed invocation did not remove the directory it was asked to remove due to a nested git repository, as it is not an error to leave the parent directories of such a nested repository. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-04-27Merge branch 'ar/clean-rmdir-empty'Libravatar Junio C Hamano1-0/+7
* ar/clean-rmdir-empty: clean: unreadable directory may still be rmdir-able if it is empty
2011-04-01clean: unreadable directory may still be rmdir-able if it is emptyLibravatar Alex Riesen1-0/+7
As a last ditch effort, try rmdir(2) when we cannot read the directory to be removed. It may be an empty directory that we can remove without any permission, as long as we can modify its parent directory. Noticed by Linus. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-03-09i18n: git-clean basic messagesLibravatar Ævar Arnfjörð Bjarmason1-3/+3
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-26t7300: add a missing SYMLINKS prerequisiteLibravatar Johannes Sixt1-1/+1
The test fails on Windows since 2dec68c (tests: add missing &&, batch 2). Even though this test allocates and leaves behind files, subsequent tests do not depend on this, so it is safe to just skip it. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-09tests: add missing &&, batch 2Libravatar Jonathan Nieder1-1/+1
Same rules as before: this patch only adds " &&" to the end of some lines in the test suite. Intended to be applied on top of or squashed with the last batch if they look okay. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-17t/t7300: workaround ancient touch by rearranging argumentsLibravatar Brandon Casey1-1/+1
The ancient touch on Solaris 7 thinks that a decimal number supplied as the first argument specifies a date_time to give to the files specified by the remaining arguments. In this case, it fails to parse '1' as a proper date_time and exits with a failure status. Workaround this flaw by rearranging the arguments supplied to touch so that a non-digit appears first and touch will not be confused. Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-04Merge branch 'ab/test-2'Libravatar Junio C Hamano1-4/+3
* ab/test-2: (51 commits) tests: factor HOME=$(pwd) in test-lib.sh test-lib: use subshell instead of cd $new && .. && cd $old tests: simplify "missing PREREQ" message t/t0000-basic.sh: Run the passing TODO test inside its own test-lib test-lib: Allow overriding of TEST_DIRECTORY test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ test: Introduce $GIT_BUILD_DIR cvs tests: do not touch test CVS repositories shipped with source t/t9602-cvsimport-branches-tags.sh: Add a PERL prerequisite t/t9601-cvsimport-vendor-branch.sh: Add a PERL prerequisite t/t7105-reset-patch.sh: Add a PERL prerequisite t/t9001-send-email.sh: convert setup code to tests t/t9001-send-email.sh: change from skip_all=* to prereq skip t/t9001-send-email.sh: Remove needless PROG=* assignment t/t9600-cvsimport.sh: change from skip_all=* to prereq skip lib-patch-mode tests: change from skip_all=* to prereq skip t/t3701-add-interactive.sh: change from skip_all=* to prereq skip tests: Move FILEMODE prerequisite to lib-prereq-FILEMODE.sh t/Makefile: Create test-results dir for smoke target ... Conflicts: t/t6035-merge-dir-to-symlink.sh
2010-08-18tests: A SANITY test prereq for testing if we're rootLibravatar Ævar Arnfjörð Bjarmason1-4/+3
Some tests depend on not being able to write to files after chmod -w. This doesn't work when running the tests as root. Change test-lib.sh to test if this works, and if so it sets a new SANITY test prerequisite. The tests that use this previously failed when run under root. There was already a test for this in t3600-rm.sh, added by Junio C Hamano in 2283645 in 2006. That check now uses the new SANITY prerequisite. Some of this was resurrected from the "Tests in Cygwin" thread in May 2009: http://thread.gmane.org/gmane.comp.version-control.git/116729/focus=118385 Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-07-20Add test for git clean -e.Libravatar Jared Hance1-0/+16
Signed-off-by: Jared Hance <jaredhance@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-13Merge branch 'nd/sparse'Libravatar Junio C Hamano1-0/+19
* nd/sparse: (25 commits) t7002: test for not using external grep on skip-worktree paths t7002: set test prerequisite "external-grep" if supported grep: do not do external grep on skip-worktree entries commit: correctly respect skip-worktree bit ie_match_stat(): do not ignore skip-worktree bit with CE_MATCH_IGNORE_VALID tests: rename duplicate t1009 sparse checkout: inhibit empty worktree Add tests for sparse checkout read-tree: add --no-sparse-checkout to disable sparse checkout support unpack-trees(): ignore worktree check outside checkout area unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout unpack-trees.c: generalize verify_* functions unpack-trees(): add CE_WT_REMOVE to remove on worktree alone Introduce "sparse checkout" dir.c: export excluded_1() and add_excludes_from_file_1() excluded_1(): support exclude files in index unpack-trees(): carry skip-worktree bit over in merged_entry() Read .gitignore from index if it is skip-worktree Avoid writing to buffer in add_excludes_from_file_1() ... Conflicts: .gitignore Documentation/config.txt Documentation/git-update-index.txt Makefile entry.c t/t7002-grep.sh
2009-08-23Read .gitignore from index if it is skip-worktreeLibravatar Nguyễn Thái Ngọc Duy1-0/+19
This adds index as a prerequisite for directory listing (with exclude). At the moment directory listing is used by "git clean", "git add", "git ls-files" and "git status"/"git commit" and unpack_trees()-related commands. These commands have been checked/modified to populate index before doing directory listing. add_excludes_from_file() does not enable this feature, because it is used to read .git/info/exclude and some explicit files specified by "git ls-files". Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-07-29clean: require double -f options to nuke nested git repository and work treeLibravatar Junio C Hamano1-0/+39
When you have an embedded git work tree in your work tree (be it an orphaned submodule, or an independent checkout of an unrelated project), "git clean -d -f" blindly descended into it and removed everything. This is rarely what the user wants. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-03-19t7300: fix clean up on WindowsLibravatar Johannes Schindelin1-3/+3
On Windows, you cannot remove files that are in use, not even with 'rm -rf'. So we need to run 'exec <foo/bar' inside a subshell lest removing the whole test repository fail. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
2008-09-03tests: use "git xyzzy" form (t7200 - t9001)Libravatar Nanako Shiraishi1-36/+36
Converts tests between t7201-t9001. Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-13t/: Use "test_must_fail git" instead of "! git"Libravatar Stephan Beyer1-2/+2
This patch changes every occurrence of "! git" -- with the meaning that a git call has to gracefully fail -- into "test_must_fail git". This is useful to - make sure the test does not fail because of a signal, e.g. SIGSEGV, and - advertise the use of "test_must_fail" for new tests. Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-05-05Fix tests breaking when checkout path contains shell metacharactersLibravatar Bryan Donlan1-1/+1
This fixes the remainder of the issues where the test script itself is at fault for failing when the git checkout path contains whitespace or other shell metacharacters. The majority of git svn tests used the idiom test_expect_success "title" "test script using $svnrepo" These were changed to have the test script in single-quotes: test_expect_success "title" 'test script using "$svnrepo"' which unfortunately makes the patch appear larger than it really is. One consequence of this change is that in the verbose test output the value of $svnrepo (and in some cases other variables, too) is no longer expanded, i.e. previously we saw * expecting success: test script using /path/to/git/t/trash/svnrepo but now it is: * expecting success: test script using "$svnrepo" Signed-off-by: Bryan Donlan <bdonlan@fushizen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-04-14git clean: Add test to verify directories aren't removed with a prefixLibravatar Shawn Bohrer1-2/+3
Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-03-07git-clean: add tests for relative pathLibravatar Junio C Hamano1-0/+52
This adds tests for recent change by Dmitry to fix the report "git clean" gives on removed paths, and also makes sure the command detects paths that is outside working tree. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-03-03Fix test for cleanup failure in t7300 on WindowsLibravatar Alex Riesen1-1/+2
Keep the file open to: the OS does not allow removal of open files. The saner systems just have a saner permission model and chmod 0 is enough for the test. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-02-20git-clean: handle errors if removing files failsLibravatar Miklos Vajna1-0/+10
git-clean simply ignored errors if removing a file or directory failed. This patch makes it raise a warning and the exit code also greater than zero if there are remaining files. Signed-off-by: Miklos Vajna <vmiklos@frugalware.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-12-05t7300: add test for clean with wildcard pathspecLibravatar Jeff King1-0/+14
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-11-14Merge branch 'maint'Libravatar Junio C Hamano1-0/+11
* maint: git-clean: honor core.excludesfile Documentation: Fix man page breakage with DocBook XSL v1.72 git-remote.txt: fix typo core-tutorial.txt: Fix argument mistake in an example. replace reference to git-rm with git-reset in git-commit doc Grammar fixes for gitattributes documentation Don't allow fast-import tree delta chains to exceed maximum depth revert/cherry-pick: allow starting from dirty work tree. t/t3404: fix test for a bogus todo file. Conflicts: fast-import.c