| Message ID | 20190912221240.18057-2-newren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix some git clean issues | expand |
Elijah Newren <newren@gmail.com> writes: > +test_expect_failure 'git clean handles being told what to clean' ' > + mkdir -p d1 d2 && > + touch d1/ut d2/ut && > + git clean -f */ut && > + test_path_is_missing d1/ut && > + test_path_is_missing d2/ut > +' Looks like d1 and d2 are new directories and the paths we see in the test are the only ones that are involved (i.e. we do not rely on any leftover cruft in d[12]/ from previous tests). If so, perhaps it is easier to follow by starting the tests with "rm -fr d1 d2 &&" or something to assure the readers of the script (not this patch, but the resulting file down the road) about the isolation? The same comment applies to the remainder. Also, you talked about tracked paths in the proposed log message; do they not participate in reproducing the issue(s)? Thanks. > +test_expect_failure 'git clean handles being told what to clean, with -d' ' > + mkdir -p d1 d2 && > + touch d1/ut d2/ut && > + git clean -ffd */ut && > + test_path_is_missing d1/ut && > + test_path_is_missing d2/ut > +' > + > +test_expect_failure 'git clean works if a glob is passed without -d' ' > + mkdir -p d1 d2 && > + touch d1/ut d2/ut && > + git clean -f "*ut" && > + test_path_is_missing d1/ut && > + test_path_is_missing d2/ut > +' > + > +test_expect_failure 'git clean works if a glob is passed with -d' ' > + mkdir -p d1 d2 && > + touch d1/ut d2/ut && > + git clean -ffd "*ut" && > + test_path_is_missing d1/ut && > + test_path_is_missing d2/ut > +' > + > test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' > test_config core.longpaths false && > a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
On Fri, Sep 13, 2019 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > > +test_expect_failure 'git clean handles being told what to clean' ' > > + mkdir -p d1 d2 && > > + touch d1/ut d2/ut && > > + git clean -f */ut && > > + test_path_is_missing d1/ut && > > + test_path_is_missing d2/ut > > +' > > Looks like d1 and d2 are new directories and the paths we see in the > test are the only ones that are involved (i.e. we do not rely on any > leftover cruft in d[12]/ from previous tests). If so, perhaps it is > easier to follow by starting the tests with "rm -fr d1 d2 &&" or > something to assure the readers of the script (not this patch, but > the resulting file down the road) about the isolation? The same > comment applies to the remainder. Makes sense. > Also, you talked about tracked paths in the proposed log message; do > they not participate in reproducing the issue(s)? If there is only one directory which has no tracked files, then the user can clean up the files -- but confusingly, they have to issue the same git-clean command multiple times. If multiple directories have no untracked files, git-clean will never clean them out. I probably didn't do a very good job explaining that although I started with the case with one tracked, that I view the case without any as the more general case -- and that solving it solves both problems. I could probably make that clearer in the commit message. (Or maybe just add more testcases even if slightly duplicative, I guess.)
Elijah Newren <newren@gmail.com> writes: >> Also, you talked about tracked paths in the proposed log message; do >> they not participate in reproducing the issue(s)? > > If there is only one directory which has no tracked files, then the > user can clean up the files -- but confusingly, they have to issue the > same git-clean command multiple times. If multiple directories have > no untracked files, git-clean will never clean them out. I probably > didn't do a very good job explaining that although I started with the > case with one tracked, that I view the case without any as the more > general case -- and that solving it solves both problems. I could > probably make that clearer in the commit message. (Or maybe just add > more testcases even if slightly duplicative, I guess.) My comment/puzzlement indeed was the lack of any tracked file in your tests, even though the log message did talk about one's presence making a difference in the outcome. Thanks.
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index d01fd120ab..2c254c773c 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -691,6 +691,38 @@ test_expect_failure 'git clean -d skips nested repo containing ignored files' ' test_path_is_file nested-repo-with-ignored-file/file ' +test_expect_failure 'git clean handles being told what to clean' ' + mkdir -p d1 d2 && + touch d1/ut d2/ut && + git clean -f */ut && + test_path_is_missing d1/ut && + test_path_is_missing d2/ut +' + +test_expect_failure 'git clean handles being told what to clean, with -d' ' + mkdir -p d1 d2 && + touch d1/ut d2/ut && + git clean -ffd */ut && + test_path_is_missing d1/ut && + test_path_is_missing d2/ut +' + +test_expect_failure 'git clean works if a glob is passed without -d' ' + mkdir -p d1 d2 && + touch d1/ut d2/ut && + git clean -f "*ut" && + test_path_is_missing d1/ut && + test_path_is_missing d2/ut +' + +test_expect_failure 'git clean works if a glob is passed with -d' ' + mkdir -p d1 d2 && + touch d1/ut d2/ut && + git clean -ffd "*ut" && + test_path_is_missing d1/ut && + test_path_is_missing d2/ut +' + test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' test_config core.longpaths false && a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
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. 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> --- t/t7300-clean.sh | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)