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>
Subject: [PATCH v2 0/2] Fix git stash with skip-worktree entries
Date: Mon, 28 Oct 2019 11:20:13 +0000	[thread overview]
Message-ID: <pull.355.v2.git.1572261615.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.355.git.gitgitgadget@gmail.com>

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.

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   | 14 ++++++++++++++
 6 files changed, 41 insertions(+), 4 deletions(-)


base-commit: 34933d0eff5d4c91fae6ad6f71a6e6a69a496ced
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-355%2Fdscho%2Ffix-stash-with-skip-worktree-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-355/dscho/fix-stash-with-skip-worktree-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/355

Range-diff vs v1:

 1:  c263eb54b3 ! 1:  86dbb11f15 update-index: optionally leave skip-worktree entries alone
     @@ -72,3 +72,28 @@
       		OPT_SET_INT(0, "info-only", &info_only,
       			N_("add to index only; do not add content to object database"), 1),
       		OPT_SET_INT(0, "force-remove", &force_remove,
     +
     + diff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh
     + --- a/t/t7012-skip-worktree-writing.sh
     + +++ b/t/t7012-skip-worktree-writing.sh
     +@@
     + 	test_i18ncmp expected result
     + '
     + 
     ++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 &&
     ++
     ++	: ignoring the worktree &&
     ++	git update-index --remove --ignore-skip-worktree-entries geroff-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
     ++'
     ++
     + #TODO test_expect_failure 'git-apply adds file' false
     + #TODO test_expect_failure 'git-apply updates file' false
     + #TODO test_expect_failure 'git-apply removes file' false
 2:  4c684be179 ! 2:  9835e66399 stash: handle staged changes in skip-worktree files correctly
     @@ -59,7 +59,7 @@
       --- a/t/t3903-stash.sh
       +++ b/t/t3903-stash.sh
      @@
     - 	test_path_is_missing to-remove
     + 	git stash apply
       '
       
      +test_expect_success 'stash handles skip-worktree entries nicely' '

-- 
gitgitgadget

  parent reply	other threads:[~2019-10-28 11:20 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 ` Johannes Schindelin via GitGitGadget [this message]
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     ` [PATCH v3 2/2] stash: handle staged changes in skip-worktree files correctly Johannes Schindelin via GitGitGadget

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=pull.355.v2.git.1572261615.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).