diff mbox series

[4/6] unpack-trees: avoid nuking untracked dir in way of locally deleted file

Message ID 01bf850bb0f0796564e9363d7faeb792a594f684.1632006924.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Fix various issues around removal of untracked files/directories | expand

Commit Message

Elijah Newren Sept. 18, 2021, 11:15 p.m. UTC 
From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t2500-untracked-overwriting.sh | 2 +-
 unpack-trees.c                   | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason Sept. 19, 2021, 1:52 p.m. UTC | #1 
On Sat, Sep 18 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  t/t2500-untracked-overwriting.sh | 2 +-
>  unpack-trees.c                   | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh
> index 017946a494f..d4d9dc928aa 100755
> --- a/t/t2500-untracked-overwriting.sh
> +++ b/t/t2500-untracked-overwriting.sh
> @@ -218,7 +218,7 @@ test_expect_success 'git am --abort and untracked dir vs. unmerged file' '
>  	)
>  '
>  
> -test_expect_failure 'git am --skip and untracked dir vs deleted file' '
> +test_expect_success 'git am --skip and untracked dir vs deleted file' '
>  	test_setup_sequencing am_skip_and_untracked &&
>  	(
>  		cd sequencing_am_skip_and_untracked &&
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3b3d1c0ff40..858595a13f1 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2395,7 +2395,11 @@ static int deleted_entry(const struct cache_entry *ce,
>  		if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
>  			return -1;
>  		return 0;
> +	} else {
> +		if (verify_absent_if_directory(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
> +			return -1;
>  	}

Maybe just "else if" ?

[...]

> +

Stray whitespace change
Elijah Newren Sept. 20, 2021, 4:12 p.m. UTC | #2 
On Sun, Sep 19, 2021 at 6:52 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Sat, Sep 18 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  t/t2500-untracked-overwriting.sh | 2 +-
> >  unpack-trees.c                   | 4 ++++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh
> > index 017946a494f..d4d9dc928aa 100755
> > --- a/t/t2500-untracked-overwriting.sh
> > +++ b/t/t2500-untracked-overwriting.sh
> > @@ -218,7 +218,7 @@ test_expect_success 'git am --abort and untracked dir vs. unmerged file' '
> >       )
> >  '
> >
> > -test_expect_failure 'git am --skip and untracked dir vs deleted file' '
> > +test_expect_success 'git am --skip and untracked dir vs deleted file' '
> >       test_setup_sequencing am_skip_and_untracked &&
> >       (
> >               cd sequencing_am_skip_and_untracked &&
> > diff --git a/unpack-trees.c b/unpack-trees.c
> > index 3b3d1c0ff40..858595a13f1 100644
> > --- a/unpack-trees.c
> > +++ b/unpack-trees.c
> > @@ -2395,7 +2395,11 @@ static int deleted_entry(const struct cache_entry *ce,
> >               if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
> >                       return -1;
> >               return 0;
> > +     } else {
> > +             if (verify_absent_if_directory(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
> > +                     return -1;
> >       }
>
> Maybe just "else if" ?

Yeah, that makes sense.

> [...]

That's kind of misleading.  ;-)  You trimmed out a single line here,
and in particular one that only contained a trailing curly brace.
Thus, your "trimming" here actually made things longer.

>
> > +
>
> Stray whitespace change

No, the whitespace addition was after making the if-block above it
more complex with the extra else block.  That if-block is now
approximately 2/3 of the length of the function, and is the part that
is relevant to the comment above it.  Since the code that follows the
if-block is separate from the comment above and the if-block became
more complex, it felt natural to add a bit of spacing.  So, it wasn't
stray, but intentional and related to the changes above.
diff mbox series

Patch

diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh
index 017946a494f..d4d9dc928aa 100755
--- a/t/t2500-untracked-overwriting.sh
+++ b/t/t2500-untracked-overwriting.sh
@@ -218,7 +218,7 @@  test_expect_success 'git am --abort and untracked dir vs. unmerged file' '
 	)
 '
 
-test_expect_failure 'git am --skip and untracked dir vs deleted file' '
+test_expect_success 'git am --skip and untracked dir vs deleted file' '
 	test_setup_sequencing am_skip_and_untracked &&
 	(
 		cd sequencing_am_skip_and_untracked &&
diff --git a/unpack-trees.c b/unpack-trees.c
index 3b3d1c0ff40..858595a13f1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2395,7 +2395,11 @@  static int deleted_entry(const struct cache_entry *ce,
 		if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
 			return -1;
 		return 0;
+	} else {
+		if (verify_absent_if_directory(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
+			return -1;
 	}
+
 	if (!(old->ce_flags & CE_CONFLICTED) && verify_uptodate(old, o))
 		return -1;
 	add_entry(o, ce, CE_REMOVE, 0);