[This patch pair has made it into next already, but since we're too close to v2.24.0, I think it would be good to revert it out of next and take v3 instead.] My colleague Dan Thompson reported a bug in a sparse checkout, where git stash (after resolving merge conflicts and then making up their mind to stash the changes instead of committing them) would record files as deleted by mistake (and files that were not even in the sparse checkout's cone!). I first considered changing the behavior of git diff-index to simply ignore skip-worktree entries. But after re-reading the documentation of the skip-worktree bit, I became convinced that this would be incorrect a fix because the command really does what it advertises to do. Then, I briefly considered introducing a flag that would change the behavior thusly, but ended up deciding against it. The actual problem, after all, is the git update-index call and that it heeds the --remove (but not the --add) option for skip-worktree entries. "Heeds", I should say, because the idea of the skip-worktree bit really is documented to imply that the worktree files should be considered identical to their staged versions. So arguably, it could be considered a bug that git update-index --remove even bothers to touch skip-worktree entries. But this behavior has been in place for over 10 years, so I opted to introduce a new mode that does what git stash needs in order to fix the bug. Changes since v2: * Avoid a file name that some might consider rude in the test of 1/2. * In the test of 1/2, verify explicitly that a deletion has been staged without --ignore-skip-worktree-entries. Changes since v1: * Added a test even for the --ignore-skip-worktree-entries option alone (i.e. without going through git stash) * Rebased onto tg/stash-refresh-index to avoid merge conflicts in t/t3903-stash.sh. Johannes Schindelin (2): update-index: optionally leave skip-worktree entries alone stash: handle staged changes in skip-worktree files correctly Documentation/git-update-index.txt | 6 ++++++ builtin/stash.c | 5 +++-- builtin/update-index.c | 6 +++++- git-legacy-stash.sh | 3 ++- t/t3903-stash.sh | 11 +++++++++++ t/t7012-skip-worktree-writing.sh | 15 +++++++++++++++ 6 files changed, 42 insertions(+), 4 deletions(-) base-commit: 34933d0eff5d4c91fae6ad6f71a6e6a69a496ced Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-355%2Fdscho%2Ffix-stash-with-skip-worktree-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-355/dscho/fix-stash-with-skip-worktree-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/355 Range-diff vs v2: 1: 86dbb11f15 ! 1: 163b42dfa2 update-index: optionally leave skip-worktree entries alone @@ -81,17 +81,18 @@ ' +test_expect_success '--ignore-skip-worktree-entries leaves worktree alone' ' -+ test_commit geroff-me && -+ git update-index --skip-worktree geroff-me.t && -+ rm geroff-me.t && ++ test_commit keep-me && ++ git update-index --skip-worktree keep-me.t && ++ rm keep-me.t && + + : ignoring the worktree && -+ git update-index --remove --ignore-skip-worktree-entries geroff-me.t && ++ git update-index --remove --ignore-skip-worktree-entries keep-me.t && + git diff-index --cached --exit-code HEAD && + + : not ignoring the worktree, a deletion is staged && -+ git update-index --remove geroff-me.t && -+ test_must_fail git diff-index --cached --exit-code HEAD ++ git update-index --remove keep-me.t && ++ test_must_fail git diff-index --cached --exit-code HEAD \ ++ --diff-filter=D -- keep-me.t +' + #TODO test_expect_failure 'git-apply adds file' false 2: 9835e66399 = 2: 8f49a393e0 stash: handle staged changes in skip-worktree files correctly -- gitgitgadget
