Patchwork log: fix log -f slow path to actually follow history

login
register
mail settings
Submitter Durham Goode
Date Dec. 5, 2014, 10:54 p.m.
Message ID <96d916622019f448bd01.1417820070@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/7015/
State Superseded
Headers show

Comments

Durham Goode - Dec. 5, 2014, 10:54 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1417818452 28800
#      Fri Dec 05 14:27:32 2014 -0800
# Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
# Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
log: fix log -f slow path to actually follow history

log -f was using ':.' instead of '::.' so it wasn't actually following history.
This fixes it and adds a test. It's been broken for years, so I guess it wasn't
that big of a deal...
Pierre-Yves David - Dec. 5, 2014, 10:56 p.m.
On 12/05/2014 02:54 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1417818452 28800
> #      Fri Dec 05 14:27:32 2014 -0800
> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
> log: fix log -f slow path to actually follow history
>
> log -f was using ':.' instead of '::.' so it wasn't actually following history.
> This fixes it and adds a test. It's been broken for years, so I guess it wasn't
> that big of a deal...

good catch.

Does this open us to linkrev related bug when using it on a file?
Durham Goode - Dec. 5, 2014, 11:19 p.m.
On 12/5/14, 2:56 PM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org>
wrote:

>
>
>On 12/05/2014 02:54 PM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1417818452 28800
>> #      Fri Dec 05 14:27:32 2014 -0800
>> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
>> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
>> log: fix log -f slow path to actually follow history
>>
>> log -f was using ':.' instead of '::.' so it wasn't actually following
>>history.
>> This fixes it and adds a test. It's been broken for years, so I guess
>>it wasn't
>> that big of a deal...
>
>good catch.
>
>Does this open us to linkrev related bug when using it on a file?

Good question.  I updated the test to check for that, and it doesn¹t seem
to affect file log traversals when the linkrev commit is not an ancestor.
I¹ll resend with the new test.
Durham Goode - Dec. 5, 2014, 11:22 p.m.
On 12/5/14 3:19 PM, Durham Goode wrote:
>
> On 12/5/14, 2:56 PM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org>
> wrote:
>
>>
>> On 12/05/2014 02:54 PM, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1417818452 28800
>>> #      Fri Dec 05 14:27:32 2014 -0800
>>> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
>>> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
>>> log: fix log -f slow path to actually follow history
>>>
>>> log -f was using ':.' instead of '::.' so it wasn't actually following
>>> history.
>>> This fixes it and adds a test. It's been broken for years, so I guess
>>> it wasn't
>>> that big of a deal...
>> good catch.
>>
>> Does this open us to linkrev related bug when using it on a file?
> Good question.  I updated the test to check for that, and it doesn¹t seem
> to affect file log traversals when the linkrev commit is not an ancestor.
> I¹ll resend with the new test.
>
>
Actually, I lied.  It does affect it :(  My test was just wrong. Sigh.  
K I'll figure out a fix and resend.
Siddharth Agarwal - Dec. 5, 2014, 11:26 p.m.
On 12/05/2014 02:54 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1417818452 28800
> #      Fri Dec 05 14:27:32 2014 -0800
> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
> log: fix log -f slow path to actually follow history
>
> log -f was using ':.' instead of '::.' so it wasn't actually following history.
> This fixes it and adds a test. It's been broken for years, so I guess it wasn't
> that big of a deal...

Still a bugfix, so should be on stable?

>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1836,7 +1836,7 @@ def getgraphlogrevs(repo, pats, opts):
>           possiblyunsorted = True
>       else:
>           if follow and len(repo) > 0:
> -            revs = repo.revs('reverse(:.)')
> +            revs = repo.revs('reverse(::.)')
>           else:
>               revs = revset.spanset(repo)
>               revs.reverse()
> @@ -1880,7 +1880,7 @@ def getlogrevs(repo, pats, opts):
>       if opts.get('rev'):
>           revs = scmutil.revrange(repo, opts['rev'])
>       elif follow:
> -        revs = repo.revs('reverse(:.)')
> +        revs = repo.revs('reverse(::.)')
>       else:
>           revs = revset.spanset(repo)
>           revs.reverse()
> diff --git a/tests/test-log.t b/tests/test-log.t
> --- a/tests/test-log.t
> +++ b/tests/test-log.t
> @@ -1543,3 +1543,19 @@ issue3772: hg log -r :null showing revis
>     
>   
>     $ cd ..
> +
> +hg log -f dir across branches
> +
> +  $ hg init acrossbranches
> +  $ cd acrossbranches
> +  $ mkdir d
> +  $ echo a > d/a && hg ci -Aqm a
> +  $ echo b > d/b && hg ci -Aqm b
> +  $ hg up -q 0
> +  $ echo c > d/c && hg ci -Aqm c
> +  $ hg log -f d -T '{desc}' -G
> +  @  c
> +  |
> +  o  a
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Durham Goode - Dec. 6, 2014, 12:05 a.m.
On 12/5/14 3:22 PM, Durham Goode wrote:
>
> On 12/5/14 3:19 PM, Durham Goode wrote:
>>
>> On 12/5/14, 2:56 PM, "Pierre-Yves David" 
>> <pierre-yves.david@ens-lyon.org>
>> wrote:
>>
>>>
>>> On 12/05/2014 02:54 PM, Durham Goode wrote:
>>>> # HG changeset patch
>>>> # User Durham Goode <durham@fb.com>
>>>> # Date 1417818452 28800
>>>> #      Fri Dec 05 14:27:32 2014 -0800
>>>> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
>>>> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
>>>> log: fix log -f slow path to actually follow history
>>>>
>>>> log -f was using ':.' instead of '::.' so it wasn't actually following
>>>> history.
>>>> This fixes it and adds a test. It's been broken for years, so I guess
>>>> it wasn't
>>>> that big of a deal...
>>> good catch.
>>>
>>> Does this open us to linkrev related bug when using it on a file?
>> Good question.  I updated the test to check for that, and it doesn¹t 
>> seem
>> to affect file log traversals when the linkrev commit is not an 
>> ancestor.
>> I¹ll resend with the new test.
>>
>>
> Actually, I lied.  It does affect it :(  My test was just wrong. 
> Sigh.  K I'll figure out a fix and resend.
Good news everyone! hg log -f <file> was broken for linkrevs even 
without my change!  So I'll send my patch out with a better test and 
will log a bug about log -f <file> being broken, since it will require a 
more indepth fix around linkrevs.
Durham Goode - Dec. 6, 2014, 12:09 a.m.
On 12/5/14 4:05 PM, Durham Goode wrote:
>
> On 12/5/14 3:22 PM, Durham Goode wrote:
>>
>> On 12/5/14 3:19 PM, Durham Goode wrote:
>>>
>>> On 12/5/14, 2:56 PM, "Pierre-Yves David" 
>>> <pierre-yves.david@ens-lyon.org>
>>> wrote:
>>>
>>>>
>>>> On 12/05/2014 02:54 PM, Durham Goode wrote:
>>>>> # HG changeset patch
>>>>> # User Durham Goode <durham@fb.com>
>>>>> # Date 1417818452 28800
>>>>> #      Fri Dec 05 14:27:32 2014 -0800
>>>>> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
>>>>> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
>>>>> log: fix log -f slow path to actually follow history
>>>>>
>>>>> log -f was using ':.' instead of '::.' so it wasn't actually 
>>>>> following
>>>>> history.
>>>>> This fixes it and adds a test. It's been broken for years, so I guess
>>>>> it wasn't
>>>>> that big of a deal...
>>>> good catch.
>>>>
>>>> Does this open us to linkrev related bug when using it on a file?
>>> Good question.  I updated the test to check for that, and it doesn¹t 
>>> seem
>>> to affect file log traversals when the linkrev commit is not an 
>>> ancestor.
>>> I¹ll resend with the new test.
>>>
>>>
>> Actually, I lied.  It does affect it :(  My test was just wrong. 
>> Sigh.  K I'll figure out a fix and resend.
> Good news everyone! hg log -f <file> was broken for linkrevs even 
> without my change!  So I'll send my patch out with a better test and 
> will log a bug about log -f <file> being broken, since it will require 
> a more indepth fix around linkrevs.
And once again I'm proven wrong because my test was wrong.  Ok, I'm 
going to stop spamming the list until I have it done.
Matt Mackall - Dec. 8, 2014, 6:03 p.m.
On Fri, 2014-12-05 at 14:54 -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1417818452 28800
> #      Fri Dec 05 14:27:32 2014 -0800
> # Node ID 96d916622019f448bd012884ee4fd78fe8dc63ba
> # Parent  406dfc63a1ad71213dccc1a45de99a3c5d5ac460
> log: fix log -f slow path to actually follow history
> 
> log -f was using ':.' instead of '::.' so it wasn't actually following history.
> This fixes it and adds a test. It's been broken for years, so I guess it wasn't
> that big of a deal...

Actually, log wasn't able to use revsets until we made them lazy, so
this bug is still newish.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1836,7 +1836,7 @@  def getgraphlogrevs(repo, pats, opts):
         possiblyunsorted = True
     else:
         if follow and len(repo) > 0:
-            revs = repo.revs('reverse(:.)')
+            revs = repo.revs('reverse(::.)')
         else:
             revs = revset.spanset(repo)
             revs.reverse()
@@ -1880,7 +1880,7 @@  def getlogrevs(repo, pats, opts):
     if opts.get('rev'):
         revs = scmutil.revrange(repo, opts['rev'])
     elif follow:
-        revs = repo.revs('reverse(:.)')
+        revs = repo.revs('reverse(::.)')
     else:
         revs = revset.spanset(repo)
         revs.reverse()
diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -1543,3 +1543,19 @@  issue3772: hg log -r :null showing revis
   
 
   $ cd ..
+
+hg log -f dir across branches
+
+  $ hg init acrossbranches
+  $ cd acrossbranches
+  $ mkdir d
+  $ echo a > d/a && hg ci -Aqm a
+  $ echo b > d/b && hg ci -Aqm b
+  $ hg up -q 0
+  $ echo c > d/c && hg ci -Aqm c
+  $ hg log -f d -T '{desc}' -G
+  @  c
+  |
+  o  a
+  
+  $ cd ..