git-style diffs broken in 2.5.1?

Matt Mackall mpm at selenic.com
Fri Apr 5 17:39:30 UTC 2013


On Fri, 2013-04-05 at 11:42 +1100, Peter Howard wrote:
> On Tue, 2013-04-02 at 12:08 +1100, Peter Howard wrote:
> > On Mon, 2013-04-01 at 19:55 -0500, Matt Mackall wrote:
> > > On Tue, 2013-04-02 at 11:44 +1100, Peter Howard wrote:
> > > > Hmmm, the mail is getting a bit long . . . what's the list policy WRT
> > > > trimming?
> > > 
> > > If you want me to remember who you are and what you're talking about,
> > > please don't trim relevant data.
> > > 
> > > > On Mon, 2013-04-01 at 19:29 -0500, Matt Mackall wrote:
> > > > > On Tue, 2013-04-02 at 11:23 +1100, Peter Howard wrote:
> > > > > > On Thu, 2013-03-28 at 08:24 -0700, Matt Mackall wrote:
> > > > > > > On Thu, 2013-03-28 at 10:42 +1100, Peter Howard wrote:
> > > > > > > > On Tue, 2013-03-19 at 00:03 -0500, Matt Mackall wrote:
> > > > > > > > > On Tue, 2013-03-19 at 09:21 +1100, Peter Howard wrote:
> > > > > > > > > > On Sun, 2013-03-17 at 15:09 -0500, Matt Mackall wrote:
> > > > > > > > > > > On Thu, 2013-03-14 at 09:31 +1100, Peter Howard wrote:
> > > > > > > > > > > > On Wed, 2013-03-13 at 14:50 -0700, Bryan O'Sullivan wrote:
> > > > > > > > > > > > > On Wed, Mar 13, 2013 at 2:43 PM, Peter Howard
> > > > > > > > > > > > > <pjh at northern-ridge.com.au> wrote:
> > > > > > > > > > > > >         The diff in question goes across a
> > > > > > > > > > > > >         merge but, as I said, I can't reproduce it simply.
> > > > > > > > > > > > >         
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Actually, if you can bisect the Mercurial repo to track down the
> > > > > > > > > > > > > changeset that breaks everything, that would be a big help. It
> > > > > > > > > > > > > shouldn't take more than a few minutes of work, and that might end up
> > > > > > > > > > > > > being enough to figure out the cause.
> > > > > > > > > > > > 
> > > > > > > > > > > > Ouch.
> > > > > > > > > > > > 
> > > > > > > > > > > > It happened much, much earlier than I noticed . . .
> > > > > > > > > > > > 
> > > > > > > > > > > > /wrk/mercurial-play/hg$ ./hg bisect -b
> > > > > > > > > > > > The first bad revision is:
> > > > > > > > > > > > changeset:   15775:91eb4512edd0
> > > > > > > > > > > > user:        Matt Mackall <mpm at selenic.com>
> > > > > > > > > > > > date:        Wed Jan 04 17:55:30 2012 -0600
> > > > > > > > > > > > summary:     copies: rewrite copy detection for non-merge users
> > > > > > > > > > > 
> > > > > > > > > > > Yes, that's a very likely point for this to have changed. Now what
> > > > > > > > > > > remains is to demonstrate that the new diff is _actually incorrect_: the
> > > > > > > > > > > old code was known to be frequently wrong.
> > > > > > > > > > > 
> > > > > > > > > > > So we're still back to either getting a repro or attempting to walk you
> > > > > > > > > > > through a diagnosis.
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Getting the actual repo: 0% chance :-(
> > > > > > > > > > Creating a demo repo: I suspect the odds are around 0.1% :-(
> > > > > > > > > > 
> > > > > > > > > > So that leaves the diagnosis.
> > > > > > > > > > 
> > > > > > > > > > > For the latter, I guess the first thing I'd like to see is, for the
> > > > > > > > > > > problematic diff:
> > > > > > > > > > 
> > > > > > > > > > I think I need a bit more information here - I've tried below and it's
> > > > > > > > > > not showing up much.  I'm treating "revision 1" as the tip, and
> > > > > > > > > > "revision 2" as the older revision.
> > > > > > > > > > 
> > > > > > > > 
> > > > > > > > OK, new set of data one my brain started working. Done using 2.5.1
> > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > - revision 1
> > > > > > > > > > > - revision 2
> > > > > > > > > > > - hg log -r "ancestor(r1, r2)" (revision 3)
> > > > > > > > > > 
> > > > > > > > > > revision 3 == revision 2.
> > > > > > > > 
> > > > > > > > Still true
> > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > ..so that we can establish the relevant topology. Then I'd like to see:
> > > > > > > > > > > 
> > > > > > > > > > > - hg manifest --debug -r r1 | grep oldfile
> > > > > > > > 
> > > > > > > > No mention.
> > > > > > > > 
> > > > > > > > > > >   hg manifest --debug -r r1 | grep newfile
> > > > > > > > 
> > > > > > > > 0a5463acd9fc59731c0f2188a354168ffe92765a 644   {path/to/newfile}
> > > > > > > > 
> > > > > > > > > > > - hg manifest --debug -r r2 | grep oldfile
> > > > > > > > 
> > > > > > > > e07f9b4a6b0240fb4dbe82e081ee28ac6402fb39 644   {path/to/oldfile}
> > > > > > > > 
> > > > > > > > > > >   hg manifest --debug -r r2 | grep newfile
> > > > > > > > 
> > > > > > > > No mention.
> > > > > > > > 
> > > > > > > > > > > - hg manifest --debug -r r3 | grep oldfile
> > > > > > > > > > >   hg manifest --debug -r r3 | grep newfile
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > 
> > > > > > > > Skipping as r3 == r2
> > > > > > > 
> > > > > > > Ok, that looks pretty sane, but perhaps backwards. To confirm, you're
> > > > > > > doing this, right?:
> > > > > > > 
> > > > > > >  hg diff -r r1 -r r2
> > > > > > 
> > > > > > I think you were assuming I was using r1 == older revision, r2 == newer
> > > > > > revision whereas I was doing the reverse.  So I'm really doing
> > > > > > 
> > > > > > hg diff -r -r r2 -r r1
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > What does this do?
> > > > > > > 
> > > > > > >  hg status -C --rev r1:r2
> > > > > > 
> > > > > > Gives a list of the files differing between the two diffs (including the
> > > > > > 'moved' file as a R (new name) and A (old name) )
> > > > > 
> > > > > I'd expect to see:
> > > > > 
> > > > > A newname
> > > > >   oldname   <- line immediately after each renamed file with the source
> > > > > ... 
> > > > > R oldname
> > > > > 
> > > > 
> > > > What I see is:
> > > > 
> > > > ...
> > > > A newname
> > > > A different file
> > > > R oldfile
> > > > 
> > > > > If you don't see that.. well, it may not have been recorded as a
> > > > > copy/rename. 
> > > > > 
> > > > > But there's a more direct way to check. Here's a copy early in the
> > > > > Mercurial history:
> > > > > 
> > > > > # find the revision where the file was introduced
> > > > > $ hg log -r 'first(file("**localrepo*"))'
> > > > > changeset:   1089:142b5d5ec9cc
> > > > > user:        mpm at selenic.com
> > > > > date:        Sat Aug 27 14:21:25 2005 -0700
> > > > > summary:     Break apart hg.py
> > > > > 
> > > > > # dump its rename metadata directly
> > > > > $ hg debugrename -r 1089 mercurial/localrepo.py
> > > > > mercurial/localrepo.py renamed from
> > > > > mercurial/hg.py:79bd2e10567756efea8c5a37c31369910f170772
> > > > > 
> > > > 
> > > > Leaving in as much real detail as I can:
> > > > 
> > > > peterh at citypig:$ hg log -r 'first(file("**newfile*"))'
> > > > changeset:   63493:f122808ab24e
> > > > user:        Peter Howard <peterh at ok-labs.com>
> > > > date:        Wed Mar 06 13:35:04 2013 +1100
> > > > summary:     Review changes
> > > > 
> > > > peterh at citypig:$ peterh at citypig:/wrk/okl4-detached-unit-test$ hg debugrename -r 63493 newfile
> > > > 
> > > > newfile renamed from oldfile:e96c1610b795e91bcbddc05a5f67f3255c29478e
> > > 
> > > Can you confirm this revision is an ancestor of the revision you're
> > > checking against?
> > > 
> > > $ hg log -r "63493 and ::the-newer-revision"
> > > 
> > > Also, that it's a descendant of the revision you're basing your diff on?
> > > 
> > > $ hg log -r "63493 and ::the-older-revision"
> > 
> > For both of the above, the result is the log for 63943.
> 
> Actually, for this case I'm reasonably sure it's _not_ a descendant of
> the base for the diff.  The sequence goes something like this:
> 
>       * Start with branch A
>       * Create new branch B off A
>       * B changes (including the rename in 63943) to B'
>       * A changes to A'
>       * B' merges in A' becoming B''
>       * B'' changes more to B'''
>       * A' changes to A''
>       * B''' merges in  A'' becoming B''''
>       * Then doing a diff of B'''' against A''
> 
> So, at this point, rev 63943 has never existed on the main branch A in
> any of its forms. 

Ok, that picture looks like this:

a----a1------a2
 \    \       \
  b-b1-b2--b3--b4
    ^
  rename

Our current copy detection code traces the history back to the common
ancestor of a2 and b4.. which is simply a2. Without the merge edge, it
would go back as far as a1, then back up to a2.. which may or may not be
sufficient. Please try this patch:

diff -r 565482e2ac6b mercurial/copies.py
--- a/mercurial/copies.py	Fri Apr 05 12:21:38 2013 -0500
+++ b/mercurial/copies.py	Fri Apr 05 12:38:56 2013 -0500
@@ -164,7 +164,11 @@
     '''find {dst at y: src at x} copy mapping for directed compare'''
     if x == y or not x or not y:
         return {}
-    a = y.ancestor(x)
+    r = y._repo
+    a = r[_findlimit(r, x.rev(), y.rev())]
+    a2 = y.ancestor(x)
+    if a != a2:
+        print "different", a.rev(), a2.rev()
     if a == x:
         return _forwardcopies(x, y)
     if a == y:

-- 
Mathematics is the supreme nostalgia of our time.





More information about the Mercurial mailing list