Re: [PATCH 3/3] stash: require a clean index to apply

From: Jeff King <peff@peff.net>
Date: Jun 10, 2015, 19:27 UTC
Message-ID: <20150610192734.GA23715@peff.net>
In-Reply-To: <xmqq616vl6gm.fsf@gitster.dls.corp.google.com>

On Wed, Jun 10, 2015 at 12:16:25PM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > So I am trying to figure out what the use case here is. Clearly the
> > above is a toy case, but why is "stash -k" followed by a quick pop
> > useful in general? Certainly I use "stash" (without "-k") and a quick
> > pop all the time, and I think that is what stash was designed for.
> >
> > The best use case I can think of is Jonathan's original: to see only the
> > staged content in the working tree, and then restore the original state.
> > But stash does not currently work very well for that, as shown above.
> 
> The canonical use case for "stash -k" is to see only the content to
> be committed (for testing), commit it after testing and then pop on
> top of the committed result, which is the same as what you saw in
> the working tree and the index when you did "stash -k".  I do not
> think "stash -k && stash pop" was in the design parameter when "-k"
> was added (as you demonstrated, it would not fundamentally work
> reliably depending on the differences between HEAD-Index-Worktree).

It seems like applying a stash made with "-k" is fundamentally misdesigned in the current code. We would want to apply to the working tree the difference between the index and the working tree, but instead we try to apply the difference between the HEAD and the working tree. Which is nonsensical for this use case (i.e., to apply the diff between $stash and $stash^2, not $stash^1).

I don't think there is any way to tell that "-k" was used, though. But even if the user knew that, I do not think there is any option to tell "stash apply" to do it this way.

I dunno. With respect to the original patch, I am OK if we just want to revert it. This area of stash seems a bit under-designed IMHO, but if people were happy enough with it before, I do not think the safety benefit from ed178ef is that great (it is not saving you from destroying working tree content, only the index state; the individual content blobs are still available from git-fsck).

-Peff