Skip to content

Fix git stash with skip-worktree entries #2378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 30, 2019

Conversation

dscho
Copy link
Member

@dscho dscho commented Oct 28, 2019

This Git for Windows PR corresponds to gitgitgadget#355, which will most likely not make it into Git v2.24.0.

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 "lose" files (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.

@dscho dscho added this to the Next release milestone Oct 30, 2019
@dscho dscho force-pushed the fix-stash-with-skip-worktree branch from 9835e66 to c4b7b8c Compare October 30, 2019 10:04
dscho added 2 commits October 30, 2019 11:11
While `git update-index` mostly ignores paths referring to index entries
whose skip-worktree bit is set, in b4d1690 (Teach Git to respect
skip-worktree bit (reading part), 2009-08-20), for reasons that are not
entirely obvious, the `--remove` option was made special: it _does_
remove index entries even if their skip-worktree bit is set.

Seeing as this behavior has been in place for a decade now, it does not
make sense to change it.

However, in preparation for fixing a bug in `git stash` where it
pretends that skip-worktree entries have actually been removed, we need
a mode where `git update-index` leaves all skip-worktree entries alone,
even if the `--remove` option was passed.

Signed-off-by: Johannes Schindelin <[email protected]>
When calling `git stash` while changes were staged for files that are
marked with the `skip-worktree` bit (e.g. files that are excluded in a
sparse checkout), the files are recorded as _deleted_ instead.

The reason is that `git stash` tries to construct the tree reflecting
the worktree essentially by copying the index to a temporary one and
then updating the files from the worktree. Crucially, it calls `git
diff-index` to update also those files that are in the HEAD but have
been unstaged in the index.

However, when the temporary index is updated via `git update-index --add
--remove`, skip-worktree entries mark the files as deleted by mistake.

Let's use the newly-introduced `--ignore-skip-worktree-entries` option
of `git update-index` to prevent exactly this from happening.

Note that the regression test case deliberately avoids replicating the
scenario described above and instead tries to recreate just the symptom.

Reported by Dan Thompson.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the fix-stash-with-skip-worktree branch from c4b7b8c to 8f49a39 Compare October 30, 2019 10:12
@dscho dscho merged commit cd663ba into git-for-windows:master Oct 30, 2019
dscho added a commit to dscho/git that referenced this pull request Oct 30, 2019
…p-worktree

Fix `git stash` with skip-worktree entries
dscho added a commit to git-for-windows/build-extra that referenced this pull request Oct 30, 2019
`git stash` [no longer records skip-worktree files as
deleted](git-for-windows/git#2378) after
resolving merge conflicts in them.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Oct 30, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Oct 30, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Oct 30, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Oct 30, 2019
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Nov 2, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Nov 2, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit to dscho/git that referenced this pull request Nov 4, 2019
…p-worktree

Fix `git stash` with skip-worktree entries
@dscho dscho mentioned this pull request Nov 4, 2019
git-for-windows-ci pushed a commit that referenced this pull request Nov 4, 2019
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Nov 5, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Nov 6, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Nov 6, 2019
Fix `git stash` with skip-worktree entries
@dscho dscho deleted the fix-stash-with-skip-worktree branch November 11, 2019 10:56
dscho added a commit that referenced this pull request Nov 16, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Nov 25, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Nov 26, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Dec 7, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Dec 10, 2019
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Dec 10, 2019
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Dec 13, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Dec 29, 2019
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Jan 4, 2020
Fix `git stash` with skip-worktree entries
dscho added a commit that referenced this pull request Jan 13, 2020
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Jan 16, 2020
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Jan 17, 2020
Fix `git stash` with skip-worktree entries
git-for-windows-ci pushed a commit that referenced this pull request Jan 22, 2020
Fix `git stash` with skip-worktree entries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant