git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v3 2/2] stash: handle staged changes in skip-worktree files correctly
Date: Wed, 30 Oct 2019 10:49:38 +0000	[thread overview]
Message-ID: <8f49a393e0ef7fff955a02963ff5229fb9885e89.1572432578.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.355.v3.git.1572432578.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 builtin/stash.c     |  5 +++--
 git-legacy-stash.sh |  3 ++-
 t/t3903-stash.sh    | 11 +++++++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index ab30d1e920..e3962bf73e 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1082,8 +1082,9 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps
 	}
 
 	cp_upd_index.git_cmd = 1;
-	argv_array_pushl(&cp_upd_index.args, "update-index", "-z", "--add",
-			 "--remove", "--stdin", NULL);
+	argv_array_pushl(&cp_upd_index.args, "update-index",
+			 "--ignore-skip-worktree-entries",
+			 "-z", "--add", "--remove", "--stdin", NULL);
 	argv_array_pushf(&cp_upd_index.env_array, "GIT_INDEX_FILE=%s",
 			 stash_index_path.buf);
 
diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
index f60e9b3e87..5398a5161d 100755
--- a/git-legacy-stash.sh
+++ b/git-legacy-stash.sh
@@ -193,7 +193,8 @@ create_stash () {
 			GIT_INDEX_FILE="$TMPindex" &&
 			export GIT_INDEX_FILE &&
 			git diff-index --name-only -z HEAD -- "$@" >"$TMP-stagenames" &&
-			git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
+			git update-index --ignore-skip-worktree-entries \
+				-z --add --remove --stdin <"$TMP-stagenames" &&
 			git write-tree &&
 			rm -f "$TMPindex"
 		) ) ||
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 392954d6dd..57258d5668 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1257,4 +1257,15 @@ test_expect_success 'stash apply should succeed with unmodified file' '
 	git stash apply
 '
 
+test_expect_success 'stash handles skip-worktree entries nicely' '
+	test_commit A &&
+	echo changed >A.t &&
+	git add A.t &&
+	git update-index --skip-worktree A.t &&
+	rm A.t &&
+	git stash &&
+
+	git rev-parse --verify refs/stash:A.t
+'
+
 test_done
-- 
gitgitgadget

      parent reply	other threads:[~2019-10-30 10:49 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-26  7:42 [PATCH 0/2] Fix git stash with skip-worktree entries Johannes Schindelin via GitGitGadget
2019-09-26  7:42 ` [PATCH 1/2] update-index: optionally leave skip-worktree entries alone Johannes Schindelin via GitGitGadget
2019-10-28  4:38   ` Junio C Hamano
2019-10-28 21:07     ` Johannes Schindelin
2019-10-29  2:27       ` Junio C Hamano
2019-09-26  7:42 ` [PATCH 2/2] stash: handle staged changes in skip-worktree files correctly Johannes Schindelin via GitGitGadget
2019-10-28  5:35   ` Junio C Hamano
2019-10-27 21:05 ` [PATCH 0/2] Fix git stash with skip-worktree entries Johannes Schindelin
2019-10-28  2:33   ` Junio C Hamano
2019-10-28 20:56     ` Johannes Schindelin
2019-10-29  2:25       ` Junio C Hamano
2019-10-29  8:15         ` Johannes Schindelin
2019-10-28 11:20 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2019-10-28 11:20   ` [PATCH v2 1/2] update-index: optionally leave skip-worktree entries alone Johannes Schindelin via GitGitGadget
2019-10-30  1:13     ` Junio C Hamano
2019-10-30  8:34       ` Johannes Schindelin
2019-11-02  3:04         ` Junio C Hamano
2019-11-02 23:03           ` Johannes Schindelin
2019-10-28 11:20   ` [PATCH v2 2/2] stash: handle staged changes in skip-worktree files correctly Johannes Schindelin via GitGitGadget
2019-10-30  1:16     ` Junio C Hamano
2019-10-30 10:49   ` [PATCH v3 0/2] Fix git stash with skip-worktree entries Johannes Schindelin via GitGitGadget
2019-10-30 10:49     ` [PATCH v3 1/2] update-index: optionally leave skip-worktree entries alone Johannes Schindelin via GitGitGadget
2019-10-30 10:49     ` Johannes Schindelin via GitGitGadget [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8f49a393e0ef7fff955a02963ff5229fb9885e89.1572432578.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).