{"thread":{"id":"64899","subject":"[PATCH] stash: honor --no-overwrite-ignore when updating index","startedAt":"2026-02-02T13:22:53Z","lastAt":"2026-02-03T20:07:27Z","messageCount":15,"participants":["Pushkar Singh","Karthik Nayak","Patrick Steinhardt","Kristoffer Haugsbakk","D. Ben Knoble","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"534972","messageId":"20260202131921.15175-2-pushkarkumarsingh1970@gmail.com","threadId":"64899","inReplyTo":null,"subject":"[PATCH] stash: honor --no-overwrite-ignore when updating index","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-02T13:19:22Z","receivedAt":"2026-02-02T13:22:53Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"The stash code unconditionally cleared opts.preserve_ignored when\nupdating the index, leaving a FIXME suggesting this should depend on\nan overwrite_ignore flag.\n\nIntroduce overwrite_ignore plumbing for git stash push/save and use it\nto control preserve_ignored during reset_tree(). Add a test to verify\nthat --no-overwrite-ignore preserves ignored files.\n\nThis removes the long-standing FIXME and aligns stash behavior with\ncheckout/reset/merge.\n---\n builtin/stash.c                    | 11 ++++++++++-\n t/t3905-stash-include-untracked.sh | 13 +++++++++++++\n 2 files changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 193e3ea47a..82d10520fe 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -150,6 +150,7 @@ static int show_stat = 1;\n static int show_patch;\n static int show_include_untracked;\n static int use_index;\n+static int overwrite_ignore = 1;\n \n /*\n  * w_commit is set to the commit containing the working tree\n@@ -360,7 +361,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n \topts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;\n \topts.update = update;\n \tif (update)\n-\t\topts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */\n+\t\topts.preserve_ignored = !overwrite_ignore;\n \topts.fn = oneway_merge;\n \n \tif (unpack_trees(nr_trees, t, &opts))\n@@ -1856,6 +1857,10 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \t\t\t N_(\"include untracked files in stash\")),\n \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n \t\t\t    N_(\"include ignore files\"), 2),\n+\t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n+\t\t\tN_(\"update ignored files (default)\")),\n+\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n+\t\t\tN_(\"do not update ignored files\")),\n \t\tOPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n \t\t\t   N_(\"stash message\")),\n \t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n@@ -1959,6 +1964,10 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n \t\t\t N_(\"include untracked files in stash\")),\n \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n \t\t\t    N_(\"include ignore files\"), 2),\n+\t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n+\t\t\t\tN_(\"update ignored files (default)\")),\n+\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n+\t\t\t\tN_(\"do not update ignored files\")),\n \t\tOPT_STRING('m', \"message\", &stash_msg, \"message\",\n \t\t\t   N_(\"stash message\")),\n \t\tOPT_END()\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 7704709054..9c5421cd76 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -427,4 +427,17 @@ test_expect_success 'stash -u ignores sub-repository' '\n \tgit stash -u\n '\n \n+test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n+\techo ignored.txt >>.gitignore &&\n+\techo before >ignored.txt &&\n+\tgit add .gitignore &&\n+\tgit commit -m \"add ignore\" &&\n+\n+\techo after >ignored.txt &&\n+\tgit stash push --no-overwrite-ignore &&\n+\n+\ttest_path_is_file ignored.txt &&\n+\tgrep after ignored.txt\n+'\n+\n test_done\n-- \n2.43.0\n\n"},{"id":"534975","messageId":"CAOLa=ZQCuka+cSuCu=KnTHm=gk1iJ_QJhDjy1Ku8WLfSgkGorw@mail.gmail.com","threadId":"64899","inReplyTo":"20260202131921.15175-2-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH] stash: honor --no-overwrite-ignore when updating index","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-02T14:10:35Z","receivedAt":"2026-02-02T14:10:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:\n\n> The stash code unconditionally cleared opts.preserve_ignored when\n> updating the index, leaving a FIXME suggesting this should depend on\n> an overwrite_ignore flag.\n>\n> Introduce overwrite_ignore plumbing for git stash push/save and use it\n> to control preserve_ignored during reset_tree(). Add a test to verify\n> that --no-overwrite-ignore preserves ignored files.\n>\n> This removes the long-standing FIXME and aligns stash behavior with\n> checkout/reset/merge.\n> ---\n>  builtin/stash.c                    | 11 ++++++++++-\n>  t/t3905-stash-include-untracked.sh | 13 +++++++++++++\n>  2 files changed, 23 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 193e3ea47a..82d10520fe 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -150,6 +150,7 @@ static int show_stat = 1;\n>  static int show_patch;\n>  static int show_include_untracked;\n>  static int use_index;\n> +static int overwrite_ignore = 1;\n>\n>  /*\n>   * w_commit is set to the commit containing the working tree\n> @@ -360,7 +361,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)\n>  \topts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;\n>  \topts.update = update;\n>  \tif (update)\n> -\t\topts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */\n> +\t\topts.preserve_ignored = !overwrite_ignore;\n>  \topts.fn = oneway_merge;\n>\n>  \tif (unpack_trees(nr_trees, t, &opts))\n> @@ -1856,6 +1857,10 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>  \t\t\t N_(\"include untracked files in stash\")),\n>  \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n>  \t\t\t    N_(\"include ignore files\"), 2),\n> +\t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> +\t\t\tN_(\"update ignored files (default)\")),\n> +\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> +\t\t\tN_(\"do not update ignored files\")),\n\nAdding an `OPT_BOOL` by default adds a [no] option.\n\nfrom Documentation/technical/api-parse-options.adoc:\n\n  `OPT_BOOL(short, long, &int_var, description)`::\n  \tIntroduce a boolean option. `int_var` is set to one with\n  \t`--option` and set to zero with `--no-option`.\n\nApart from that, isn't this plain wrong?? The '--overwrite-ignore' and\n'--no-overwrite-ignore' do the same thing here?\n\n>  \t\tOPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n>  \t\t\t   N_(\"stash message\")),\n>  \t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n> @@ -1959,6 +1964,10 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n>  \t\t\t N_(\"include untracked files in stash\")),\n>  \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n>  \t\t\t    N_(\"include ignore files\"), 2),\n> +\t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> +\t\t\t\tN_(\"update ignored files (default)\")),\n> +\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> +\t\t\t\tN_(\"do not update ignored files\")),\n\nHere too.\n\n>  \t\tOPT_STRING('m', \"message\", &stash_msg, \"message\",\n>  \t\t\t   N_(\"stash message\")),\n>  \t\tOPT_END()\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 7704709054..9c5421cd76 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -427,4 +427,17 @@ test_expect_success 'stash -u ignores sub-repository' '\n>  \tgit stash -u\n>  '\n>\n> +test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n> +\techo ignored.txt >>.gitignore &&\n> +\techo before >ignored.txt &&\n> +\tgit add .gitignore &&\n> +\tgit commit -m \"add ignore\" &&\n> +\n> +\techo after >ignored.txt &&\n> +\tgit stash push --no-overwrite-ignore &&\n> +\n> +\ttest_path_is_file ignored.txt &&\n> +\tgrep after ignored.txt\n> +'\n> +\n\nTo confirm, changing the test\n\nmodified   t/t3905-stash-include-untracked.sh\n@@ -434,7 +434,7 @@ test_expect_success 'stash push\n--no-overwrite-ignore preserves ignored files' '\n \tgit commit -m \"add ignore\" &&\n\n \techo after >ignored.txt &&\n-\tgit stash push --no-overwrite-ignore &&\n+\tgit stash push --overwrite-ignore &&\n\n \ttest_path_is_file ignored.txt &&\n \tgrep after ignored.txt\n\nstill passes the test. We should be testing both scenarios.\n\n>  test_done\n> --\n> 2.43.0\n"},{"id":"534976","messageId":"aYCyk02vG8ObH02j@pks.im","threadId":"64899","inReplyTo":"20260202131921.15175-2-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH] stash: honor --no-overwrite-ignore when updating index","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-02T14:20:03Z","receivedAt":"2026-02-02T14:20:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Feb 02, 2026 at 01:19:22PM +0000, Pushkar Singh wrote:\n> The stash code unconditionally cleared opts.preserve_ignored when\n> updating the index, leaving a FIXME suggesting this should depend on\n> an overwrite_ignore flag.\n> \n> Introduce overwrite_ignore plumbing for git stash push/save and use it\n> to control preserve_ignored during reset_tree(). Add a test to verify\n> that --no-overwrite-ignore preserves ignored files.\n\nIt's somewhat surprising that this requires so little code changes. Do\nthe mailing list archives yield any justification for why specifically\nthis feature wasn't implemented?\n\n> This removes the long-standing FIXME and aligns stash behavior with\n> checkout/reset/merge.\n\nMissing signoff.\n\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 193e3ea47a..82d10520fe 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -1856,6 +1857,10 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>  \t\t\t N_(\"include untracked files in stash\")),\n>  \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n>  \t\t\t    N_(\"include ignore files\"), 2),\n> +\t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> +\t\t\tN_(\"update ignored files (default)\")),\n> +\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> +\t\t\tN_(\"do not update ignored files\")),\n>  \t\tOPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n>  \t\t\t   N_(\"stash message\")),\n>  \t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n\n`OPT_BOOL()` already handles both the positive and negative case, so\nthere's no need to specify both here. Furthermore, both of your options\nactually do the exact same thing.\n\n> @@ -1959,6 +1964,10 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n>  \t\t\t N_(\"include untracked files in stash\")),\n>  \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n>  \t\t\t    N_(\"include ignore files\"), 2),\n> +\t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> +\t\t\t\tN_(\"update ignored files (default)\")),\n> +\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> +\t\t\t\tN_(\"do not update ignored files\")),\n>  \t\tOPT_STRING('m', \"message\", &stash_msg, \"message\",\n>  \t\t\t   N_(\"stash message\")),\n>  \t\tOPT_END()\n\nSame here.\n\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 7704709054..9c5421cd76 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -427,4 +427,17 @@ test_expect_success 'stash -u ignores sub-repository' '\n>  \tgit stash -u\n>  '\n>  \n> +test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n> +\techo ignored.txt >>.gitignore &&\n\nIs there any specific reason why we append instead of overwriting the\ngitignore file? Overwriting would probably be preferred so that it's\neasier to reason about the test without requiring context around what\nthe current contents of this file are.\n\n> +\techo before >ignored.txt &&\n> +\tgit add .gitignore &&\n> +\tgit commit -m \"add ignore\" &&\n> +\n> +\techo after >ignored.txt &&\n> +\tgit stash push --no-overwrite-ignore &&\n> +\n> +\ttest_path_is_file ignored.txt &&\n> +\tgrep after ignored.txt\n\nI think another good step would be to verify that `git stash push\n--overwrite-ignore` _would_ cause us to overwrite the file.\n\nI guess this test only happens to work because the first option\ntakes precedence over the ambiguous second one?\n\nPatrick\n"},{"id":"534977","messageId":"1abb1fa0-3548-4258-95d9-0505ea446043@app.fastmail.com","threadId":"64899","inReplyTo":"20260202131921.15175-2-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH] stash: honor --no-overwrite-ignore when updating index","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-02-02T14:21:01Z","receivedAt":"2026-02-02T14:21:22Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Feb 2, 2026, at 14:19, Pushkar Singh wrote:\n> The stash code unconditionally cleared opts.preserve_ignored when\n> updating the index, leaving a FIXME suggesting this should depend on\n> an overwrite_ignore flag.\n\nThe commit message should discuss what the code does without the patch\nin the present tense (SubmittingPathces, “present-tense”).\n\n>\n> Introduce overwrite_ignore plumbing for git stash push/save and use it\n> to control preserve_ignored during reset_tree(). Add a test to verify\n> that --no-overwrite-ignore preserves ignored files.\n>\n> This removes the long-standing FIXME and aligns stash behavior with\n> checkout/reset/merge.\n\nMissing signoff.\n\n> ---\n>  builtin/stash.c                    | 11 ++++++++++-\n>  t/t3905-stash-include-untracked.sh | 13 +++++++++++++\n>  2 files changed, 23 insertions(+), 1 deletion(-)\n>[snip]\n"},{"id":"534982","messageId":"20260202162225.35206-3-pushkarkumarsingh1970@gmail.com","threadId":"64899","inReplyTo":"20260202131921.15175-2-pushkarkumarsingh1970@gmail.com","subject":"[PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-02T16:22:27Z","receivedAt":"2026-02-02T16:32:07Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore\nis given by downgrading INCLUDE_ALL_FILES to include-untracked.\n\nThis fixes ignored files being incorrectly removed despite\n--no-overwrite-ignore, and removes the stash FIXME by plumbing\noverwrite_ignore into unpack_trees().\n\nAdd regression tests covering both overwrite and no-overwrite cases.\n\nChanges since v1:\n- Use OPT_BOOL correctly for overwrite-ignore.\n- Fix stash -a cleanup when --no-overwrite-ignore is given by downgrading\n  INCLUDE_ALL_FILES to include-untracked.\n- Add regression test for --overwrite-ignore.\n- Adjust no-overwrite-ignore test to explicitly use -a.\n- Add Signed-off-by.\n\nSigned-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n---\n builtin/stash.c                    | 14 ++++++++------\n t/t3905-stash-include-untracked.sh | 16 ++++++++++++++--\n 2 files changed, 22 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 82d10520fe..c3ee33cce1 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1858,9 +1858,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n \t\t\t    N_(\"include ignore files\"), 2),\n \t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n-\t\t\tN_(\"update ignored files (default)\")),\n-\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n-\t\t\tN_(\"do not update ignored files\")),\n+\t\t\t N_(\"update ignored files\")),\n \t\tOPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n \t\t\t   N_(\"stash message\")),\n \t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n@@ -1894,6 +1892,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \tparse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,\n \t\t       prefix, argv);\n \n+\tif (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n+\t\tinclude_untracked = 1;\n+\n \tif (pathspec_from_file) {\n \t\tif (patch_mode)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--pathspec-from-file\", \"--patch\");\n@@ -1965,9 +1966,7 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n \t\t\t    N_(\"include ignore files\"), 2),\n \t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n-\t\t\t\tN_(\"update ignored files (default)\")),\n-\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n-\t\t\t\tN_(\"do not update ignored files\")),\n+\t\t\t N_(\"update ignored files\")),\n \t\tOPT_STRING('m', \"message\", &stash_msg, \"message\",\n \t\t\t   N_(\"stash message\")),\n \t\tOPT_END()\n@@ -1994,6 +1993,9 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n \t\t\tdie(_(\"the option '%s' requires '%s'\"), \"--inter-hunk-context\", \"--patch\");\n \t}\n \n+\tif (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n+\t\tinclude_untracked = 1;\n+\n \tret = do_push_stash(&ps, stash_msg, quiet, keep_index,\n \t\t\t    patch_mode, &add_p_opt, include_untracked,\n \t\t\t    only_staged);\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 9c5421cd76..a979831a64 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -427,17 +427,29 @@ test_expect_success 'stash -u ignores sub-repository' '\n \tgit stash -u\n '\n \n-test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n+test_expect_success 'stash push -a --no-overwrite-ignore preserves ignored files' '\n \techo ignored.txt >>.gitignore &&\n \techo before >ignored.txt &&\n \tgit add .gitignore &&\n \tgit commit -m \"add ignore\" &&\n \n \techo after >ignored.txt &&\n-\tgit stash push --no-overwrite-ignore &&\n+\tgit stash push -a --no-overwrite-ignore &&\n \n \ttest_path_is_file ignored.txt &&\n \tgrep after ignored.txt\n '\n \n+test_expect_success 'stash push -a --overwrite-ignore overwrites ignored files' '\n+\techo ignored.txt >>.gitignore &&\n+\techo before >ignored.txt &&\n+\tgit add .gitignore &&\n+\tgit commit -m \"add ignore\" &&\n+\n+\techo after >ignored.txt &&\n+\tgit stash push -a --overwrite-ignore &&\n+\n+\t! grep after ignored.txt\n+'\n+\n test_done\n-- \n2.43.0\n\n"},{"id":"534984","messageId":"fd0da056-effa-43c8-a387-1db02b5636c8@app.fastmail.com","threadId":"64899","inReplyTo":"20260202162225.35206-3-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-02-02T16:48:07Z","receivedAt":"2026-02-02T16:48:29Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Feb 2, 2026, at 17:22, Pushkar Singh wrote:\n> Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore\n> is given by downgrading INCLUDE_ALL_FILES to include-untracked.\n>\n> This fixes ignored files being incorrectly removed despite\n> --no-overwrite-ignore, and removes the stash FIXME by plumbing\n> overwrite_ignore into unpack_trees().\n>\n> Add regression tests covering both overwrite and no-overwrite cases.\n>\n> Changes since v1:\n> - Use OPT_BOOL correctly for overwrite-ignore.\n> - Fix stash -a cleanup when --no-overwrite-ignore is given by downgrading\n>   INCLUDE_ALL_FILES to include-untracked.\n> - Add regression test for --overwrite-ignore.\n> - Adjust no-overwrite-ignore test to explicitly use -a.\n> - Add Signed-off-by.\n\nThese patch version changes are supposed to go after the `---` (after\nthe `Signed-off-by`). I guess people who are comfortable editing patches\nwrite them manually in that place (unless something like b4 or\ngigitgadget does it for them). I prefer to use `--notes` and let\ngit-format-patch(1) inject it for me. :)\n\n>\n> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n> ---\n>  builtin/stash.c                    | 14 ++++++++------\n>  t/t3905-stash-include-untracked.sh | 16 ++++++++++++++--\n>  2 files changed, 22 insertions(+), 8 deletions(-)\n>[snip]\n"},{"id":"534986","messageId":"20260202170933.37155-1-pushkarkumarsingh1970@gmail.com","threadId":"64899","inReplyTo":"fd0da056-effa-43c8-a387-1db02b5636c8@app.fastmail.com","subject":"Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-02T17:09:33Z","receivedAt":"2026-02-02T17:09:40Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Thanks for pointing that out, understood.\n\nI'll move the \"Changes since v1\" section below the `---` in the next revision.\n\nThanks for the tip.\n\n"},{"id":"535006","messageId":"CALnO6CDXwbxiQ-UjJLxgrjbgryQwxMro106BnewfFvcqchb2sw@mail.gmail.com","threadId":"64899","inReplyTo":"CAOLa=ZQCuka+cSuCu=KnTHm=gk1iJ_QJhDjy1Ku8WLfSgkGorw@mail.gmail.com","subject":"Re: [PATCH] stash: honor --no-overwrite-ignore when updating index","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-02-02T19:37:24Z","receivedAt":"2026-02-02T19:37:35Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Feb 2, 2026 at 9:13 AM Karthik Nayak <karthik.188@gmail.com> wrote:\n> To confirm, changing the test\n>\n> modified   t/t3905-stash-include-untracked.sh\n> @@ -434,7 +434,7 @@ test_expect_success 'stash push\n> --no-overwrite-ignore preserves ignored files' '\n>         git commit -m \"add ignore\" &&\n>\n>         echo after >ignored.txt &&\n> -       git stash push --no-overwrite-ignore &&\n> +       git stash push --overwrite-ignore &&\n>\n>         test_path_is_file ignored.txt &&\n>         grep after ignored.txt\n>\n> still passes the test. We should be testing both scenarios.\n\nHm. Using \"git stash push\" (no new flag) on my build of 2.53.0 with\nonly this test added passes, so I agree it seems unlikely to be\nexercising the intent that led to the FIXME.\n\n-- \nD. Ben Knoble\n"},{"id":"535008","messageId":"CALnO6CDQiSo7QYnjUmwxgRJJ1=A15JZ5TTWaHKUMgfiMoJHsww@mail.gmail.com","threadId":"64899","inReplyTo":"20260202162225.35206-3-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-02-02T20:00:57Z","receivedAt":"2026-02-02T20:01:09Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Feb 2, 2026 at 11:37 AM Pushkar Singh\n<pushkarkumarsingh1970@gmail.com> wrote:\n>\n> Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore\n> is given by downgrading INCLUDE_ALL_FILES to include-untracked.\n>\n> This fixes ignored files being incorrectly removed despite\n> --no-overwrite-ignore, and removes the stash FIXME by plumbing\n> overwrite_ignore into unpack_trees().\n>\n> Add regression tests covering both overwrite and no-overwrite cases.\n>\n> Changes since v1:\n> - Use OPT_BOOL correctly for overwrite-ignore.\n> - Fix stash -a cleanup when --no-overwrite-ignore is given by downgrading\n>   INCLUDE_ALL_FILES to include-untracked.\n> - Add regression test for --overwrite-ignore.\n> - Adjust no-overwrite-ignore test to explicitly use -a.\n> - Add Signed-off-by.\n>\n> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n> ---\n>  builtin/stash.c                    | 14 ++++++++------\n>  t/t3905-stash-include-untracked.sh | 16 ++++++++++++++--\n>  2 files changed, 22 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 82d10520fe..c3ee33cce1 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -1858,9 +1858,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>                 OPT_SET_INT('a', \"all\", &include_untracked,\n>                             N_(\"include ignore files\"), 2),\n>                 OPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> -                       N_(\"update ignored files (default)\")),\n> -               OPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> -                       N_(\"do not update ignored files\")),\n> +                        N_(\"update ignored files\")),\n>                 OPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n>                            N_(\"stash message\")),\n>                 OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n\nThis doesn't apply on top of, say, the master branch; it looks like\nyou generated this patch on top of the previous version?\n\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 9c5421cd76..a979831a64 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -427,17 +427,29 @@ test_expect_success 'stash -u ignores sub-repository' '\n>         git stash -u\n>  '\n>\n> -test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n> +test_expect_success 'stash push -a --no-overwrite-ignore preserves ignored files' '\n>         echo ignored.txt >>.gitignore &&\n>         echo before >ignored.txt &&\n>         git add .gitignore &&\n>         git commit -m \"add ignore\" &&\n>\n>         echo after >ignored.txt &&\n> -       git stash push --no-overwrite-ignore &&\n> +       git stash push -a --no-overwrite-ignore &&\n>\n>         test_path_is_file ignored.txt &&\n>         grep after ignored.txt\n>  '\n>\n> +test_expect_success 'stash push -a --overwrite-ignore overwrites ignored files' '\n> +       echo ignored.txt >>.gitignore &&\n> +       echo before >ignored.txt &&\n> +       git add .gitignore &&\n> +       git commit -m \"add ignore\" &&\n> +\n> +       echo after >ignored.txt &&\n> +       git stash push -a --overwrite-ignore &&\n> +\n> +       ! grep after ignored.txt\n> +'\n\nAfter removing --overwrite-ignore from these 2 tests to run them on\nunmodified Git, the first one fails (good: exercising new feature and\nshowing improvement) and the 2nd one succeeds (probably good:\nexercising existing behavior, but strange: see below).\n\nUse test_grep instead of plain grep. For example, it reveals that in\nthe first test, ignored.txt doesn't exist in current Git! (Which is\nexpected with -a, although should be changed by this series).\n\nFor the second test, I think we're hitting a similar issue (test_grep\ncomplains ignored.txt doesn't exist, so while inverted grep would\nsucceed, we actually want to see that the file doesn't exist, right?).\nAnyway, the current \"! grep …\" passes on current Git because it's not\nreally the right test, plus \"--overwrite-ignore\" is the current\nbehavior of \"-a\", so a modified version of this test _should_ pass on\ncurrent Git. I think we want \"test_path_is_missing\" here?\n\n-- \nD. Ben Knoble\n"},{"id":"535010","messageId":"CABPp-BEZkhYW+fWgtGn8yHuLfak+UYo9A_HwdiCkAf5A0H6hBA@mail.gmail.com","threadId":"64899","inReplyTo":"20260202162225.35206-3-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-02-02T20:31:07Z","receivedAt":"2026-02-02T20:31:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Feb 2, 2026 at 8:37 AM Pushkar Singh\n<pushkarkumarsingh1970@gmail.com> wrote:\n>\n> Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore\n> is given by downgrading INCLUDE_ALL_FILES to include-untracked.\n>\n> This fixes ignored files being incorrectly removed despite\n> --no-overwrite-ignore, and removes the stash FIXME by plumbing\n> overwrite_ignore into unpack_trees().\n>\n> Add regression tests covering both overwrite and no-overwrite cases.\n>\n> Changes since v1:\n> - Use OPT_BOOL correctly for overwrite-ignore.\n> - Fix stash -a cleanup when --no-overwrite-ignore is given by downgrading\n>   INCLUDE_ALL_FILES to include-untracked.\n> - Add regression test for --overwrite-ignore.\n> - Adjust no-overwrite-ignore test to explicitly use -a.\n> - Add Signed-off-by.\n>\n> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n> ---\n>  builtin/stash.c                    | 14 ++++++++------\n>  t/t3905-stash-include-untracked.sh | 16 ++++++++++++++--\n>  2 files changed, 22 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 82d10520fe..c3ee33cce1 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -1858,9 +1858,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>                 OPT_SET_INT('a', \"all\", &include_untracked,\n>                             N_(\"include ignore files\"), 2),\n>                 OPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> -                       N_(\"update ignored files (default)\")),\n> -               OPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> -                       N_(\"do not update ignored files\")),\n\nWhat's the basis for this patch?  I don't see any \"overwrite-ignore\"\nanywhere in builtin/stash.c .\n\n> +                        N_(\"update ignored files\")),\n>                 OPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n>                            N_(\"stash message\")),\n>                 OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n> @@ -1894,6 +1892,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>         parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,\n>                        prefix, argv);\n>\n> +       if (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n> +               include_untracked = 1;\n\nThis suggests that --all and --no-overwrite-ignore are incompatible,\nyes?  Shouldn't they be reported as such rather than having one\nsilently override the other?\n\n> +\n>         if (pathspec_from_file) {\n>                 if (patch_mode)\n>                         die(_(\"options '%s' and '%s' cannot be used together\"), \"--pathspec-from-file\", \"--patch\");\n> @@ -1965,9 +1966,7 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n>                 OPT_SET_INT('a', \"all\", &include_untracked,\n>                             N_(\"include ignore files\"), 2),\n>                 OPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> -                               N_(\"update ignored files (default)\")),\n> -               OPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> -                               N_(\"do not update ignored files\")),\n> +                        N_(\"update ignored files\")),\n>                 OPT_STRING('m', \"message\", &stash_msg, \"message\",\n>                            N_(\"stash message\")),\n>                 OPT_END()\n> @@ -1994,6 +1993,9 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n>                         die(_(\"the option '%s' requires '%s'\"), \"--inter-hunk-context\", \"--patch\");\n>         }\n>\n> +       if (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n> +               include_untracked = 1;\n> +\n\nSame comments as above.\n\nAlso, the commit message claims you are removing a FIXME comment, but\nno such removal is found in this patch.  Is this simply a patch\nagainst v1?  If so, don't do that; please send a corrected patch\n\nI took a look at v1 as well, and I'll note here that if that FIXME was\nthe only line that needed fixing, I would have just fixed it at the\ntime.  I left the FIXME there because I knew it was *one* of the\nplaces that would need fixing and didn't have the time or energy\n(already being a few levels deep in the rabbit hole) to track down all\nthe stash related issues in this area.  Perhaps the other sites have\nsince been fixed by someone else, but if so, that should really be\ndocumented in the commit message.  I'd personally be pretty surprised\nif the other locations have been fixed; see\nhttps://lore.kernel.org/git/CABPp-BFyR19ch71W10oJDFuRX1OHzQ3si971pMn6dPtHKxJDXQ@mail.gmail.com/\nand perhaps the references to stash in\nhttps://lore.kernel.org/git/pull.1627.git.1703643931314.gitgitgadget@gmail.com/\n; there may also be other issues within stash, those were just the\nones I was aware of that looked fishy at the time.\n\n>         ret = do_push_stash(&ps, stash_msg, quiet, keep_index,\n>                             patch_mode, &add_p_opt, include_untracked,\n>                             only_staged);\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 9c5421cd76..a979831a64 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -427,17 +427,29 @@ test_expect_success 'stash -u ignores sub-repository' '\n>         git stash -u\n>  '\n>\n> -test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n> +test_expect_success 'stash push -a --no-overwrite-ignore preserves ignored files' '\n>         echo ignored.txt >>.gitignore &&\n>         echo before >ignored.txt &&\n>         git add .gitignore &&\n>         git commit -m \"add ignore\" &&\n>\n>         echo after >ignored.txt &&\n> -       git stash push --no-overwrite-ignore &&\n> +       git stash push -a --no-overwrite-ignore &&\n\nIsn't this a non-sensical combination of command line options?\n--no-overwrite-ignore is explicitly setting include_untracked to 1\nwhile --all sets include_untracked to 2, and I believe those are the\n_only_ things each of those flags do, which means these ought to be\nincompatible flags.\n\n>\n>         test_path_is_file ignored.txt &&\n>         grep after ignored.txt\n>  '\n>\n> +test_expect_success 'stash push -a --overwrite-ignore overwrites ignored files' '\n> +       echo ignored.txt >>.gitignore &&\n> +       echo before >ignored.txt &&\n> +       git add .gitignore &&\n> +       git commit -m \"add ignore\" &&\n> +\n> +       echo after >ignored.txt &&\n> +       git stash push -a --overwrite-ignore &&\n> +\n> +       ! grep after ignored.txt\n> +'\n> +\n>  test_done\n> --\n> 2.43.0\n"},{"id":"535081","messageId":"20260203180359.602905-2-pushkarkumarsingh1970@gmail.com","threadId":"64899","inReplyTo":"20260202162225.35206-3-pushkarkumarsingh1970@gmail.com","subject":"[PATCH v3] stash: honor --no-overwrite-ignore with --all","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-03T18:04:00Z","receivedAt":"2026-02-03T18:07:10Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore\nis given by downgrading INCLUDE_ALL_FILES to include-untracked.\n\nThis fixes ignored files being incorrectly removed despite\n--no-overwrite-ignore.\n\nAdd regression tests covering both overwrite and no-overwrite cases.\n\nSigned-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n---\nChanges since v2:\n- Use test_grep instead of grep\n- Use test_path_is_missing for overwrite-ignore test\n- Rebase onto current master so patch applies cleanly\n\n builtin/stash.c                    | 14 ++++++++------\n t/t3905-stash-include-untracked.sh | 18 +++++++++++++++---\n 2 files changed, 23 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 82d10520fe..c3ee33cce1 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1858,9 +1858,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n \t\t\t    N_(\"include ignore files\"), 2),\n \t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n-\t\t\tN_(\"update ignored files (default)\")),\n-\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n-\t\t\tN_(\"do not update ignored files\")),\n+\t\t\t N_(\"update ignored files\")),\n \t\tOPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n \t\t\t   N_(\"stash message\")),\n \t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n@@ -1894,6 +1892,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \tparse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,\n \t\t       prefix, argv);\n \n+\tif (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n+\t\tinclude_untracked = 1;\n+\n \tif (pathspec_from_file) {\n \t\tif (patch_mode)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--pathspec-from-file\", \"--patch\");\n@@ -1965,9 +1966,7 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n \t\tOPT_SET_INT('a', \"all\", &include_untracked,\n \t\t\t    N_(\"include ignore files\"), 2),\n \t\tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n-\t\t\t\tN_(\"update ignored files (default)\")),\n-\t\tOPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n-\t\t\t\tN_(\"do not update ignored files\")),\n+\t\t\t N_(\"update ignored files\")),\n \t\tOPT_STRING('m', \"message\", &stash_msg, \"message\",\n \t\t\t   N_(\"stash message\")),\n \t\tOPT_END()\n@@ -1994,6 +1993,9 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n \t\t\tdie(_(\"the option '%s' requires '%s'\"), \"--inter-hunk-context\", \"--patch\");\n \t}\n \n+\tif (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n+\t\tinclude_untracked = 1;\n+\n \tret = do_push_stash(&ps, stash_msg, quiet, keep_index,\n \t\t\t    patch_mode, &add_p_opt, include_untracked,\n \t\t\t    only_staged);\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 9c5421cd76..63b59de47b 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -427,17 +427,29 @@ test_expect_success 'stash -u ignores sub-repository' '\n \tgit stash -u\n '\n \n-test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n+test_expect_success 'stash push -a --no-overwrite-ignore preserves ignored files' '\n \techo ignored.txt >>.gitignore &&\n \techo before >ignored.txt &&\n \tgit add .gitignore &&\n \tgit commit -m \"add ignore\" &&\n \n \techo after >ignored.txt &&\n-\tgit stash push --no-overwrite-ignore &&\n+\tgit stash push -a --no-overwrite-ignore &&\n \n \ttest_path_is_file ignored.txt &&\n-\tgrep after ignored.txt\n+\ttest_grep after ignored.txt\n+'\n+\n+test_expect_success 'stash push -a --overwrite-ignore overwrites ignored files' '\n+\techo ignored.txt >>.gitignore &&\n+\techo before >ignored.txt &&\n+\tgit add .gitignore &&\n+\tgit commit -m \"add ignore\" &&\n+\n+\techo after >ignored.txt &&\n+\tgit stash push -a --overwrite-ignore &&\n+\n+\ttest_path_is_missing ignored.txt\n '\n \n test_done\n-- \n2.43.0\n\n"},{"id":"535083","messageId":"20260203181845.602979-1-pushkarkumarsingh1970@gmail.com","threadId":"64899","inReplyTo":"CABPp-BEZkhYW+fWgtGn8yHuLfak+UYo9A_HwdiCkAf5A0H6hBA@mail.gmail.com","subject":"Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-03T18:18:45Z","receivedAt":"2026-02-03T18:18:54Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Elijah,\n\nThanks for the detailed feedback. Much appreciated.\n\n> What's the basis for this patch? I don't see any \"overwrite-ignore\"\n> anywhere in builtin/stash.c .\n\nThe basis was the existing behavior where git stash push -a would remove\nignored files even when --no-overwrite-ignore is provided. The intent was\nto make stash honor --no-overwrite-ignore consistently with other callers\nof unpack_trees, limited specifically to the stash -a cleanup path.\n\nIn v3 I rebased onto current master and also removed the commit message\nclaim about removing the stash FIXME, since this series only addresses\nthe concrete stash behavior and does not attempt to solve the broader\nunpack_trees issues.\n\n> This suggests that --all and --no-overwrite-ignore are incompatible,\n> yes? Shouldn't they be reported as such rather than having one silently\n> override the other?\n\nI agree they are philosophically contradictory. I chose to downgrade\nINCLUDE_ALL_FILES to include-untracked when --no-overwrite-ignore is given\nso that users explicitly requesting preservation of ignored files are not\nsurprised by their removal.\n\nI am open to changing this to an explicit error instead if that is\npreferred. I went with downgrading to preserve backwards compatibility\nand to honor the more conservative option.\n\n> Also, the commit message claims you are removing a FIXME comment, but\n> no such removal is found in this patch.\n\nYes, that was an error in v2. In v3 the commit message no longer claims\nto remove the FIXME and is scoped only to fixing stash -a behavior plus\nadding regression tests.\n\nRegarding tests, v3 now explicitly covers both:\n\n- stash push -a --no-overwrite-ignore preserving ignored files\n- stash push -a --overwrite-ignore removing them\n\nusing test_grep and test_path_is_missing as suggested.\n\nThanks also for the references to the broader unpack_trees and stash\nhistory. I understand this patch only addresses a small part of a much\nlarger and messier area.\n\nPlease let me know if you would prefer the -a plus --no-overwrite-ignore\ncombination to error out instead of downgrading.\n\nThanks,\nPushkar\n"},{"id":"535086","messageId":"CABPp-BEP=KBKdF-hgMgF0ngJDsBHMehCJoRc=ww=-W=F3s5rcQ@mail.gmail.com","threadId":"64899","inReplyTo":"20260203181845.602979-1-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-02-03T19:22:47Z","receivedAt":"2026-02-03T19:23:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Pushkar,\n\nOn Tue, Feb 3, 2026 at 10:18 AM Pushkar Singh\n<pushkarkumarsingh1970@gmail.com> wrote:\n>\n> Hi Elijah,\n>\n> Thanks for the detailed feedback. Much appreciated.\n>\n> > What's the basis for this patch? I don't see any \"overwrite-ignore\"\n> > anywhere in builtin/stash.c .\n>\n> The basis was the existing behavior where git stash push -a would remove\n> ignored files even when --no-overwrite-ignore is provided.\n\nBy basis, I meant what commit was it based on.  The patch(es) you send\nneed to be applied by others, and if they are based on commits only\nyou have locally, others can't apply or try them and have to guess the\ndetails of all your intermediate patches.  v2 should be what you would\nhave sent to the list if you had gotten everything right the first\ntime.  Same with v3, v4, etc.\n\nYou appear to have thought in terms of the purpose of the patch, but\nyour stated purpose doesn't make sense either.  There is no\n--no-overwrite-ignore option, so complaining about how the command\nbehaved when that non-existent option is given doesn't help me\nunderstand the purpose of the new option.\n\n> The intent was\n> to make stash honor --no-overwrite-ignore consistently with other callers\n> of unpack_trees, limited specifically to the stash -a cleanup path.\n\nOh, so the point of the patch is an attempt to make command line\noptions more consistent along some axis?  If so, I think you picked a\nplace where it doesn't actually make sense.  We need to back up and\nfigure out what the user-side desired behavior is and what they cannot\nachieve today, or what is confusing today, and find ways to improve\nthat and implement it.  Starting from the low-level details can work,\nbut only if at the end we can explain to users why our changes make\nsense.\n\n> In v3 I rebased onto current master and also removed the commit message\n> claim about removing the stash FIXME, since this series only addresses\n> the concrete stash behavior and does not attempt to solve the broader\n> unpack_trees issues.\n>\n> > This suggests that --all and --no-overwrite-ignore are incompatible,\n> > yes? Shouldn't they be reported as such rather than having one silently\n> > override the other?\n>\n> I agree they are philosophically contradictory. I chose to downgrade\n> INCLUDE_ALL_FILES to include-untracked when --no-overwrite-ignore is given\n> so that users explicitly requesting preservation of ignored files are not\n> surprised by their removal.\n\nYou don't want people who pass \"--no-overwrite-ignore\" (a new option)\nto be surprised when ignores are overwritten, but don't care about\npeople who pass \"-a\" (\"--all\") getting surprised that all files aren't\nincluded in the stash?  I don't quite understand the logic.\n\n> I am open to changing this to an explicit error instead if that is\n> preferred. I went with downgrading to preserve backwards compatibility\n> and to honor the more conservative option.\n\nYou added a new option and only changed behavior relative to when that\nnew option is invoked, so I don't understand the claim about\npreserving backwards compatibility; how can backwards compatibility\neven be relevant in this situation?\n\nI'm not sure I understand the \"honor the more conservative option\"\neither.  Is that a cyclical argument (you're introducing a new option\nand deciding to honor it in order to honor it), or am I\nmisunderstanding?\n"},{"id":"535087","messageId":"CABPp-BG6wM4p0wAizEppT7QdtY710xBJ8NwgfzrDpP3Oyg=a0w@mail.gmail.com","threadId":"64899","inReplyTo":"20260203180359.602905-2-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v3] stash: honor --no-overwrite-ignore with --all","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-02-03T19:22:57Z","receivedAt":"2026-02-03T19:23:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Feb 3, 2026 at 10:09 AM Pushkar Singh\n<pushkarkumarsingh1970@gmail.com> wrote:\n>\n> Teach stash push/save to avoid -a cleanup when --no-overwrite-ignore\n> is given by downgrading INCLUDE_ALL_FILES to include-untracked.\n\nThis feels like you're regurgitating the patch with low-enough level\nof details (\"-a cleanup\", INCLUDE_ALL_FILES, include-untracked) that\nit'll only be intelligible to someone who has builtin/stash.c code\nfresh on their mind.  It doesn't explain the high-level purpose behind\nyour patch, and, in fact, will likely lead readers to try to read the\npatch in order to understand the commit message, when usually we hope\nfor the opposite.\n\n> This fixes ignored files being incorrectly removed despite\n> --no-overwrite-ignore.\n\nThis claim makes no sense; --no-overwrite-ignore doesn't exist in git\nyet, and this is the first (and only) patch in your series, so at best\nyou're claiming to fix something you introduced?  Very confusing.\n\n> Add regression tests covering both overwrite and no-overwrite cases.\n>\n> Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n> ---\n> Changes since v2:\n> - Use test_grep instead of grep\n> - Use test_path_is_missing for overwrite-ignore test\n> - Rebase onto current master so patch applies cleanly\n\nGreat...but you still seem to be submitting a patch that is based on\nyour previous (rebased?) patches, without submitting the previous\npatches, leaving us to guess how you got to your current state, as\nnoted below.  You should have been editing your previous patch and\nthen submitting the edited patch.  After v2, you should have squashed\nand resent.\n\n>  builtin/stash.c                    | 14 ++++++++------\n>  t/t3905-stash-include-untracked.sh | 18 +++++++++++++++---\n>  2 files changed, 23 insertions(+), 9 deletions(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 82d10520fe..c3ee33cce1 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -1858,9 +1858,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>                 OPT_SET_INT('a', \"all\", &include_untracked,\n>                             N_(\"include ignore files\"), 2),\n>                 OPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> -                       N_(\"update ignored files (default)\")),\n> -               OPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> -                       N_(\"do not update ignored files\")),\n> +                        N_(\"update ignored files\")),\n\nAnd here's where it's clear that this patch was broken in the same way\nas v2: \"no-overwrite-ignore\" has never appeared in any version of\nbuiltin/stash.c upstream (same with \"overwrite-ignore\"), so this patch\nis clearly against some local state you have.  The base of your series\n(or the base of your patch, since you only have one patch in this\nseries) needs to be an upstream commit, not some other commit that\nonly you have access to.  Might I interest you in using gitgitgadget,\nwhich would make it easier to submit patches?\n\n>                 OPT_STRING('m', \"message\", &stash_msg, N_(\"message\"),\n>                            N_(\"stash message\")),\n>                 OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n> @@ -1894,6 +1892,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n>         parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,\n>                        prefix, argv);\n>\n> +       if (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n> +               include_untracked = 1;\n> +\n>         if (pathspec_from_file) {\n>                 if (patch_mode)\n>                         die(_(\"options '%s' and '%s' cannot be used together\"), \"--pathspec-from-file\", \"--patch\");\n> @@ -1965,9 +1966,7 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n>                 OPT_SET_INT('a', \"all\", &include_untracked,\n>                             N_(\"include ignore files\"), 2),\n>                 OPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore,\n> -                               N_(\"update ignored files (default)\")),\n> -               OPT_BOOL(0, \"no-overwrite-ignore\", &overwrite_ignore,\n> -                               N_(\"do not update ignored files\")),\n> +                        N_(\"update ignored files\")),\n>                 OPT_STRING('m', \"message\", &stash_msg, \"message\",\n>                            N_(\"stash message\")),\n>                 OPT_END()\n> @@ -1994,6 +1993,9 @@ static int save_stash(int argc, const char **argv, const char *prefix,\n>                         die(_(\"the option '%s' requires '%s'\"), \"--inter-hunk-context\", \"--patch\");\n>         }\n>\n> +       if (!overwrite_ignore && include_untracked == INCLUDE_ALL_FILES)\n> +               include_untracked = 1;\n> +\n>\n>         ret = do_push_stash(&ps, stash_msg, quiet, keep_index,\n>                             patch_mode, &add_p_opt, include_untracked,\n>                             only_staged);\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 9c5421cd76..63b59de47b 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -427,17 +427,29 @@ test_expect_success 'stash -u ignores sub-repository' '\n>         git stash -u\n>  '\n>\n> -test_expect_success 'stash push --no-overwrite-ignore preserves ignored files' '\n> +test_expect_success 'stash push -a --no-overwrite-ignore preserves ignored files' '\n>         echo ignored.txt >>.gitignore &&\n>         echo before >ignored.txt &&\n>         git add .gitignore &&\n>         git commit -m \"add ignore\" &&\n>\n>         echo after >ignored.txt &&\n> -       git stash push --no-overwrite-ignore &&\n> +       git stash push -a --no-overwrite-ignore &&\n\nNot only is the patch broken (\"no-overwrite-ignore\" has never appeared\nin any version of git; so this patch is clearly against your local\nstate), but the command line makes no sense:\n  -a : stash ignored files too\n  --no-overwrite-ignore: wait, we don't want to mess with ignored\nfiles, so nevermind, don't stash them\n\nWhy wouldn't the user just leave off \"-a\" if they don't want them stashed?\n\n>         test_path_is_file ignored.txt &&\n> -       grep after ignored.txt\n> +       test_grep after ignored.txt\n> +'\n> +\n> +test_expect_success 'stash push -a --overwrite-ignore overwrites ignored files' '\n> +       echo ignored.txt >>.gitignore &&\n> +       echo before >ignored.txt &&\n> +       git add .gitignore &&\n> +       git commit -m \"add ignore\" &&\n> +\n> +       echo after >ignored.txt &&\n> +       git stash push -a --overwrite-ignore &&\n\nAnd this command line makes no sense either:\n  -a: stash ignored files too\n  --overwrite-ignore: yes, I'm explicitly giving you permission to pay\nattention to the fact that I already passed you the \"-a\" parameter.\nPlease do what that other parameter says.\n\nWhy would the user need an extra flag instead of just using \"-a\"?\n\nAdditionally, if there is some user problem you're trying to solve\nhere, then these tests look rather incomplete; they only test the push\nside and not the pop side.  What if someone runs \"git stash push -a\"\nfollowed by \"git stash pop --no-overwrite-ignore\"?  Or is that flag\nnot going to be added to pop?  Do we only care about protecting\nignored files at push/save time and not at pop time?  Why?  (And if we\ndo care about pop time, won't we need to worry about both former\nuntracked and former ignored files both having the possibility of\noverwriting files that are now ignored?  And if we do allow users to\nnot overwrite ignored files at pop time, do we have a similar special\nflag to avoid overwriting untracked files at pop time?  If not, are\nignored files thus more important or special than untracked files?)\n\nI don't understand the user-driven problem this patch is attempting to solve.\n"},{"id":"535091","messageId":"CALE2CrSwN7AB05Qd7G7LOGjSNu3=BbLLVBfoNf5a95SMRBm5WQ@mail.gmail.com","threadId":"64899","inReplyTo":"CABPp-BG6wM4p0wAizEppT7QdtY710xBJ8NwgfzrDpP3Oyg=a0w@mail.gmail.com","subject":"Re: [PATCH v3] stash: honor --no-overwrite-ignore with --all","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-03T20:07:15Z","receivedAt":"2026-02-03T20:07:27Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Elijah,\n\nThank you for taking the time to explain this so clearly.\n\nYou are absolutely right. I misunderstood what you meant by “basis”\nand also approached this patch from the implementation side instead of\nstarting from a concrete user problem.\n\nI also realize now that I incorrectly stacked this on top of my local\nchanges instead of rebasing and editing the previous version, which\nmade the patch impossible to apply upstream. Sorry about that.\n\nGiven your feedback, I agree that I need to step back and rethink this\nfrom a user perspective (what real workflow is broken today, how -a\nshould behave, and whether any new flags even make sense here), rather\nthan trying to force consistency at a low level.\n\nI will drop this series for now, spend time understanding stash\nbehavior and the broader context you pointed out, and only resend if I\ncan clearly articulate a user-driven problem with a clean patch based\ndirectly on upstream.\n\nThanks again for your patience and guidance.\n\nPushkar\n"}]}