t7300-clean: demonstrate deleting nested repo with an ignored file breakage
diff mbox series

Message ID 20190825185918.3909-1-szeder.dev@gmail.com
State New
Headers show
Series
  • t7300-clean: demonstrate deleting nested repo with an ignored file breakage
Related show

Commit Message

SZEDER Gábor Aug. 25, 2019, 6:59 p.m. UTC
'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(+)

Comments

SZEDER Gábor Aug. 25, 2019, 8:34 p.m. UTC | #1
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.)
Philip Oakley Aug. 25, 2019, 10:32 p.m. UTC | #2
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 &&
SZEDER Gábor Aug. 26, 2019, 7:48 a.m. UTC | #3
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 &&
>

Patch
diff mbox series

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 &&