git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 15, 2015, 17:42 UTC
Message-ID
<xmqqsi9samx1.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20150610192734.GA23715@peff.net>
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> 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.

Previous: bärNext: Jeff King
Message 9 of 16 in “Re: [PATCH 3/3] stash: require a clean index to apply”
  1. Jonathan KamensJun 5, 2015
  2. Jeff KingJun 7, 2015
  3. Jeff KingJun 7, 2015
  4. bärJun 10, 2015
  5. Jeff KingJun 10, 2015
  6. Junio C HamanoJun 10, 2015
  7. Jeff KingJun 10, 2015
  8. bärJun 10, 2015
  9. Junio C HamanoJun 15, 2015
  10. Revert "stash: require a clean index to apply"Jeff King, Jun 15, 2015
  11. Junio C HamanoJun 15, 2015
  12. Jonathan KamensJun 25, 2015
  13. Jeff KingJun 26, 2015
  14. Jonathan KamensJun 26, 2015
  15. Jeff KingJun 26, 2015
  16. Junio C HamanoJun 26, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.