Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Feb 2, 2026, 20:00 UTC
- Message-ID
- <CALnO6CDQiSo7QYnjUmwxgRJJ1=A15JZ5TTWaHKUMgfiMoJHsww@mail.gmail.com>
- In-Reply-To
- <20260202162225.35206-3-pushkarkumarsingh1970@gmail.com>
On Mon, Feb 2, 2026 at 11:37 AM Pushkar Singh <pushkarkumarsingh1970@gmail.com> wrote:
Show 39 quoted lines
>
> Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore
> is given by downgrading INCLUDE_ALL_FILES to include-untracked.
>
> This fixes ignored files being incorrectly removed despite
> --no-overwrite-ignore, and removes the stash FIXME by plumbing
> overwrite_ignore into unpack_trees().
>
> Add regression tests covering both overwrite and no-overwrite cases.
>
> Changes since v1:
> - Use OPT_BOOL correctly for overwrite-ignore.
> - Fix stash -a cleanup when --no-overwrite-ignore is given by downgrading
> INCLUDE_ALL_FILES to include-untracked.
> - Add regression test for --overwrite-ignore.
> - Adjust no-overwrite-ignore test to explicitly use -a.
> - Add Signed-off-by.
>
> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> ---
> builtin/stash.c | 14 ++++++++------
> t/t3905-stash-include-untracked.sh | 16 ++++++++++++++--
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 82d10520fe..c3ee33cce1 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1858,9 +1858,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> OPT_SET_INT('a', "all", &include_untracked,
> N_("include ignore files"), 2),
> OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore,
> - N_("update ignored files (default)")),
> - OPT_BOOL(0, "no-overwrite-ignore", &overwrite_ignore,
> - N_("do not update ignored files")),
> + N_("update ignored files")),
> OPT_STRING('m', "message", &stash_msg, N_("message"),
> N_("stash message")),
> OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),This doesn't apply on top of, say, the master branch; it looks like you generated this patch on top of the previous version?
Show 34 quoted lines
> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh > index 9c5421cd76..a979831a64 100755 > --- a/t/t3905-stash-include-untracked.sh > +++ b/t/t3905-stash-include-untracked.sh > @@ -427,17 +427,29 @@ test_expect_success 'stash -u ignores sub-repository' ' > git stash -u > ' > > -test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' ' > +test_expect_success 'stash push -a --no-overwrite-ignore preserves ignored files' ' > echo ignored.txt >>.gitignore && > echo before >ignored.txt && > git add .gitignore && > git commit -m "add ignore" && > > echo after >ignored.txt && > - git stash push --no-overwrite-ignore && > + git stash push -a --no-overwrite-ignore && > > test_path_is_file ignored.txt && > grep after ignored.txt > ' > > +test_expect_success 'stash push -a --overwrite-ignore overwrites ignored files' ' > + echo ignored.txt >>.gitignore && > + echo before >ignored.txt && > + git add .gitignore && > + git commit -m "add ignore" && > + > + echo after >ignored.txt && > + git stash push -a --overwrite-ignore && > + > + ! grep after ignored.txt > +'
After removing --overwrite-ignore from these 2 tests to run them on unmodified Git, the first one fails (good: exercising new feature and showing improvement) and the 2nd one succeeds (probably good: exercising existing behavior, but strange: see below).
Use test_grep instead of plain grep. For example, it reveals that in the first test, ignored.txt doesn't exist in current Git! (Which is expected with -a, although should be changed by this series).
For the second test, I think we're hitting a similar issue (test_grep complains ignored.txt doesn't exist, so while inverted grep would succeed, we actually want to see that the file doesn't exist, right?). Anyway, the current "! grep …" passes on current Git because it's not really the right test, plus "--overwrite-ignore" is the current behavior of "-a", so a modified version of this test _should_ pass on current Git. I think we want "test_path_is_missing" here?
-- D. Ben Knoble