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

From: Junio C Hamano <[email-masked]>
Date: Mon, 15 Jun 2015 10:42:18 -0700

Jeff King <[email-masked]> writes:

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

Yeah, I agree.   Somebody care to do the log message?

This is a tangent, but I'd actually not just agree with "'stash -k'
is a bit under-designed" but also would say something stronger than
that, IMHO ;-)

The very original idea of "git stash" to ssave away working tree
changes with respect to HEAD without bothering the index was at
least internally consistent.  You save away and then may later pop
the change on top of a different commit after dealing with an
emergency, at which time there may be conflicts that you would need
to resolve, and because it is only between HEAD and working tree,
you can safely use the index to resolve the conflicts just like in
any other situation when you help Git to resolve them.  In that
context, the flaw ed178ef1 (stash: require a clean index to apply,
2015-04-22) tried to correct made a lot of sense.

What broke the entire system was the throwing the "save the index,
too" into the mix, which was done without careful thinking.  While
it made the command more useful (when it works), it made the command
internally inconsistent---the command has to punt restoring index
state when it can't, for example, losing the state it tried to save.
I think that is where the "under-designed" started, I would think;
of course, "-k" needed to build on top of "save also the index",
but that is not the primary culprit.