{"thread":{"id":"51923","subject":"[PATCH 0/2] Fix git stash with skip-worktree entries","startedAt":"2019-09-26T07:42:34Z","lastAt":"2019-11-02T23:03:57Z","messageCount":23,"participants":["Johannes Schindelin via GitGitGadget","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"382954","messageId":"pull.355.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":null,"subject":"[PATCH 0/2] Fix git stash with skip-worktree entries","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T07:42:29Z","receivedAt":"2019-09-26T07:42:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"My colleague Dan Thompson reported a bug in a sparse checkout, where git\nstash (after resolving merge conflicts and then making up their mind to\nstash the changes instead of committing them) would \"lose\" files (and files\nthat were not even in the sparse checkout's cone!).\n\nI first considered changing the behavior of git diff-index to simply ignore\nskip-worktree entries. But after re-reading the documentation of the\nskip-worktree bit, I became convinced that this would be incorrect a fix\nbecause the command really does what it advertises to do.\n\nThen, I briefly considered introducing a flag that would change the behavior\nthusly, but ended up deciding against it.\n\nThe actual problem, after all, is the git update-index call and that it\nheeds the --remove (but not the --add) option for skip-worktree entries.\n\"Heeds\", I should say, because the idea of the skip-worktree bit really is\ndocumented to imply that the worktree files should be considered identical\nto their staged versions.\n\nSo arguably, it could be considered a bug that git update-index --remove \neven bothers to touch skip-worktree entries. But this behavior has been in\nplace for over 10 years, so I opted to introduce a new mode that does what \ngit stash needs in order to fix the bug.\n\nJohannes Schindelin (2):\n  update-index: optionally leave skip-worktree entries alone\n  stash: handle staged changes in skip-worktree files correctly\n\n Documentation/git-update-index.txt |  6 ++++++\n builtin/stash.c                    |  5 +++--\n builtin/update-index.c             |  6 +++++-\n git-legacy-stash.sh                |  3 ++-\n t/t3903-stash.sh                   | 11 +++++++++++\n 5 files changed, 27 insertions(+), 4 deletions(-)\n\n\nbase-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-355%2Fdscho%2Ffix-stash-with-skip-worktree-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-355/dscho/fix-stash-with-skip-worktree-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/355\n-- \ngitgitgadget\n"},{"id":"382955","messageId":"c263eb54b30b3e93abb74662d120d8bb882322bb.1569483748.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T07:42:31Z","receivedAt":"2019-09-26T07:42:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhile `git update-index` mostly ignores paths referring to index entries\nwhose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\nskip-worktree bit (reading part), 2009-08-20), for reasons that are not\nentirely obvious, the `--remove` option was made special: it _does_\nremove index entries even if their skip-worktree bit is set.\n\nSeeing as this behavior has been in place for a decade now, it does not\nmake sense to change it.\n\nHowever, in preparation for fixing a bug in `git stash` where it\npretends that skip-worktree entries have actually been removed, we need\na mode where `git update-index` leaves all skip-worktree entries alone,\neven if the `--remove` option was passed.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-update-index.txt | 6 ++++++\n builtin/update-index.c             | 6 +++++-\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex 1c4d146a41..08393445e7 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -16,6 +16,7 @@ SYNOPSIS\n \t     [--chmod=(+|-)x]\n \t     [--[no-]assume-unchanged]\n \t     [--[no-]skip-worktree]\n+\t     [--[no-]ignore-skip-worktree-entries]\n \t     [--[no-]fsmonitor-valid]\n \t     [--ignore-submodules]\n \t     [--[no-]split-index]\n@@ -113,6 +114,11 @@ you will need to handle the situation manually.\n \tset and unset the \"skip-worktree\" bit for the paths. See\n \tsection \"Skip-worktree bit\" below for more information.\n \n+\n+--[no-]ignore-skip-worktree-entries::\n+\tDo not remove skip-worktree (AKA \"index-only\") entries even when\n+\tthe `--remove` option was specified.\n+\n --[no-]fsmonitor-valid::\n \tWhen one of these flags is specified, the object name recorded\n \tfor the paths are not updated. Instead, these options\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex dff2f4b837..074d563df0 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -35,6 +35,7 @@ static int verbose;\n static int mark_valid_only;\n static int mark_skip_worktree_only;\n static int mark_fsmonitor_only;\n+static int ignore_skip_worktree_entries;\n #define MARK_FLAG 1\n #define UNMARK_FLAG 2\n static struct strbuf mtime_dir = STRBUF_INIT;\n@@ -381,7 +382,8 @@ static int process_path(const char *path, struct stat *st, int stat_errno)\n \t\t * so updating it does not make sense.\n \t\t * On the other hand, removing it from index should work\n \t\t */\n-\t\tif (allow_remove && remove_file_from_cache(path))\n+\t\tif (!ignore_skip_worktree_entries && allow_remove &&\n+\t\t    remove_file_from_cache(path))\n \t\t\treturn error(\"%s: cannot remove from the index\", path);\n \t\treturn 0;\n \t}\n@@ -1013,6 +1015,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t{OPTION_SET_INT, 0, \"no-skip-worktree\", &mark_skip_worktree_only, NULL,\n \t\t\tN_(\"clear skip-worktree bit\"),\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG},\n+\t\tOPT_BOOL(0, \"ignore-skip-worktree-entries\", &ignore_skip_worktree_entries,\n+\t\t\t N_(\"do not touch index-only entries\")),\n \t\tOPT_SET_INT(0, \"info-only\", &info_only,\n \t\t\tN_(\"add to index only; do not add content to object database\"), 1),\n \t\tOPT_SET_INT(0, \"force-remove\", &force_remove,\n-- \ngitgitgadget\n\n"},{"id":"382956","messageId":"4c684be1794cd0c62cc19d9c9558981584ec3e9a.1569483748.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] stash: handle staged changes in skip-worktree files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-26T07:42:32Z","receivedAt":"2019-09-26T07:42:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen calling `git stash` while changes were staged for files that are\nmarked with the `skip-worktree` bit (e.g. files that are excluded in a\nsparse checkout), the files are recorded as _deleted_ instead.\n\nThe reason is that `git stash` tries to construct the tree reflecting\nthe worktree essentially by copying the index to a temporary one and\nthen updating the files from the worktree. Crucially, it calls `git\ndiff-index` to update also those files that are in the HEAD but have\nbeen unstaged in the index.\n\nHowever, when the temporary index is updated via `git update-index --add\n--remove`, skip-worktree entries mark the files as deleted by mistake.\n\nLet's use the newly-introduced `--ignore-skip-worktree-entries` option\nof `git update-index` to prevent exactly this from happening.\n\nNote that the regression test case deliberately avoids replicating the\nscenario described above and instead tries to recreate just the symptom.\n\nReported by Dan Thompson.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/stash.c     |  5 +++--\n git-legacy-stash.sh |  3 ++-\n t/t3903-stash.sh    | 11 +++++++++++\n 3 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex b5a301f24d..56f3b551e4 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1082,8 +1082,9 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \t}\n \n \tcp_upd_index.git_cmd = 1;\n-\targv_array_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n-\t\t\t \"--remove\", \"--stdin\", NULL);\n+\targv_array_pushl(&cp_upd_index.args, \"update-index\",\n+\t\t\t \"--ignore-skip-worktree-entries\",\n+\t\t\t \"-z\", \"--add\", \"--remove\", \"--stdin\", NULL);\n \targv_array_pushf(&cp_upd_index.env_array, \"GIT_INDEX_FILE=%s\",\n \t\t\t stash_index_path.buf);\n \ndiff --git a/git-legacy-stash.sh b/git-legacy-stash.sh\nindex f60e9b3e87..5398a5161d 100755\n--- a/git-legacy-stash.sh\n+++ b/git-legacy-stash.sh\n@@ -193,7 +193,8 @@ create_stash () {\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n \t\t\tgit diff-index --name-only -z HEAD -- \"$@\" >\"$TMP-stagenames\" &&\n-\t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n+\t\t\tgit update-index --ignore-skip-worktree-entries \\\n+\t\t\t\t-z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n \t\t) ) ||\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex b8e337893f..1e977145b8 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1241,4 +1241,15 @@ test_expect_success 'stash --keep-index with file deleted in index does not resu\n \ttest_path_is_missing to-remove\n '\n \n+test_expect_success 'stash handles skip-worktree entries nicely' '\n+\ttest_commit A &&\n+\techo changed >A.t &&\n+\tgit add A.t &&\n+\tgit update-index --skip-worktree A.t &&\n+\trm A.t &&\n+\tgit stash &&\n+\n+\tgit rev-parse --verify refs/stash:A.t\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"384936","messageId":"nycvar.QRO.7.76.6.1910272203360.46@tvgsbejvaqbjf.bet","threadId":"51923","inReplyTo":"pull.355.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Fix git stash with skip-worktree entries","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-27T21:05:30Z","receivedAt":"2019-10-27T21:37:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 26 Sep 2019, Johannes Schindelin via GitGitGadget wrote:\n\n> My colleague Dan Thompson reported a bug in a sparse checkout, where git\n> stash (after resolving merge conflicts and then making up their mind to\n> stash the changes instead of committing them) would \"lose\" files (and files\n> that were not even in the sparse checkout's cone!).\n\nI only realized _now_ that this patch has not made it anywhere; I would\nlike to have it at least in Git for Windows v2.24.0.\n\nCould I ask for this to still be picked up into `pu` at least, so that\nit does not fall into oblivion?\n\nThanks,\nDscho\n\n>\n> I first considered changing the behavior of git diff-index to simply ignore\n> skip-worktree entries. But after re-reading the documentation of the\n> skip-worktree bit, I became convinced that this would be incorrect a fix\n> because the command really does what it advertises to do.\n>\n> Then, I briefly considered introducing a flag that would change the behavior\n> thusly, but ended up deciding against it.\n>\n> The actual problem, after all, is the git update-index call and that it\n> heeds the --remove (but not the --add) option for skip-worktree entries.\n> \"Heeds\", I should say, because the idea of the skip-worktree bit really is\n> documented to imply that the worktree files should be considered identical\n> to their staged versions.\n>\n> So arguably, it could be considered a bug that git update-index --remove\n> even bothers to touch skip-worktree entries. But this behavior has been in\n> place for over 10 years, so I opted to introduce a new mode that does what\n> git stash needs in order to fix the bug.\n>\n> Johannes Schindelin (2):\n>   update-index: optionally leave skip-worktree entries alone\n>   stash: handle staged changes in skip-worktree files correctly\n>\n>  Documentation/git-update-index.txt |  6 ++++++\n>  builtin/stash.c                    |  5 +++--\n>  builtin/update-index.c             |  6 +++++-\n>  git-legacy-stash.sh                |  3 ++-\n>  t/t3903-stash.sh                   | 11 +++++++++++\n>  5 files changed, 27 insertions(+), 4 deletions(-)\n>\n>\n> base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-355%2Fdscho%2Ffix-stash-with-skip-worktree-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-355/dscho/fix-stash-with-skip-worktree-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/355\n> --\n> gitgitgadget\n>\n>\n"},{"id":"384962","messageId":"xmqqpniha9a8.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"nycvar.QRO.7.76.6.1910272203360.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/2] Fix git stash with skip-worktree entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-28T02:33:35Z","receivedAt":"2019-10-28T02:33:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Junio,\n>\n> On Thu, 26 Sep 2019, Johannes Schindelin via GitGitGadget wrote:\n>\n>> My colleague Dan Thompson reported a bug in a sparse checkout, where git\n>> stash (after resolving merge conflicts and then making up their mind to\n>> stash the changes instead of committing them) would \"lose\" files (and files\n>> that were not even in the sparse checkout's cone!).\n>\n> I only realized _now_ that this patch has not made it anywhere.\n\nYeah, I do not recall seeing any of the patches in the topic (nor\nthe cover letter).  It is not clear to me what \"lose\" above means,\nwhich is an indication that I didn't read the topic a month ago X-<.\n\nDid it even get any review by skip worktree bit experts back then?\n"},{"id":"384966","messageId":"xmqq36fda3i8.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"c263eb54b30b3e93abb74662d120d8bb882322bb.1569483748.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-28T04:38:23Z","receivedAt":"2019-10-28T04:39:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> While `git update-index` mostly ignores paths referring to index entries\n> whose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\n> skip-worktree bit (reading part), 2009-08-20), for reasons that are not\n> entirely obvious, the `--remove` option was made special: it _does_\n> remove index entries even if their skip-worktree bit is set.\n\nI think that made sense to notice removal of the path, because\nskip-worktree bit was not \"apply --cached semantics instead of\nlooking at the working tree files\".  In other words, it was only\nabout contents inside the files, and not existence of paths.\n\nI am not offhand sure if it still makes sense; if I were being asked\nto review that commit today, I suspect that I may be tempted to say\nthat we should ignore both contents change and presence change for\nentries that have skip-worktree bit set.\n\n> However, in preparation for fixing a bug in `git stash` where it\n> pretends that skip-worktree entries have actually been removed, we need\n> a mode where `git update-index` leaves all skip-worktree entries alone,\n> even if the `--remove` option was passed.\n\nWe might want to make this the default eventually (is there a known\nuse case where the current behaviour makes sense, I wonder?), but\nI agree that this is a safe thing to do at least for now.\n\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  Documentation/git-update-index.txt | 6 ++++++\n>  builtin/update-index.c             | 6 +++++-\n>  2 files changed, 11 insertions(+), 1 deletion(-)\n\nIsn't this something reasonably easy to guard against regression with\na test or two?\n\n>\n> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\n> index 1c4d146a41..08393445e7 100644\n> --- a/Documentation/git-update-index.txt\n> +++ b/Documentation/git-update-index.txt\n> @@ -16,6 +16,7 @@ SYNOPSIS\n>  \t     [--chmod=(+|-)x]\n>  \t     [--[no-]assume-unchanged]\n>  \t     [--[no-]skip-worktree]\n> +\t     [--[no-]ignore-skip-worktree-entries]\n>  \t     [--[no-]fsmonitor-valid]\n>  \t     [--ignore-submodules]\n>  \t     [--[no-]split-index]\n> @@ -113,6 +114,11 @@ you will need to handle the situation manually.\n>  \tset and unset the \"skip-worktree\" bit for the paths. See\n>  \tsection \"Skip-worktree bit\" below for more information.\n>  \n> +\n> +--[no-]ignore-skip-worktree-entries::\n> +\tDo not remove skip-worktree (AKA \"index-only\") entries even when\n> +\tthe `--remove` option was specified.\n> +\n>  --[no-]fsmonitor-valid::\n>  \tWhen one of these flags is specified, the object name recorded\n>  \tfor the paths are not updated. Instead, these options\n> diff --git a/builtin/update-index.c b/builtin/update-index.c\n> index dff2f4b837..074d563df0 100644\n> --- a/builtin/update-index.c\n> +++ b/builtin/update-index.c\n> @@ -35,6 +35,7 @@ static int verbose;\n>  static int mark_valid_only;\n>  static int mark_skip_worktree_only;\n>  static int mark_fsmonitor_only;\n> +static int ignore_skip_worktree_entries;\n>  #define MARK_FLAG 1\n>  #define UNMARK_FLAG 2\n>  static struct strbuf mtime_dir = STRBUF_INIT;\n> @@ -381,7 +382,8 @@ static int process_path(const char *path, struct stat *st, int stat_errno)\n>  \t\t * so updating it does not make sense.\n>  \t\t * On the other hand, removing it from index should work\n>  \t\t */\n> -\t\tif (allow_remove && remove_file_from_cache(path))\n> +\t\tif (!ignore_skip_worktree_entries && allow_remove &&\n> +\t\t    remove_file_from_cache(path))\n>  \t\t\treturn error(\"%s: cannot remove from the index\", path);\n>  \t\treturn 0;\n>  \t}\n> @@ -1013,6 +1015,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n>  \t\t{OPTION_SET_INT, 0, \"no-skip-worktree\", &mark_skip_worktree_only, NULL,\n>  \t\t\tN_(\"clear skip-worktree bit\"),\n>  \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG},\n> +\t\tOPT_BOOL(0, \"ignore-skip-worktree-entries\", &ignore_skip_worktree_entries,\n> +\t\t\t N_(\"do not touch index-only entries\")),\n>  \t\tOPT_SET_INT(0, \"info-only\", &info_only,\n>  \t\t\tN_(\"add to index only; do not add content to object database\"), 1),\n>  \t\tOPT_SET_INT(0, \"force-remove\", &force_remove,\n"},{"id":"384967","messageId":"xmqqy2x58mas.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"4c684be1794cd0c62cc19d9c9558981584ec3e9a.1569483748.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] stash: handle staged changes in skip-worktree files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-28T05:35:23Z","receivedAt":"2019-10-28T05:35:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> When calling `git stash` while changes were staged for files that are\n> marked with the `skip-worktree` bit (e.g. files that are excluded in a\n> sparse checkout), the files are recorded as _deleted_ instead.\n\nAssuming that 1/2 makes sense to all of us, I think the changes in\n2/2 make 100% sense.\n\nWill queue.  Thanks.\n"},{"id":"384975","messageId":"pull.355.v2.git.1572261615.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Fix git stash with skip-worktree entries","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T11:20:13Z","receivedAt":"2019-10-28T11:20:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"My colleague Dan Thompson reported a bug in a sparse checkout, where git\nstash (after resolving merge conflicts and then making up their mind to\nstash the changes instead of committing them) would \"lose\" files (and files\nthat were not even in the sparse checkout's cone!).\n\nI first considered changing the behavior of git diff-index to simply ignore\nskip-worktree entries. But after re-reading the documentation of the\nskip-worktree bit, I became convinced that this would be incorrect a fix\nbecause the command really does what it advertises to do.\n\nThen, I briefly considered introducing a flag that would change the behavior\nthusly, but ended up deciding against it.\n\nThe actual problem, after all, is the git update-index call and that it\nheeds the --remove (but not the --add) option for skip-worktree entries.\n\"Heeds\", I should say, because the idea of the skip-worktree bit really is\ndocumented to imply that the worktree files should be considered identical\nto their staged versions.\n\nSo arguably, it could be considered a bug that git update-index --remove \neven bothers to touch skip-worktree entries. But this behavior has been in\nplace for over 10 years, so I opted to introduce a new mode that does what \ngit stash needs in order to fix the bug.\n\nChanges since v1:\n\n * Added a test even for the --ignore-skip-worktree-entries option alone\n   (i.e. without going through git stash)\n * Rebased onto tg/stash-refresh-index to avoid merge conflicts in \n   t/t3903-stash.sh.\n\nJohannes Schindelin (2):\n  update-index: optionally leave skip-worktree entries alone\n  stash: handle staged changes in skip-worktree files correctly\n\n Documentation/git-update-index.txt |  6 ++++++\n builtin/stash.c                    |  5 +++--\n builtin/update-index.c             |  6 +++++-\n git-legacy-stash.sh                |  3 ++-\n t/t3903-stash.sh                   | 11 +++++++++++\n t/t7012-skip-worktree-writing.sh   | 14 ++++++++++++++\n 6 files changed, 41 insertions(+), 4 deletions(-)\n\n\nbase-commit: 34933d0eff5d4c91fae6ad6f71a6e6a69a496ced\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-355%2Fdscho%2Ffix-stash-with-skip-worktree-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-355/dscho/fix-stash-with-skip-worktree-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/355\n\nRange-diff vs v1:\n\n 1:  c263eb54b3 ! 1:  86dbb11f15 update-index: optionally leave skip-worktree entries alone\n     @@ -72,3 +72,28 @@\n       \t\tOPT_SET_INT(0, \"info-only\", &info_only,\n       \t\t\tN_(\"add to index only; do not add content to object database\"), 1),\n       \t\tOPT_SET_INT(0, \"force-remove\", &force_remove,\n     +\n     + diff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh\n     + --- a/t/t7012-skip-worktree-writing.sh\n     + +++ b/t/t7012-skip-worktree-writing.sh\n     +@@\n     + \ttest_i18ncmp expected result\n     + '\n     + \n     ++test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' '\n     ++\ttest_commit geroff-me &&\n     ++\tgit update-index --skip-worktree geroff-me.t &&\n     ++\trm geroff-me.t &&\n     ++\n     ++\t: ignoring the worktree &&\n     ++\tgit update-index --remove --ignore-skip-worktree-entries geroff-me.t &&\n     ++\tgit diff-index --cached --exit-code HEAD &&\n     ++\n     ++\t: not ignoring the worktree, a deletion is staged &&\n     ++\tgit update-index --remove geroff-me.t &&\n     ++\ttest_must_fail git diff-index --cached --exit-code HEAD\n     ++'\n     ++\n     + #TODO test_expect_failure 'git-apply adds file' false\n     + #TODO test_expect_failure 'git-apply updates file' false\n     + #TODO test_expect_failure 'git-apply removes file' false\n 2:  4c684be179 ! 2:  9835e66399 stash: handle staged changes in skip-worktree files correctly\n     @@ -59,7 +59,7 @@\n       --- a/t/t3903-stash.sh\n       +++ b/t/t3903-stash.sh\n      @@\n     - \ttest_path_is_missing to-remove\n     + \tgit stash apply\n       '\n       \n      +test_expect_success 'stash handles skip-worktree entries nicely' '\n\n-- \ngitgitgadget\n"},{"id":"384976","messageId":"9835e66399b5feeb3313bcee62f0519c025a4ca2.1572261615.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.v2.git.1572261615.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] stash: handle staged changes in skip-worktree files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T11:20:15Z","receivedAt":"2019-10-28T11:20:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen calling `git stash` while changes were staged for files that are\nmarked with the `skip-worktree` bit (e.g. files that are excluded in a\nsparse checkout), the files are recorded as _deleted_ instead.\n\nThe reason is that `git stash` tries to construct the tree reflecting\nthe worktree essentially by copying the index to a temporary one and\nthen updating the files from the worktree. Crucially, it calls `git\ndiff-index` to update also those files that are in the HEAD but have\nbeen unstaged in the index.\n\nHowever, when the temporary index is updated via `git update-index --add\n--remove`, skip-worktree entries mark the files as deleted by mistake.\n\nLet's use the newly-introduced `--ignore-skip-worktree-entries` option\nof `git update-index` to prevent exactly this from happening.\n\nNote that the regression test case deliberately avoids replicating the\nscenario described above and instead tries to recreate just the symptom.\n\nReported by Dan Thompson.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/stash.c     |  5 +++--\n git-legacy-stash.sh |  3 ++-\n t/t3903-stash.sh    | 11 +++++++++++\n 3 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex ab30d1e920..e3962bf73e 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1082,8 +1082,9 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \t}\n \n \tcp_upd_index.git_cmd = 1;\n-\targv_array_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n-\t\t\t \"--remove\", \"--stdin\", NULL);\n+\targv_array_pushl(&cp_upd_index.args, \"update-index\",\n+\t\t\t \"--ignore-skip-worktree-entries\",\n+\t\t\t \"-z\", \"--add\", \"--remove\", \"--stdin\", NULL);\n \targv_array_pushf(&cp_upd_index.env_array, \"GIT_INDEX_FILE=%s\",\n \t\t\t stash_index_path.buf);\n \ndiff --git a/git-legacy-stash.sh b/git-legacy-stash.sh\nindex f60e9b3e87..5398a5161d 100755\n--- a/git-legacy-stash.sh\n+++ b/git-legacy-stash.sh\n@@ -193,7 +193,8 @@ create_stash () {\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n \t\t\tgit diff-index --name-only -z HEAD -- \"$@\" >\"$TMP-stagenames\" &&\n-\t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n+\t\t\tgit update-index --ignore-skip-worktree-entries \\\n+\t\t\t\t-z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n \t\t) ) ||\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 392954d6dd..57258d5668 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1257,4 +1257,15 @@ test_expect_success 'stash apply should succeed with unmodified file' '\n \tgit stash apply\n '\n \n+test_expect_success 'stash handles skip-worktree entries nicely' '\n+\ttest_commit A &&\n+\techo changed >A.t &&\n+\tgit add A.t &&\n+\tgit update-index --skip-worktree A.t &&\n+\trm A.t &&\n+\tgit stash &&\n+\n+\tgit rev-parse --verify refs/stash:A.t\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"384977","messageId":"86dbb11f159375da281acd6266df019106abeadb.1572261615.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.v2.git.1572261615.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T11:20:14Z","receivedAt":"2019-10-28T11:20:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhile `git update-index` mostly ignores paths referring to index entries\nwhose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\nskip-worktree bit (reading part), 2009-08-20), for reasons that are not\nentirely obvious, the `--remove` option was made special: it _does_\nremove index entries even if their skip-worktree bit is set.\n\nSeeing as this behavior has been in place for a decade now, it does not\nmake sense to change it.\n\nHowever, in preparation for fixing a bug in `git stash` where it\npretends that skip-worktree entries have actually been removed, we need\na mode where `git update-index` leaves all skip-worktree entries alone,\neven if the `--remove` option was passed.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-update-index.txt |  6 ++++++\n builtin/update-index.c             |  6 +++++-\n t/t7012-skip-worktree-writing.sh   | 14 ++++++++++++++\n 3 files changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex 1c4d146a41..08393445e7 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -16,6 +16,7 @@ SYNOPSIS\n \t     [--chmod=(+|-)x]\n \t     [--[no-]assume-unchanged]\n \t     [--[no-]skip-worktree]\n+\t     [--[no-]ignore-skip-worktree-entries]\n \t     [--[no-]fsmonitor-valid]\n \t     [--ignore-submodules]\n \t     [--[no-]split-index]\n@@ -113,6 +114,11 @@ you will need to handle the situation manually.\n \tset and unset the \"skip-worktree\" bit for the paths. See\n \tsection \"Skip-worktree bit\" below for more information.\n \n+\n+--[no-]ignore-skip-worktree-entries::\n+\tDo not remove skip-worktree (AKA \"index-only\") entries even when\n+\tthe `--remove` option was specified.\n+\n --[no-]fsmonitor-valid::\n \tWhen one of these flags is specified, the object name recorded\n \tfor the paths are not updated. Instead, these options\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex dff2f4b837..074d563df0 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -35,6 +35,7 @@ static int verbose;\n static int mark_valid_only;\n static int mark_skip_worktree_only;\n static int mark_fsmonitor_only;\n+static int ignore_skip_worktree_entries;\n #define MARK_FLAG 1\n #define UNMARK_FLAG 2\n static struct strbuf mtime_dir = STRBUF_INIT;\n@@ -381,7 +382,8 @@ static int process_path(const char *path, struct stat *st, int stat_errno)\n \t\t * so updating it does not make sense.\n \t\t * On the other hand, removing it from index should work\n \t\t */\n-\t\tif (allow_remove && remove_file_from_cache(path))\n+\t\tif (!ignore_skip_worktree_entries && allow_remove &&\n+\t\t    remove_file_from_cache(path))\n \t\t\treturn error(\"%s: cannot remove from the index\", path);\n \t\treturn 0;\n \t}\n@@ -1013,6 +1015,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t{OPTION_SET_INT, 0, \"no-skip-worktree\", &mark_skip_worktree_only, NULL,\n \t\t\tN_(\"clear skip-worktree bit\"),\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG},\n+\t\tOPT_BOOL(0, \"ignore-skip-worktree-entries\", &ignore_skip_worktree_entries,\n+\t\t\t N_(\"do not touch index-only entries\")),\n \t\tOPT_SET_INT(0, \"info-only\", &info_only,\n \t\t\tN_(\"add to index only; do not add content to object database\"), 1),\n \t\tOPT_SET_INT(0, \"force-remove\", &force_remove,\ndiff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh\nindex 9d1abe50ef..28b1b0b2b9 100755\n--- a/t/t7012-skip-worktree-writing.sh\n+++ b/t/t7012-skip-worktree-writing.sh\n@@ -134,6 +134,20 @@ test_expect_success 'git-clean, dirty case' '\n \ttest_i18ncmp expected result\n '\n \n+test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' '\n+\ttest_commit geroff-me &&\n+\tgit update-index --skip-worktree geroff-me.t &&\n+\trm geroff-me.t &&\n+\n+\t: ignoring the worktree &&\n+\tgit update-index --remove --ignore-skip-worktree-entries geroff-me.t &&\n+\tgit diff-index --cached --exit-code HEAD &&\n+\n+\t: not ignoring the worktree, a deletion is staged &&\n+\tgit update-index --remove geroff-me.t &&\n+\ttest_must_fail git diff-index --cached --exit-code HEAD\n+'\n+\n #TODO test_expect_failure 'git-apply adds file' false\n #TODO test_expect_failure 'git-apply updates file' false\n #TODO test_expect_failure 'git-apply removes file' false\n-- \ngitgitgadget\n\n"},{"id":"385013","messageId":"nycvar.QRO.7.76.6.1910282152080.46@tvgsbejvaqbjf.bet","threadId":"51923","inReplyTo":"xmqqpniha9a8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/2] Fix git stash with skip-worktree entries","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-28T20:56:05Z","receivedAt":"2019-10-28T20:56:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 28 Oct 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> > On Thu, 26 Sep 2019, Johannes Schindelin via GitGitGadget wrote:\n> >\n> >> My colleague Dan Thompson reported a bug in a sparse checkout,\n> >> where git stash (after resolving merge conflicts and then making up\n> >> their mind to stash the changes instead of committing them) would\n> >> \"lose\" files (and files that were not even in the sparse checkout's\n> >> cone!).\n> >\n> > I only realized _now_ that this patch has not made it anywhere.\n>\n> Yeah, I do not recall seeing any of the patches in the topic (nor\n> the cover letter).  It is not clear to me what \"lose\" above means,\n> which is an indication that I didn't read the topic a month ago X-<.\n\nSorry, in this context, \"to lose a file\" means that a `git stash` would\nrecord such a file as deleted. A subsequent `git stash apply` _would_\nthen delete it.\n\n> Did it even get any review by skip worktree bit experts back then?\n\nSadly, it did not get any review back then...\n\nCiao,\nDscho\n"},{"id":"385014","messageId":"nycvar.QRO.7.76.6.1910282206081.46@tvgsbejvaqbjf.bet","threadId":"51923","inReplyTo":"xmqq36fda3i8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-28T21:07:15Z","receivedAt":"2019-10-28T21:07:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 28 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > While `git update-index` mostly ignores paths referring to index entries\n> > whose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\n> > skip-worktree bit (reading part), 2009-08-20), for reasons that are not\n> > entirely obvious, the `--remove` option was made special: it _does_\n> > remove index entries even if their skip-worktree bit is set.\n>\n> I think that made sense to notice removal of the path, because\n> skip-worktree bit was not \"apply --cached semantics instead of\n> looking at the working tree files\".  In other words, it was only\n> about contents inside the files, and not existence of paths.\n>\n> I am not offhand sure if it still makes sense; if I were being asked\n> to review that commit today, I suspect that I may be tempted to say\n> that we should ignore both contents change and presence change for\n> entries that have skip-worktree bit set.\n>\n> > However, in preparation for fixing a bug in `git stash` where it\n> > pretends that skip-worktree entries have actually been removed, we need\n> > a mode where `git update-index` leaves all skip-worktree entries alone,\n> > even if the `--remove` option was passed.\n>\n> We might want to make this the default eventually (is there a known\n> use case where the current behaviour makes sense, I wonder?), but\n> I agree that this is a safe thing to do at least for now.\n>\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  Documentation/git-update-index.txt | 6 ++++++\n> >  builtin/update-index.c             | 6 +++++-\n> >  2 files changed, 11 insertions(+), 1 deletion(-)\n>\n> Isn't this something reasonably easy to guard against regression with\n> a test or two?\n\nI sent out a v2 with tests added to 1/2.\n\nThanks,\nDscho\n\n>\n> >\n> > diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\n> > index 1c4d146a41..08393445e7 100644\n> > --- a/Documentation/git-update-index.txt\n> > +++ b/Documentation/git-update-index.txt\n> > @@ -16,6 +16,7 @@ SYNOPSIS\n> >  \t     [--chmod=(+|-)x]\n> >  \t     [--[no-]assume-unchanged]\n> >  \t     [--[no-]skip-worktree]\n> > +\t     [--[no-]ignore-skip-worktree-entries]\n> >  \t     [--[no-]fsmonitor-valid]\n> >  \t     [--ignore-submodules]\n> >  \t     [--[no-]split-index]\n> > @@ -113,6 +114,11 @@ you will need to handle the situation manually.\n> >  \tset and unset the \"skip-worktree\" bit for the paths. See\n> >  \tsection \"Skip-worktree bit\" below for more information.\n> >\n> > +\n> > +--[no-]ignore-skip-worktree-entries::\n> > +\tDo not remove skip-worktree (AKA \"index-only\") entries even when\n> > +\tthe `--remove` option was specified.\n> > +\n> >  --[no-]fsmonitor-valid::\n> >  \tWhen one of these flags is specified, the object name recorded\n> >  \tfor the paths are not updated. Instead, these options\n> > diff --git a/builtin/update-index.c b/builtin/update-index.c\n> > index dff2f4b837..074d563df0 100644\n> > --- a/builtin/update-index.c\n> > +++ b/builtin/update-index.c\n> > @@ -35,6 +35,7 @@ static int verbose;\n> >  static int mark_valid_only;\n> >  static int mark_skip_worktree_only;\n> >  static int mark_fsmonitor_only;\n> > +static int ignore_skip_worktree_entries;\n> >  #define MARK_FLAG 1\n> >  #define UNMARK_FLAG 2\n> >  static struct strbuf mtime_dir = STRBUF_INIT;\n> > @@ -381,7 +382,8 @@ static int process_path(const char *path, struct stat *st, int stat_errno)\n> >  \t\t * so updating it does not make sense.\n> >  \t\t * On the other hand, removing it from index should work\n> >  \t\t */\n> > -\t\tif (allow_remove && remove_file_from_cache(path))\n> > +\t\tif (!ignore_skip_worktree_entries && allow_remove &&\n> > +\t\t    remove_file_from_cache(path))\n> >  \t\t\treturn error(\"%s: cannot remove from the index\", path);\n> >  \t\treturn 0;\n> >  \t}\n> > @@ -1013,6 +1015,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n> >  \t\t{OPTION_SET_INT, 0, \"no-skip-worktree\", &mark_skip_worktree_only, NULL,\n> >  \t\t\tN_(\"clear skip-worktree bit\"),\n> >  \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG},\n> > +\t\tOPT_BOOL(0, \"ignore-skip-worktree-entries\", &ignore_skip_worktree_entries,\n> > +\t\t\t N_(\"do not touch index-only entries\")),\n> >  \t\tOPT_SET_INT(0, \"info-only\", &info_only,\n> >  \t\t\tN_(\"add to index only; do not add content to object database\"), 1),\n> >  \t\tOPT_SET_INT(0, \"force-remove\", &force_remove,\n>\n"},{"id":"385037","messageId":"xmqqzhhk70fd.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"nycvar.QRO.7.76.6.1910282152080.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/2] Fix git stash with skip-worktree entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-29T02:25:26Z","receivedAt":"2019-10-29T02:25:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Yeah, I do not recall seeing any of the patches in the topic (nor\n>> the cover letter).  It is not clear to me what \"lose\" above means,\n>> which is an indication that I didn't read the topic a month ago X-<.\n>\n> Sorry, in this context, \"to lose a file\" means that a `git stash` would\n> record such a file as deleted. A subsequent `git stash apply` _would_\n> then delete it.\n\nThat would be a good thing to explain in the log message.\n\nThanks.\n"},{"id":"385039","messageId":"xmqqr22w70c2.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"nycvar.QRO.7.76.6.1910282206081.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-29T02:27:25Z","receivedAt":"2019-10-29T02:27:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Isn't this something reasonably easy to guard against regression with\n>> a test or two?\n>\n> I sent out a v2 with tests added to 1/2.\n\nGood; that way, not just \"stash\" but anything that relies on update-index\nwould be protected from regressions.\n\nThanks.\n"},{"id":"385048","messageId":"nycvar.QRO.7.76.6.1910290913110.46@tvgsbejvaqbjf.bet","threadId":"51923","inReplyTo":"xmqqzhhk70fd.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/2] Fix git stash with skip-worktree entries","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-29T08:15:12Z","receivedAt":"2019-10-29T08:15:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 29 Oct 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> >> Yeah, I do not recall seeing any of the patches in the topic (nor\n> >> the cover letter).  It is not clear to me what \"lose\" above means,\n> >> which is an indication that I didn't read the topic a month ago X-<.\n> >\n> > Sorry, in this context, \"to lose a file\" means that a `git stash` would\n> > record such a file as deleted. A subsequent `git stash apply` _would_\n> > then delete it.\n>\n> That would be a good thing to explain in the log message.\n\nI thought that this paragraph in the commit message was describing this\nissue appropriately?\n\n    When calling `git stash` while changes were staged for files that are\n    marked with the `skip-worktree` bit (e.g. files that are excluded in a\n    sparse checkout), the files are recorded as _deleted_ instead.\n\nDo you agree that this is good enough?\n\nCiao,\nDscho\n"},{"id":"385124","messageId":"xmqq5zk7593d.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"86dbb11f159375da281acd6266df019106abeadb.1572261615.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-30T01:13:26Z","receivedAt":"2019-10-30T01:13:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> While `git update-index` mostly ignores paths referring to index entries\n> whose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\n> skip-worktree bit (reading part), 2009-08-20), for reasons that are not\n> entirely obvious, the `--remove` option was made special: it _does_\n> remove index entries even if their skip-worktree bit is set.\n>\n> Seeing as this behavior has been in place for a decade now, it does not\n> make sense to change it.\n\nIf this were end-user facing Porcelain behaviour, even it is a\ndecade old, the story would have been different, but given that it\nis in an obscure corner in a plumbing command, I agree that it does\nnot make sense to even transition the default over releases.\n\n> +test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' '\n> +\ttest_commit geroff-me &&\n> +\tgit update-index --skip-worktree geroff-me.t &&\n> +\trm geroff-me.t &&\n\nI do not see a need to swear with a sample file name.  It may make\nsense to use words that relate to what the test is checking (e.g.\nskip-me or something like that), but otherwise meaningless filenames\nused in other tests (like 1, 2, etc) would be more in line with the\nexisting tests.\n\n> +\n> +\t: ignoring the worktree &&\n> +\tgit update-index --remove --ignore-skip-worktree-entries geroff-me.t &&\n> +\tgit diff-index --cached --exit-code HEAD &&\n\nHEAD has it, working tree does not, and the one in the index should\nhave been kept thanks to the new option added by this patch.  Makes\nsense.\n\n> +\t: not ignoring the worktree, a deletion is staged &&\n> +\tgit update-index --remove geroff-me.t &&\n> +\ttest_must_fail git diff-index --cached --exit-code HEAD\n\nTesting the other side of the coin (i.e. adding the new feature did\nnot accidentally stop the command from removing by default) is good;\n\"should have no difference\" was a good test for the other side, but\nin contrast, \"should have some difference\" is a very loose test when\nthe difference we want to see is that the particular path gets removed\nand no other changes.\n\n> +'\n> +\n>  #TODO test_expect_failure 'git-apply adds file' false\n>  #TODO test_expect_failure 'git-apply updates file' false\n>  #TODO test_expect_failure 'git-apply removes file' false\n"},{"id":"385125","messageId":"xmqq1ruv58yv.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"9835e66399b5feeb3313bcee62f0519c025a4ca2.1572261615.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] stash: handle staged changes in skip-worktree files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-30T01:16:08Z","receivedAt":"2019-10-30T01:16:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> When calling `git stash` while changes were staged for files that are\n> marked with the `skip-worktree` bit (e.g. files that are excluded in a\n> sparse checkout), the files are recorded as _deleted_ instead.\n\nGood.  Much easier to see what is going on than the verb \"lose\" used\nin the cover letter of v1 that puzzled me.\n\n> However, when the temporary index is updated via `git update-index --add\n> --remove`, skip-worktree entries mark the files as deleted by mistake.\n>\n> Let's use the newly-introduced `--ignore-skip-worktree-entries` option\n> of `git update-index` to prevent exactly this from happening.\n\nGood.\n\n> Note that the regression test case deliberately avoids replicating the\n> scenario described above and instead tries to recreate just the symptom.\n\nThat's good.  Testing the end-user visible effect is just as\nimportant as the narrowly pointed root-cause test like the one in\nthe previous patch.\n\nThanks.\n"},{"id":"385146","messageId":"nycvar.QRO.7.76.6.1910300930440.46@tvgsbejvaqbjf.bet","threadId":"51923","inReplyTo":"xmqq5zk7593d.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-30T08:34:38Z","receivedAt":"2019-10-30T08:35:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 30 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > While `git update-index` mostly ignores paths referring to index entries\n> > whose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\n> > skip-worktree bit (reading part), 2009-08-20), for reasons that are not\n> > entirely obvious, the `--remove` option was made special: it _does_\n> > remove index entries even if their skip-worktree bit is set.\n> >\n> > Seeing as this behavior has been in place for a decade now, it does not\n> > make sense to change it.\n>\n> If this were end-user facing Porcelain behaviour, even it is a\n> decade old, the story would have been different, but given that it\n> is in an obscure corner in a plumbing command, I agree that it does\n> not make sense to even transition the default over releases.\n>\n> > +test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' '\n> > +\ttest_commit geroff-me &&\n> > +\tgit update-index --skip-worktree geroff-me.t &&\n> > +\trm geroff-me.t &&\n>\n> I do not see a need to swear with a sample file name.  It may make\n> sense to use words that relate to what the test is checking (e.g.\n> skip-me or something like that), but otherwise meaningless filenames\n> used in other tests (like 1, 2, etc) would be more in line with the\n> existing tests.\n\nI changed this locally to `keep-me`. But then I saw that you merged this\npatch pair to `next` already... Do you want an add-on patch, or revert\nit out of `next`, or leave as-is?\n\nI'd like to know because I still want to merge this into Git for Windows\nv2.24.0-rc2, and I would love to deviate as little as possible from\ngit.git there.\n\n> > +\n> > +\t: ignoring the worktree &&\n> > +\tgit update-index --remove --ignore-skip-worktree-entries geroff-me.t &&\n> > +\tgit diff-index --cached --exit-code HEAD &&\n>\n> HEAD has it, working tree does not, and the one in the index should\n> have been kept thanks to the new option added by this patch.  Makes\n> sense.\n>\n> > +\t: not ignoring the worktree, a deletion is staged &&\n> > +\tgit update-index --remove geroff-me.t &&\n> > +\ttest_must_fail git diff-index --cached --exit-code HEAD\n>\n> Testing the other side of the coin (i.e. adding the new feature did\n> not accidentally stop the command from removing by default) is good;\n> \"should have no difference\" was a good test for the other side, but\n> in contrast, \"should have some difference\" is a very loose test when\n> the difference we want to see is that the particular path gets removed\n> and no other changes.\n\nTrue. I changed it to `test_must_fail git rev-parse :keep-me` locally\n(to test for the staged deletion, although it just occurred to me that I\nwould rather want to add the `--diff-filter=D` option and filter by the\nfile name to really verify that a deletion was staged), but again, I\nnoticed that you already merged this to `next`...\n\nSo: revert out of `next`, add-on patch, or leave as-is?\n\nThanks,\nDscho\n\n>\n> > +'\n> > +\n> >  #TODO test_expect_failure 'git-apply adds file' false\n> >  #TODO test_expect_failure 'git-apply updates file' false\n> >  #TODO test_expect_failure 'git-apply removes file' false\n>\n>\n"},{"id":"385156","messageId":"pull.355.v3.git.1572432578.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.v2.git.1572261615.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] Fix git stash with skip-worktree entries","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-30T10:49:36Z","receivedAt":"2019-10-30T10:49:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"[This patch pair has made it into next already, but since we're too close to\nv2.24.0, I think it would be good to revert it out of next and take v3\ninstead.]\n\nMy colleague Dan Thompson reported a bug in a sparse checkout, where git\nstash (after resolving merge conflicts and then making up their mind to\nstash the changes instead of committing them) would record files as deleted\nby mistake (and files that were not even in the sparse checkout's cone!).\n\nI first considered changing the behavior of git diff-index to simply ignore\nskip-worktree entries. But after re-reading the documentation of the\nskip-worktree bit, I became convinced that this would be incorrect a fix\nbecause the command really does what it advertises to do.\n\nThen, I briefly considered introducing a flag that would change the behavior\nthusly, but ended up deciding against it.\n\nThe actual problem, after all, is the git update-index call and that it\nheeds the --remove (but not the --add) option for skip-worktree entries.\n\"Heeds\", I should say, because the idea of the skip-worktree bit really is\ndocumented to imply that the worktree files should be considered identical\nto their staged versions.\n\nSo arguably, it could be considered a bug that git update-index --remove \neven bothers to touch skip-worktree entries. But this behavior has been in\nplace for over 10 years, so I opted to introduce a new mode that does what \ngit stash needs in order to fix the bug.\n\nChanges since v2:\n\n * Avoid a file name that some might consider rude in the test of 1/2.\n * In the test of 1/2, verify explicitly that a deletion has been staged\n   without --ignore-skip-worktree-entries.\n\nChanges since v1:\n\n * Added a test even for the --ignore-skip-worktree-entries option alone\n   (i.e. without going through git stash)\n * Rebased onto tg/stash-refresh-index to avoid merge conflicts in \n   t/t3903-stash.sh.\n\nJohannes Schindelin (2):\n  update-index: optionally leave skip-worktree entries alone\n  stash: handle staged changes in skip-worktree files correctly\n\n Documentation/git-update-index.txt |  6 ++++++\n builtin/stash.c                    |  5 +++--\n builtin/update-index.c             |  6 +++++-\n git-legacy-stash.sh                |  3 ++-\n t/t3903-stash.sh                   | 11 +++++++++++\n t/t7012-skip-worktree-writing.sh   | 15 +++++++++++++++\n 6 files changed, 42 insertions(+), 4 deletions(-)\n\n\nbase-commit: 34933d0eff5d4c91fae6ad6f71a6e6a69a496ced\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-355%2Fdscho%2Ffix-stash-with-skip-worktree-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-355/dscho/fix-stash-with-skip-worktree-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/355\n\nRange-diff vs v2:\n\n 1:  86dbb11f15 ! 1:  163b42dfa2 update-index: optionally leave skip-worktree entries alone\n     @@ -81,17 +81,18 @@\n       '\n       \n      +test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' '\n     -+\ttest_commit geroff-me &&\n     -+\tgit update-index --skip-worktree geroff-me.t &&\n     -+\trm geroff-me.t &&\n     ++\ttest_commit keep-me &&\n     ++\tgit update-index --skip-worktree keep-me.t &&\n     ++\trm keep-me.t &&\n      +\n      +\t: ignoring the worktree &&\n     -+\tgit update-index --remove --ignore-skip-worktree-entries geroff-me.t &&\n     ++\tgit update-index --remove --ignore-skip-worktree-entries keep-me.t &&\n      +\tgit diff-index --cached --exit-code HEAD &&\n      +\n      +\t: not ignoring the worktree, a deletion is staged &&\n     -+\tgit update-index --remove geroff-me.t &&\n     -+\ttest_must_fail git diff-index --cached --exit-code HEAD\n     ++\tgit update-index --remove keep-me.t &&\n     ++\ttest_must_fail git diff-index --cached --exit-code HEAD \\\n     ++\t\t--diff-filter=D -- keep-me.t\n      +'\n      +\n       #TODO test_expect_failure 'git-apply adds file' false\n 2:  9835e66399 = 2:  8f49a393e0 stash: handle staged changes in skip-worktree files correctly\n\n-- \ngitgitgadget\n"},{"id":"385157","messageId":"163b42dfa21c306dc1dc573c5edfc8bda5c99fd0.1572432578.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.v3.git.1572432578.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-30T10:49:37Z","receivedAt":"2019-10-30T10:49:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhile `git update-index` mostly ignores paths referring to index entries\nwhose skip-worktree bit is set, in b4d1690df11 (Teach Git to respect\nskip-worktree bit (reading part), 2009-08-20), for reasons that are not\nentirely obvious, the `--remove` option was made special: it _does_\nremove index entries even if their skip-worktree bit is set.\n\nSeeing as this behavior has been in place for a decade now, it does not\nmake sense to change it.\n\nHowever, in preparation for fixing a bug in `git stash` where it\npretends that skip-worktree entries have actually been removed, we need\na mode where `git update-index` leaves all skip-worktree entries alone,\neven if the `--remove` option was passed.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-update-index.txt |  6 ++++++\n builtin/update-index.c             |  6 +++++-\n t/t7012-skip-worktree-writing.sh   | 15 +++++++++++++++\n 3 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex 1c4d146a41..08393445e7 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -16,6 +16,7 @@ SYNOPSIS\n \t     [--chmod=(+|-)x]\n \t     [--[no-]assume-unchanged]\n \t     [--[no-]skip-worktree]\n+\t     [--[no-]ignore-skip-worktree-entries]\n \t     [--[no-]fsmonitor-valid]\n \t     [--ignore-submodules]\n \t     [--[no-]split-index]\n@@ -113,6 +114,11 @@ you will need to handle the situation manually.\n \tset and unset the \"skip-worktree\" bit for the paths. See\n \tsection \"Skip-worktree bit\" below for more information.\n \n+\n+--[no-]ignore-skip-worktree-entries::\n+\tDo not remove skip-worktree (AKA \"index-only\") entries even when\n+\tthe `--remove` option was specified.\n+\n --[no-]fsmonitor-valid::\n \tWhen one of these flags is specified, the object name recorded\n \tfor the paths are not updated. Instead, these options\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex dff2f4b837..074d563df0 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -35,6 +35,7 @@ static int verbose;\n static int mark_valid_only;\n static int mark_skip_worktree_only;\n static int mark_fsmonitor_only;\n+static int ignore_skip_worktree_entries;\n #define MARK_FLAG 1\n #define UNMARK_FLAG 2\n static struct strbuf mtime_dir = STRBUF_INIT;\n@@ -381,7 +382,8 @@ static int process_path(const char *path, struct stat *st, int stat_errno)\n \t\t * so updating it does not make sense.\n \t\t * On the other hand, removing it from index should work\n \t\t */\n-\t\tif (allow_remove && remove_file_from_cache(path))\n+\t\tif (!ignore_skip_worktree_entries && allow_remove &&\n+\t\t    remove_file_from_cache(path))\n \t\t\treturn error(\"%s: cannot remove from the index\", path);\n \t\treturn 0;\n \t}\n@@ -1013,6 +1015,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t{OPTION_SET_INT, 0, \"no-skip-worktree\", &mark_skip_worktree_only, NULL,\n \t\t\tN_(\"clear skip-worktree bit\"),\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG},\n+\t\tOPT_BOOL(0, \"ignore-skip-worktree-entries\", &ignore_skip_worktree_entries,\n+\t\t\t N_(\"do not touch index-only entries\")),\n \t\tOPT_SET_INT(0, \"info-only\", &info_only,\n \t\t\tN_(\"add to index only; do not add content to object database\"), 1),\n \t\tOPT_SET_INT(0, \"force-remove\", &force_remove,\ndiff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh\nindex 9d1abe50ef..7476781979 100755\n--- a/t/t7012-skip-worktree-writing.sh\n+++ b/t/t7012-skip-worktree-writing.sh\n@@ -134,6 +134,21 @@ test_expect_success 'git-clean, dirty case' '\n \ttest_i18ncmp expected result\n '\n \n+test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' '\n+\ttest_commit keep-me &&\n+\tgit update-index --skip-worktree keep-me.t &&\n+\trm keep-me.t &&\n+\n+\t: ignoring the worktree &&\n+\tgit update-index --remove --ignore-skip-worktree-entries keep-me.t &&\n+\tgit diff-index --cached --exit-code HEAD &&\n+\n+\t: not ignoring the worktree, a deletion is staged &&\n+\tgit update-index --remove keep-me.t &&\n+\ttest_must_fail git diff-index --cached --exit-code HEAD \\\n+\t\t--diff-filter=D -- keep-me.t\n+'\n+\n #TODO test_expect_failure 'git-apply adds file' false\n #TODO test_expect_failure 'git-apply updates file' false\n #TODO test_expect_failure 'git-apply removes file' false\n-- \ngitgitgadget\n\n"},{"id":"385158","messageId":"8f49a393e0ef7fff955a02963ff5229fb9885e89.1572432578.git.gitgitgadget@gmail.com","threadId":"51923","inReplyTo":"pull.355.v3.git.1572432578.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] stash: handle staged changes in skip-worktree files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-30T10:49:38Z","receivedAt":"2019-10-30T10:49:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen calling `git stash` while changes were staged for files that are\nmarked with the `skip-worktree` bit (e.g. files that are excluded in a\nsparse checkout), the files are recorded as _deleted_ instead.\n\nThe reason is that `git stash` tries to construct the tree reflecting\nthe worktree essentially by copying the index to a temporary one and\nthen updating the files from the worktree. Crucially, it calls `git\ndiff-index` to update also those files that are in the HEAD but have\nbeen unstaged in the index.\n\nHowever, when the temporary index is updated via `git update-index --add\n--remove`, skip-worktree entries mark the files as deleted by mistake.\n\nLet's use the newly-introduced `--ignore-skip-worktree-entries` option\nof `git update-index` to prevent exactly this from happening.\n\nNote that the regression test case deliberately avoids replicating the\nscenario described above and instead tries to recreate just the symptom.\n\nReported by Dan Thompson.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/stash.c     |  5 +++--\n git-legacy-stash.sh |  3 ++-\n t/t3903-stash.sh    | 11 +++++++++++\n 3 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex ab30d1e920..e3962bf73e 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1082,8 +1082,9 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps\n \t}\n \n \tcp_upd_index.git_cmd = 1;\n-\targv_array_pushl(&cp_upd_index.args, \"update-index\", \"-z\", \"--add\",\n-\t\t\t \"--remove\", \"--stdin\", NULL);\n+\targv_array_pushl(&cp_upd_index.args, \"update-index\",\n+\t\t\t \"--ignore-skip-worktree-entries\",\n+\t\t\t \"-z\", \"--add\", \"--remove\", \"--stdin\", NULL);\n \targv_array_pushf(&cp_upd_index.env_array, \"GIT_INDEX_FILE=%s\",\n \t\t\t stash_index_path.buf);\n \ndiff --git a/git-legacy-stash.sh b/git-legacy-stash.sh\nindex f60e9b3e87..5398a5161d 100755\n--- a/git-legacy-stash.sh\n+++ b/git-legacy-stash.sh\n@@ -193,7 +193,8 @@ create_stash () {\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n \t\t\tgit diff-index --name-only -z HEAD -- \"$@\" >\"$TMP-stagenames\" &&\n-\t\t\tgit update-index -z --add --remove --stdin <\"$TMP-stagenames\" &&\n+\t\t\tgit update-index --ignore-skip-worktree-entries \\\n+\t\t\t\t-z --add --remove --stdin <\"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n \t\t) ) ||\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 392954d6dd..57258d5668 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1257,4 +1257,15 @@ test_expect_success 'stash apply should succeed with unmodified file' '\n \tgit stash apply\n '\n \n+test_expect_success 'stash handles skip-worktree entries nicely' '\n+\ttest_commit A &&\n+\techo changed >A.t &&\n+\tgit add A.t &&\n+\tgit update-index --skip-worktree A.t &&\n+\trm A.t &&\n+\tgit stash &&\n+\n+\tgit rev-parse --verify refs/stash:A.t\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"385288","messageId":"xmqqpnib567f.fsf@gitster-ct.c.googlers.com","threadId":"51923","inReplyTo":"nycvar.QRO.7.76.6.1910300930440.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-02T03:04:52Z","receivedAt":"2019-11-02T03:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>\n> I changed this locally to `keep-me`. But then I saw that you merged this\n> patch pair to `next` already... Do you want an add-on patch, or revert\n> it out of `next`, or leave as-is?\n>\n> I'd like to know because I still want to merge this into Git for Windows\n> v2.24.0-rc2, and I would love to deviate as little as possible from\n> git.git there.\n\n>...\n> So: revert out of `next`, add-on patch, or leave as-is?\n\nSorry for a late response.  I was under the weather and mostly\noffline for the past few days.\n\nWhichever is easier for GGG is fine, but for the purpose of\nresulting history, I think revert-replace would be the best.\n\nThanks.\n"},{"id":"385324","messageId":"nycvar.QRO.7.76.6.1911030002410.46@tvgsbejvaqbjf.bet","threadId":"51923","inReplyTo":"xmqqpnib567f.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/2] update-index: optionally leave skip-worktree entries alone","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-02T23:03:32Z","receivedAt":"2019-11-02T23:03:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sat, 2 Nov 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> > I changed this locally to `keep-me`. But then I saw that you merged this\n> > patch pair to `next` already... Do you want an add-on patch, or revert\n> > it out of `next`, or leave as-is?\n> >\n> > I'd like to know because I still want to merge this into Git for Windows\n> > v2.24.0-rc2, and I would love to deviate as little as possible from\n> > git.git there.\n>\n> >...\n> > So: revert out of `next`, add-on patch, or leave as-is?\n>\n> Sorry for a late response.  I was under the weather and mostly\n> offline for the past few days.\n\nI hope you are feeling better?\n\n> Whichever is easier for GGG is fine, but for the purpose of\n> resulting history, I think revert-replace would be the best.\n\nTechnically, GitGitGadget should be fine with reverting out and\nre-integrating. If not, that's a bug ;-)\n\nThanks for replacing,\nDscho\n"}]}