Patchwork [V2] cvsps: fix computation of parent revisions when log caching is on

On Fri, Oct 09, 2015 at 03:29:35PM +0300, Emanuele Giaquinta wrote:
> # HG changeset patch
> # User Emanuele Giaquinta <emanuele.giaquinta@gmail.com>
> # Date 1444206832 -10800
> #      Wed Oct 07 11:33:52 2015 +0300
> # Node ID 724877229b9a3b513c0f325bb362626d6ed46210
> # Parent  a024e2db4553492e173032f52464e2c4efe0d4fa
> cvsps: fix computation of parent revisions when log caching is on

With this applied I'm seeing consistent failures of test-convert-cvs.t like this:





>
> cvsps computes the parent revisions of log entries by walking the cvs log
> sorted by (rcs, revision) and by iteratively maintaining a 'versions'
> dictionary which maps a (rcs, branch) pair onto the last revision seen for that
> pair. When log caching is on and a log cache exists, cvsps fails to set the
> parent revisions of new log entries because it does not iterate over the log
> cache in the parents computation. A complication is that a file rcs can change
> (move to/from the attic), with respect to its value in the log cache, if the
> file is removed/added back. This patch adds an iteration over the log cache to
> update the rcs of cached log entries, if changed, and to properly populate the
> 'versions' dictionary.
>
> diff -r a024e2db4553 -r 724877229b9a hgext/convert/cvsps.py
> --- a/hgext/convert/cvsps.py	Mon Oct 05 02:33:45 2015 -0700
> +++ b/hgext/convert/cvsps.py	Wed Oct 07 11:33:52 2015 +0300
> @@ -207,6 +207,7 @@
>      # state machine begins here
>      tags = {}     # dictionary of revisions on current file with their tags
>      branchmap = {} # mapping between branch names and revision numbers
> +    rcsmap = {}
>      state = 0
>      store = False # set when a new record can be appended
>
> @@ -439,6 +440,8 @@
>
>              log.append(e)
>
> +            rcsmap[e.rcs.replace('/Attic/', '/')] = e.rcs
> +
>              if len(log) % 100 == 0:
>                  ui.status(util.ellipsis('%d %s' % (len(log), e.file), 80)+'\n')
>
> @@ -446,6 +449,13 @@
>
>      # find parent revisions of individual files
>      versions = {}
> +    for e in sorted(oldlog, key=lambda x: (x.rcs, x.revision)):
> +        rcs = e.rcs.replace('/Attic/', '/')
> +        if rcs in rcsmap:
> +            e.rcs = rcsmap[rcs]
> +        branch = e.revision[:-1]
> +        versions[(e.rcs, branch)] = e.revision
> +
>      for e in log:
>          branch = e.revision[:-1]
>          p = versions.get((e.rcs, branch), None)
> diff -r a024e2db4553 -r 724877229b9a tests/test-convert-cvs.t
> --- a/tests/test-convert-cvs.t	Mon Oct 05 02:33:45 2015 -0700
> +++ b/tests/test-convert-cvs.t	Wed Oct 07 11:33:52 2015 +0300
> @@ -333,13 +333,35 @@
>  testing debugcvsps
>
>    $ cd src
> -  $ hg debugcvsps --fuzz=2
> +  $ hg debugcvsps --fuzz=2 -x >/dev/null
> +
> +commit a new revision changing a and removing b/c
> +
> +  $ cvscall -q update -A
> +  U a
> +  U b/c
> +  $ echo h >> a
> +  $ cvscall -q remove -f b/c
> +  cvs remove: use 'cvs commit' to remove this file permanently
> +  $ cvscall -q commit -mci
> +  Checking in a;
> +  $TESTTMP/cvsrepo/src/a,v  <--  a
> +  new revision: 1.3; previous revision: 1.2
> +  done
> +  Removing b/c;
> +  $TESTTMP/cvsrepo/src/b/c,v  <--  c
> +  new revision: delete; previous revision: 1.3
> +  done
> +
> +update and verify the cvsps cache
> +
> +  $ hg debugcvsps --fuzz=2 -u
>    collecting CVS rlog
> -  11 log entries
> -  cvslog hook: 11 entries
> +  13 log entries
> +  cvslog hook: 13 entries
>    creating changesets
> -  10 changeset entries
> -  cvschangesets hook: 10 changesets
> +  11 changeset entries
> +  cvschangesets hook: 11 changesets
>    ---------------------
>    PatchSet 1
>    Date: * (glob)
> @@ -466,5 +488,18 @@
>    Members:
>       b/c:1.1.2.1->1.1.2.2
>
> +  ---------------------
> +  PatchSet 11
> +  Date: * (glob)
> +  Author: * (glob)
> +  Branch: HEAD
> +  Tag: (none)
> +  Log:
> +  ci
> +
> +  Members:
> +     a:1.2->1.3
> +     b/c:1.3->1.4(DEAD)
> +
>
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

On Fri, Oct 09, 2015 at 01:44:26PM -0400, Augie Fackler wrote:
> On Fri, Oct 09, 2015 at 03:29:35PM +0300, Emanuele Giaquinta wrote:
> > # HG changeset patch
> > # User Emanuele Giaquinta <emanuele.giaquinta@gmail.com>
> > # Date 1444206832 -10800
> > #      Wed Oct 07 11:33:52 2015 +0300
> > # Node ID 724877229b9a3b513c0f325bb362626d6ed46210
> > # Parent  a024e2db4553492e173032f52464e2c4efe0d4fa
> > cvsps: fix computation of parent revisions when log caching is on
> 
> With this applied I'm seeing consistent failures of test-convert-cvs.t like this:
> 
> --- /home/augie/hg/tests/test-convert-cvs.t
> +++ /home/augie/hg/tests/test-convert-cvs.t.err
> @@ -344,10 +344,6 @@
>    $ cvscall -q remove -f b/c
>    cvs remove: use 'cvs commit' to remove this file permanently
>    $ cvscall -q commit -mci
> -  Checking in a;
> -  $TESTTMP/cvsrepo/src/a,v  <--  a
> -  new revision: 1.3; previous revision: 1.2
> -  done
>    Removing b/c;
>    $TESTTMP/cvsrepo/src/b/c,v  <--  c
>    new revision: delete; previous revision: 1.3
> @@ -357,8 +353,8 @@
> 
>    $ hg debugcvsps --fuzz=2 -u
>    collecting CVS rlog
> -  13 log entries
> -  cvslog hook: 13 entries
> +  12 log entries
> +  cvslog hook: 12 entries
>    creating changesets
>    11 changeset entries
>    cvschangesets hook: 11 changesets
> @@ -498,7 +494,6 @@
>    ci
> 
>    Members:
> -       a:1.2->1.3
>         b/c:1.3->1.4(DEAD)

This part belongs to the new test: it seems that cvs did not commit the
change to file 'a', weird. On which system/cvs version do you get this?

I have just found a different problem in the test with cvs-1.12.13 (I
originally tested it with cvs-1.11.23): there are small differences in
the output of cvs remove/commit between the two versions, and I have to
account for them. I'll send an updated patch once we sort out the above
failure.

Emanuele