| Message ID | 20190825185918.3909-1-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t7300-clean: demonstrate deleting nested repo with an ignored file breakage | expand |
On Sun, Aug 25, 2019 at 08:59:18PM +0200, SZEDER Gábor wrote: > '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> > --- > > BEWARE: Our toplevel '.gitignore' currently contains the '*.manifest' > rule [1], which ignores the file 'compat/win32/git.manifest' [2], so > if you use nested worktrees in your git repo, then a 'git clean -fd' > will delete them. OK, singling out that manifest file is just nonsense, any object file, etc... in the nested worktree/repo can trigger the same issue just as well. (It just so happened that when I ran 'git clean -fd' I had a nested worktree where I haven't build anything yet, and besides the .git file only that 'git.manifest' file survived in the nested worktree, and then I got misled by it.)
Hi Szeder, On 25/08/2019 19:59, SZEDER Gábor wrote: > 'git clean -fd' must not delete an untracked directory if it belongs s/untracked// I don't believe it should matter either way for a sub-module (sub-directory). > to a different Git repository or worktree. msybr split the assertion from the fault explanation. > 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 good so far. > except that ignored file, potentially leading to data loss. this appears at cross purposes as the description has changed from 'ignored/untracked directory' to 'ignored file'. I'm not sure which part the data loss is meant to refer to. > > 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> > --- > > BEWARE: Our toplevel '.gitignore' currently contains the '*.manifest' > rule [1], which ignores the file 'compat/win32/git.manifest' [2], so > if you use nested worktrees in your git repo, then a 'git clean -fd' > will delete them. > > [1] 516dfb8416 (.gitignore: touch up the entries regarding Visual > Studio, 2019-07-29) > [2] fe90397604 (mingw: embed a manifest to trick UAC into Doing The > Right Thing, 2019-06-27) > > > t/t7300-clean.sh | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > index a2c45d1902..d01fd120ab 100755 > --- a/t/t7300-clean.sh > +++ b/t/t7300-clean.sh > @@ -669,6 +669,28 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files' > test_path_is_missing foo/b/bb > ' > > +test_expect_failure 'git clean -d skips nested repo containing ignored files' ' > + test_when_finished "rm -rf nested-repo-with-ignored-file" && > + > + git init nested-repo-with-ignored-file && > + ( > + cd nested-repo-with-ignored-file && > + >file && > + git add file && > + git commit -m Initial && > + > + # This file is ignored by a .gitignore rule in the outer repo > + # added in the previous test. > + >ignoreme > + ) && > + > + git clean -fd && > + > + test_path_is_file nested-repo-with-ignored-file/.git/index && > + test_path_is_file nested-repo-with-ignored-file/ignoreme && > + test_path_is_file nested-repo-with-ignored-file/file > +' > + > test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' > test_config core.longpaths false && > a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
On Sun, Aug 25, 2019 at 11:32:28PM +0100, Philip Oakley wrote: > Hi Szeder, > > On 25/08/2019 19:59, SZEDER Gábor wrote: > >'git clean -fd' must not delete an untracked directory if it belongs > s/untracked// > I don't believe it should matter either way for a sub-module > (sub-directory). I just paraphrased the documentation of the '-d' option for a bit of context. Remove untracked directories in addition to untracked files. If an untracked directory is managed by a different Git repository, it is not removed by default. Use -f option twice if you really want to remove such a directory. > >to a different Git repository or worktree. > msybr split the assertion from the fault explanation. > > 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 > good so far. > >except that ignored file, potentially leading to data loss. > this appears at cross purposes as the description has changed from > 'ignored/untracked directory' to 'ignored file'. The description does not mention any ignored directories. > I'm not sure which part the > data loss is meant to refer to. Well, there is only one part where the description talks about stuff getting deleted... and that's what it refers to :) > >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> > >--- On a related note, 'git clean -fdx' does leave the nested repository or worktree intact in the same situation, as it should. > > t/t7300-clean.sh | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > >diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > >index a2c45d1902..d01fd120ab 100755 > >--- a/t/t7300-clean.sh > >+++ b/t/t7300-clean.sh > >@@ -669,6 +669,28 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files' > > test_path_is_missing foo/b/bb > > ' > >+test_expect_failure 'git clean -d skips nested repo containing ignored files' ' > >+ test_when_finished "rm -rf nested-repo-with-ignored-file" && > >+ > >+ git init nested-repo-with-ignored-file && > >+ ( > >+ cd nested-repo-with-ignored-file && > >+ >file && > >+ git add file && > >+ git commit -m Initial && > >+ > >+ # This file is ignored by a .gitignore rule in the outer repo > >+ # added in the previous test. > >+ >ignoreme > >+ ) && > >+ > >+ git clean -fd && > >+ > >+ test_path_is_file nested-repo-with-ignored-file/.git/index && > >+ test_path_is_file nested-repo-with-ignored-file/ignoreme && > >+ test_path_is_file nested-repo-with-ignored-file/file > >+' > >+ > > test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' > > test_config core.longpaths false && > > a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && >
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index a2c45d1902..d01fd120ab 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -669,6 +669,28 @@ test_expect_success 'git clean -d skips untracked dirs containing ignored files' test_path_is_missing foo/b/bb ' +test_expect_failure 'git clean -d skips nested repo containing ignored files' ' + test_when_finished "rm -rf nested-repo-with-ignored-file" && + + git init nested-repo-with-ignored-file && + ( + cd nested-repo-with-ignored-file && + >file && + git add file && + git commit -m Initial && + + # This file is ignored by a .gitignore rule in the outer repo + # added in the previous test. + >ignoreme + ) && + + git clean -fd && + + test_path_is_file nested-repo-with-ignored-file/.git/index && + test_path_is_file nested-repo-with-ignored-file/ignoreme && + test_path_is_file nested-repo-with-ignored-file/file +' + test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' test_config core.longpaths false && a50=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
'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> --- BEWARE: Our toplevel '.gitignore' currently contains the '*.manifest' rule [1], which ignores the file 'compat/win32/git.manifest' [2], so if you use nested worktrees in your git repo, then a 'git clean -fd' will delete them. [1] 516dfb8416 (.gitignore: touch up the entries regarding Visual Studio, 2019-07-29) [2] fe90397604 (mingw: embed a manifest to trick UAC into Doing The Right Thing, 2019-06-27) t/t7300-clean.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)