Re: [PATCH] stash: honor --no-overwrite-ignore when updating index
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 2, 2026, 14:20 UTC
- Message-ID
- <aYCyk02vG8ObH02j@pks.im>
- In-Reply-To
- <20260202131921.15175-2-pushkarkumarsingh1970@gmail.com>
On Mon, Feb 02, 2026 at 01:19:22PM +0000, Pushkar Singh wrote:
Show 7 quoted lines
> The stash code unconditionally cleared opts.preserve_ignored when > updating the index, leaving a FIXME suggesting this should depend on > an overwrite_ignore flag. > > Introduce overwrite_ignore plumbing for git stash push/save and use it > to control preserve_ignored during reset_tree(). Add a test to verify > that --no-overwrite-ignore preserves ignored files.
It's somewhat surprising that this requires so little code changes. Do the mailing list archives yield any justification for why specifically this feature wasn't implemented?
> This removes the long-standing FIXME and aligns stash behavior with > checkout/reset/merge.
Missing signoff.
Show 15 quoted lines
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 193e3ea47a..82d10520fe 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1856,6 +1857,10 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> N_("include untracked files in stash")),
> 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")),
> OPT_STRING('m', "message", &stash_msg, N_("message"),
> N_("stash message")),
> OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),`OPT_BOOL()` already handles both the positive and negative case, so there's no need to specify both here. Furthermore, both of your options actually do the exact same thing.
Show 11 quoted lines
> @@ -1959,6 +1964,10 @@ static int save_stash(int argc, const char **argv, const char *prefix,
> N_("include untracked files in stash")),
> 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")),
> OPT_STRING('m', "message", &stash_msg, "message",
> N_("stash message")),
> OPT_END()Same here.
Show 10 quoted lines
> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh > index 7704709054..9c5421cd76 100755 > --- a/t/t3905-stash-include-untracked.sh > +++ b/t/t3905-stash-include-untracked.sh > @@ -427,4 +427,17 @@ test_expect_success 'stash -u ignores sub-repository' ' > git stash -u > ' > > +test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' ' > + echo ignored.txt >>.gitignore &&
Is there any specific reason why we append instead of overwriting the gitignore file? Overwriting would probably be preferred so that it's easier to reason about the test without requiring context around what the current contents of this file are.
Show 9 quoted lines
> + echo before >ignored.txt && > + git add .gitignore && > + git commit -m "add ignore" && > + > + echo after >ignored.txt && > + git stash push --no-overwrite-ignore && > + > + test_path_is_file ignored.txt && > + grep after ignored.txt
I think another good step would be to verify that `git stash push --overwrite-ignore` _would_ cause us to overwrite the file.
I guess this test only happens to work because the first option takes precedence over the ambiguous second one?
Patrick