[PATCH 2 of 2 STABLE] log: copy the way of ancestor traversal to --follow matcher (issue5376)

Jun Wu quark at fb.com
Sat Sep 24 13:59:45 EDT 2016


Thanks for the clean fix! I was trying a different approach but cannot get
it right easily. These patches should be the correct fix. Marked
Pre-Reviewed.

A tricky case could be when multiple files are followed, for example:

  $ hg init repo
  $ cd repo
  $ for i in 1 2; do
  >   echo $i >> a
  >   hg commit -A a -m $i
  > done
  $ hg update 0 -q
  $ echo 2 >> a
  $ echo 1 > b
  $ hg commit -A b -A a -m '2 with b' -q
  $ hg update 1 -q
  $ echo 1 > b
  $ echo 1 > a
  $ hg commit -A b a -m 'b'
  $ hg merge 2 -q
  $ echo 1 > b
  $ hg commit -m merge
  $ hg log -G -p -T '{rev}:{node|short} {desc}'        # more patches
  $ hg log -G -p -T '{rev}:{node|short} {desc}' -f a b # less patches

At first sight, it seemed wrong to output less patches for the second
command. But after a second thought, I believe showing less patches is the
sane and desired behavior.

- off topic -

By the way, I just encountered an _adjustlinkrev related perf issue when
working on fastannotate internally. introrev (_adjustlinkrev) can take up
seconds (and sadly it just returns ctx.linerev() most of the time). Seconds
is unacceptable comparing with the annotate logic which only takes tens of
milliseconds.

I think a stupid way to make it better is to have a map of {fctxnode:
[linknode]}. To make the map small, it only stores entries where
len([linknode]) > 1. The challenge would be how to keep the map up-to-date.


Excerpts from Yuya Nishihara's message of 2016-09-24 23:00:38 +0900:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1474714703 -32400
> #      Sat Sep 24 19:58:23 2016 +0900
> # Branch stable
> # Node ID 4237fd4f828807a9f8d711625da00b24a2ca9461
> # Parent  bf616dd17b4244d4d5fc92b0ada65404596fe4b2
> # EXP-Topic followmatcher
> log: copy the way of ancestor traversal to --follow matcher (issue5376)
> 
> We can't use fctx.linkrev() because follow() revset tries hard to simulate
> the traversal of changelog DAG, not filelog DAG. This patch fixes
> _makefollowlogfilematcher() to walk file ancestors in the same way as
> revset._follow().
> 
> I'll factor out a common function in future patches.
> 
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1940,7 +1940,7 @@ def _makefollowlogfilematcher(repo, file
>      # --follow, we want the names of the ancestors of FILE in the
>      # revision, stored in "fcache". "fcache" is populated by
>      # reproducing the graph traversal already done by --follow revset
> -    # and relating linkrevs to file names (which is not "correct" but
> +    # and relating revs to file names (which is not "correct" but
>      # good enough).
>      fcache = {}
>      fcacheready = [False]
> @@ -1949,9 +1949,9 @@ def _makefollowlogfilematcher(repo, file
>      def populate():
>          for fn in files:
>              fctx = pctx[fn]
> -            fcache.setdefault(fctx.linkrev(), set()).add(fctx.path())
> +            fcache.setdefault(fctx.introrev(), set()).add(fctx.path())
>              for c in fctx.ancestors(followfirst=followfirst):
> -                fcache.setdefault(c.linkrev(), set()).add(c.path())
> +                fcache.setdefault(c.rev(), set()).add(c.path())
>  
>      def filematcher(rev):
>          if not fcacheready[0]:
> diff --git a/tests/test-log.t b/tests/test-log.t
> --- a/tests/test-log.t
> +++ b/tests/test-log.t
> @@ -920,6 +920,78 @@ log -r tip --stat
>  
>    $ cd ..
>  
> +log --follow --patch FILE in repository where linkrev isn't trustworthy
> +(issue5376)
> +
> +  $ hg init follow-dup
> +  $ cd follow-dup
> +  $ cat <<EOF >> .hg/hgrc
> +  > [ui]
> +  > logtemplate = '=== {rev}: {desc}\n'
> +  > [diff]
> +  > nodates = True
> +  > EOF
> +  $ echo 0 >> a
> +  $ hg ci -qAm 'a0'
> +  $ echo 1 >> a
> +  $ hg ci -m 'a1'
> +  $ hg up -q 0
> +  $ echo 1 >> a
> +  $ touch b
> +  $ hg ci -qAm 'a1 with b'
> +  $ echo 3 >> a
> +  $ hg ci -m 'a3'
> +
> + fctx.rev() == 2, but fctx.linkrev() == 1
> +
> +  $ hg log -pf a
> +  === 3: a3
> +  diff -r 4ea02ba94d66 -r e7a6331a34f0 a
> +  --- a/a
> +  +++ b/a
> +  @@ -1,2 +1,3 @@
> +   0
> +   1
> +  +3
> +  
> +  === 2: a1 with b
> +  diff -r 49b5e81287e2 -r 4ea02ba94d66 a
> +  --- a/a
> +  +++ b/a
> +  @@ -1,1 +1,2 @@
> +   0
> +  +1
> +  
> +  === 0: a0
> +  diff -r 000000000000 -r 49b5e81287e2 a
> +  --- /dev/null
> +  +++ b/a
> +  @@ -0,0 +1,1 @@
> +  +0
> +  
> +
> + fctx.introrev() == 2, but fctx.linkrev() == 1
> +
> +  $ hg up -q 2
> +  $ hg log -pf a
> +  === 2: a1 with b
> +  diff -r 49b5e81287e2 -r 4ea02ba94d66 a
> +  --- a/a
> +  +++ b/a
> +  @@ -1,1 +1,2 @@
> +   0
> +  +1
> +  
> +  === 0: a0
> +  diff -r 000000000000 -r 49b5e81287e2 a
> +  --- /dev/null
> +  +++ b/a
> +  @@ -0,0 +1,1 @@
> +  +0
> +  
> +
> +  $ cd ..
> +
>  Test that log should respect the order of -rREV even if multiple OR conditions
>  are specified (issue5100):
>  


More information about the Mercurial-devel mailing list