git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@github.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Brian Lopez <brian@github.com>
Subject: Re: [RFC/PATCH 2/2] stash: drop dirty worktree check on apply
Date: Tue, 5 Apr 2011 19:02:45 -0400	[thread overview]
Message-ID: <20110405230245.GA7631@sigill.intra.peff.net> (raw)
In-Reply-To: <20110405225038.GB7358@sigill.intra.peff.net>

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

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

  reply	other threads:[~2011-04-05 23:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-05 21:20 [PATCH 1/2] stash: fix accidental apply of non-existent stashes Jeff King
2011-04-05 21:23 ` [RFC/PATCH 2/2] stash: drop dirty worktree check on apply Jeff King
2011-04-05 21:59   ` Junio C Hamano
2011-04-05 22:18     ` Jeff King
2011-04-05 22:50       ` Jeff King
2011-04-05 23:02         ` Jeff King [this message]
2011-04-05 23:17           ` Junio C Hamano
2011-04-06 23:01             ` Junio C Hamano
2011-04-05 23:23 ` [PATCH 1/2] stash: fix accidental apply of non-existent stashes Jon Seymour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110405230245.GA7631@sigill.intra.peff.net \
    --to=peff@github.com \
    --cc=brian@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).