Re: [PATCH] stash: honor --no-overwrite-ignore when updating index

[Patrick Steinhardt <ps@pks.im>]

On Mon, Feb 02, 2026 at 01:19:22PM +0000, Pushkar Singh wrote:
> 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.

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

> @@ -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.

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

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