Re: [RFC/PATCH 2/2] stash: drop dirty worktree check on apply

On Tue, Apr 05, 2011 at 06:50:38PM -0400, Jeff King wrote:

quoted

quoted

I _think_ the reason we originally insisted on clean working tree was that
while merge-resolve has always had an acurate check, merge-recursive's
check was not very good, especially when renames are involved.  So
probably this part of your comment ...

[...]

quoted

That makes sense to me. I'd be a lot more comfortable if I could find
the actual place where merge-recursive got more accurate. I'll see if
it's simple to bisect.

Hmm, no such luck. In v1.5.0, before stash even existed, "git merge"
will properly fail on this case (though the error message isn't as
pretty):
[...]
which leads me to believe there is a more complex case that
merge-recursive wasn't handling at the time, and which may or may not be
handled better today. What that case would be, I have no clue.

Hmm. I think that code is due to your comment on the original git-stash
(then "git-save") here:

  http://article.gmane.org/gmane.comp.version-control.git/50749

  > +function restore_save () {
  > +     save=$(git rev-parse --verify --default saved "$1")
  > +     h_tree=$(git rev-parse --verify $save:base)
  > +     i_tree=$(git rev-parse --verify $save:indx)
  > +     w_tree=$(git rev-parse --verify $save:work)
  > +
  > +     git-merge-recursive $h_tree -- HEAD^{tree} $w_tree
  > +}

  The same "robustness" comments for the save_work function apply
  here.  You probably do not want to restore on a dirty tree; the
  intended use case is "stash away, pull, then restore", so I
  think it is Ok to assume that you will only be restoring on a
  clean state (and it would make the implementation simpler).

So perhaps there is no broken case at all, and it was just a matter of
being overly conservative from the beginning.

-Peff