Re: [PATCH v2 1/2] check-ref-format --print: Normalize refnames that start with slashes

From: Jeff King <peff@peff.net>
Date: Aug 29, 2011, 21:27 UTC
Message-ID: <20110829212746.GA14299@sigill.intra.peff.net>
In-Reply-To: <7vhb50543g.fsf@alter.siamese.dyndns.org>

On Mon, Aug 29, 2011 at 01:56:35PM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > On Mon, Aug 29, 2011 at 02:50:12PM -0400, Jeff King wrote:
> >
> >>   3. Having better tool support for comparing two sets of commits. The
> >>      ideal interface (to me) for this workflow would be something like:
> >
> > BTW, this discussion is obviously about comparing what was applied
> > upstream with what was submitted, but I think such a tool could have
> > other purposes, too. It should be able to provide a much nicer interdiff
> > between two versions of the same series (e.g., for reviewers looking at
> > a re-roll).
> 
> I agree with it in principle, but I am not going to write one any time
> soon.

Heh. I didn't expect you to. :) I may make an attempt at something, but I'd be happy if somebody else has ideas, too.

Show 6 quoted lines

> I think the most difficult case is when the submitter based a fix on
> 'next' or 'pu' and I fixed it up so that it applies to 'maint'. It is not
> trivial to express conflict resolution in such a case, as what your tool
> has to do its work are (1) the original patch that may apply to
> "somewhere" (the tool could ask "am -3" to help it out), and (2) the patch
> that is queued somewhere in 'pu'.

The nastiest part of such a thing is not necessarily comparing the patches afterwards, but the actual conflicts you have to resolve to convince "am -3" to apply the patch on "master" in the first place.

Consider this toy example, which accidentally bases a topic off of "next" instead of "master":

  git init repo &&
  cd repo &&
  perl -le 'print "line $_" for (1..20)' >file &&
  git add file &&
  git commit -m base &&
  git checkout -b next &&
  perl -pi -e 's/10/$& unrelated topic on next/' file &&
  git commit -a -m 'unrelated topic' &&
  git checkout -b our-topic next && # oops!
  perl -pi -e 's/11/$& our topic/' file &&
  git commit -a -m 'our topic' &&
  git format-patch --stdout -1 >patch &&
  git checkout master

Obviously "git am" will fail to apply the patch. You can do it with "am -3", but get a conflict like:

  ++<<<<<<< HEAD
   +line 10
   +line 11
  ++=======
  + line 10 unrelated topic on next
  + line 11 our topic
  ++>>>>>>> our topic

which is obviously easy to resolve in this toy example, but harder in the real world (I assume in really hard cases, you just complain to the submitter to rebase it properly).

But assuming you resolve that properly to:

  line 10
  line 11 our topic

what does the original submitter see? If they try to rebase their topic onto master, they'll get this pretty sane conflict:

  ++<<<<<<< HEAD
   +line 10
  ++=======
  + line 10 unrelated topic on next
  ++>>>>>>> our topic

I think the tool behavior I outlined in the previous message would yield:

  diff --git a/file b/file
  --- a/file
  +++ b/file
   line 8
   line 9
  -line 10 unrelated topic on next
  +line 10
   line 11 our topic
   line 12
  Accept this change [y,n,q]?

That is, it would look at the final versions of what each patch produces, and try to apply the differences from one to the other, and then use the result to apply on top of our rebase-in-progress.

So really, I think the ugliness of what the submitter would see is no uglier than what you had to do during "git am". If you make trivial changes, they'll see trivial changes. If you had to pick apart large chunks of code into their constituent topics, then that's what the submitter will see when reviewing the differences.

But note that I didn't actually run the above "this is what my tool should do" through an actual script. I haven't written anything yet, so I am just guessing, and there may be more complex cases where it breaks down.

-Peff