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

From: Jeff King <peff@github.com>
Date: Apr 5, 2011, 23:02 UTC
Message-ID: <20110405230245.GA7631@sigill.intra.peff.net>
In-Reply-To: <20110405225038.GB7358@sigill.intra.peff.net>

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

Show 16 quoted lines

> > > 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 ...
> [...]
> > 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