git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Nanako Shiraishi <nanako3@lavabit.com>, Brian Lopez <brian@github.com>
Subject: [RFC/PATCH 2/2] stash: drop dirty worktree check on apply
Date: Tue, 5 Apr 2011 17:23:15 -0400	[thread overview]
Message-ID: <20110405212314.GA3613@sigill.intra.peff.net> (raw)
In-Reply-To: <20110405212025.GA3579@sigill.intra.peff.net>

Before we apply a stash, we make sure there are no changes
in the worktree that are not in the index. This check dates
back to the original git-stash.sh, and is presumably
intended to prevent changes in the working tree from being
accidentally lost during the merge.

However, this check has two problems:

  1. It is overly restrictive. If my stash changes only file
     "foo", but "bar" is dirty in the working tree, it will
     prevent us from applying the stash.

  2. It is redundant. We don't touch the working tree at all
     until we actually call merge-recursive. But it has its
     own (much more accurate) checks to avoid losing working
     tree data, and will abort the merge with a nicer
     message telling us which paths were problems.

So we can simply drop the check entirely.

Signed-off-by: Jeff King <peff@peff.net>
---
As with any time we loosen a safety valve, I am worried that I'm missing
a case where it is still doing something useful. Thus the RFC tag on
this patch.

I'm not sure if the check was perhaps even required when git-stash was
written, and has simply since become useless as merge-recursive became
more careful.

 git-stash.sh     |    4 +---
 t/t3903-stash.sh |   16 ++++++++++++++--
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index a5b1dc3..07ac323 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -334,9 +334,7 @@ apply_stash () {
 
 	assert_stash_like "$@"
 
-	git update-index -q --refresh &&
-	git diff-files --quiet --ignore-submodules ||
-		die 'Cannot apply to a dirty working tree, please stage your changes'
+	git update-index -q --refresh || die 'unable to refresh index'
 
 	# current index state
 	c_tree=$(git write-tree) ||
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 11077f0..966bc46 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -43,14 +43,26 @@ test_expect_success 'applying bogus stash does nothing' '
 	test_cmp expect file
 '
 
-test_expect_success 'apply needs clean working directory' '
+test_expect_success 'apply does not need clean working directory' '
 	echo 4 > other-file &&
 	git add other-file &&
 	echo 5 > other-file &&
-	test_must_fail git stash apply
+	git stash apply &&
+	echo 3 >expect &&
+	test_cmp expect file
+'
+
+test_expect_success 'apply does not clobber working directory changes' '
+	git reset --hard &&
+	echo 4 >file &&
+	test_must_fail git stash apply &&
+	echo 4 >expect &&
+	test_cmp expect file
 '
 
 test_expect_success 'apply stashed changes' '
+	git reset --hard &&
+	echo 5 >other-file &&
 	git add other-file &&
 	test_tick &&
 	git commit -m other-file &&
-- 
1.7.4.3.13.g0b769.dirty

  reply	other threads:[~2011-04-05 21:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-05 21:20 [PATCH 1/2] stash: fix accidental apply of non-existent stashes Jeff King
2011-04-05 21:23 ` Jeff King [this message]
2011-04-05 21:59   ` [RFC/PATCH 2/2] stash: drop dirty worktree check on apply Junio C Hamano
2011-04-05 22:18     ` Jeff King
2011-04-05 22:50       ` Jeff King
2011-04-05 23:02         ` Jeff King
2011-04-05 23:17           ` Junio C Hamano
2011-04-06 23:01             ` Junio C Hamano
2011-04-05 23:23 ` [PATCH 1/2] stash: fix accidental apply of non-existent stashes Jon Seymour

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20110405212314.GA3613@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=brian@github.com \
    --cc=git@vger.kernel.org \
    --cc=nanako3@lavabit.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).