git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] stash: require a clean index to apply
@ 2015-06-05  0:43 Jonathan Kamens
  2015-06-07 12:40 ` Jeff King
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Kamens @ 2015-06-05  0:43 UTC (permalink / raw)
  To: git

I'm writing about the patch that Jeff King submitted on April 22, in 
<20150422193101.GC27945@peff.net>, in particular, 
https://github.com/git/git/commit/ed178ef13a26136d86ff4e33bb7b1afb5033f908 
. It appears that this patch was included in git 2.4.2, and it breaks my 
workflow.

In particular, I have a pre-commit hook whith does the following:

1. Stash unstaged changes ("git stash -k").
2. Run flake8 over all staged changes.
3. If flake8 complains, then error out of the commit.
4. Otherwise, apply the stash and exit.

This way I am prevented from committing staged changes that don't pass 
flake8. I can't imagine that this is a terribly uncommon workflow.

This worked fine until the aforementioned comment, after which my hook 
complains, "Cannot apply stash: Your index contains uncommitted changes."

The reason I have to do things this way is as follows. Suppose I did the 
following:

1. Stage changes that have a flake8 violation.
2. Fix the flake8 violation in the unstaged version of the staged file.
3. Commit the previously staged changes.

If my commit hook runs over the unstaged version of the file, then it 
won't detect the flake8 violation, and as a result the violation will be 
committed.

If anyone has a suggestion for how I can achieve the desired goal within 
the constraints of the 2.4.2 version of git-stash.sh, I'd love to hear 
it. Otherwise, I'd like to ask for this patch to be reconsidered.

Thank you,

Jonathan Kamens

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: How do I resolve conflict after popping stash without adding the file to index?
@ 2015-04-22 19:29 Jeff King
  2015-04-22 19:31 ` [PATCH 3/3] stash: require a clean index to apply Jeff King
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff King @ 2015-04-22 19:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dmitry Gutov, git

On Wed, Apr 22, 2015 at 02:35:40PM -0400, Jeff King wrote:

> Ironically, the message before e0e2a9c actually recommends staging
> changes before applying the stash, which would lead to this exact
> situation! So I think the most trivial patch is:
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index d4cf818..f1865c9 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -442,6 +442,7 @@ apply_stash () {
>  	assert_stash_like "$@"
>  
>  	git update-index -q --refresh || die "$(gettext "unable to refresh index")"
> +	git diff-index --cached HEAD || die "dirty index; cannot apply stash"
>  
>  	# current index state
>  	c_tree=$(git write-tree) ||
> 
> but it makes me wonder if somebody would find it annoying that they
> cannot apply a stash into their work-in-progress (i.e., it _might_ cause
> annoyance, but most of the time it will be convenient to do so).

It does actually fail a test in t3903, but I think that test just
incidentally had a dirty index, and didn't care about that particular
feature.

> We also have require_clean_work_tree() in git-sh-setup.sh. We definitely
> don't want the first half of that, but we do want the diff-index check.
> So probably we'd want to refactor that into two separate functions, and
> only call the require_clean_index part.

This turned out to be more work than it was worth. Most of the effort
in that function is about adjusting the messages to handle the cases
when either or both of the working tree and index are dirty. I did pick
up the useful bits from there, though:

  - use --quiet to suppress output and so that the exit code actually
    matters

  - use "--" to disambiguate the ref

  - I didn't pick up the `rev-parse HEAD` call. I don't think it's
    necessary (i.e., diff-index should barf for us if it can't read
    HEAD).

Here are the patches.

  [1/3]: t3903: stop hard-coding commit sha1s
  [2/3]: t3903: avoid applying onto dirty index
  [3/3]: stash: require a clean index to apply

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-06-26  4:16 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-05  0:43 [PATCH 3/3] stash: require a clean index to apply Jonathan Kamens
2015-06-07 12:40 ` Jeff King
2015-06-07 12:47   ` Jeff King
2015-06-10 18:19   ` bär
2015-06-10 18:56     ` Jeff King
2015-06-10 19:16       ` Junio C Hamano
2015-06-10 19:27         ` Jeff King
2015-06-10 21:54           ` bär
2015-06-15 17:42           ` Junio C Hamano
2015-06-15 18:27             ` [PATCH] Revert "stash: require a clean index to apply" Jeff King
2015-06-15 20:11               ` Junio C Hamano
2015-06-25 21:51                 ` Jonathan Kamens
     [not found]                 ` <f06e573d-02e3-47e9-85d8-3bb6551d72f5.maildroid@localhost>
2015-06-26  0:27                   ` Jeff King
2015-06-26  1:12                     ` Jonathan Kamens
2015-06-26  4:03                       ` Jeff King
2015-06-26  4:15                         ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2015-04-22 19:29 How do I resolve conflict after popping stash without adding the file to index? Jeff King
2015-04-22 19:31 ` [PATCH 3/3] stash: require a clean index to apply Jeff King

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).