Re: [PATCH v2] Add svnrdump

On Tue, Jul 13, 2010 at 10:11:05PM +0200, Stefan Sperling wrote:

A couple of additional remarks:

Playing with svnrdump and comparing its output to the output of svnadmin dump --deltas, I noticed that:

 - svnrdump doesn't dump revision 0.
   It should dump revision 0, because that revision can contain important
   revprops such as metadata for svnsync (svn:sync-last-merge-rev etc.)
 - You're missing a couple of fields:
   The UUID of the repository.
   Text-content-sha1
   Text-delta-base-md5
   Text-delta-base-sha1
 - I've seen a "Prop-delta: true" line which svnadmin dump does not print.
 - You're missing some newlines that svnadmin dump prints (cosmetic,
   but it would be nice if both produced matching output).

How to reproduce what I'm seeing:
  Use svnsync to get a copy of the numptyphysics repository at
    https://vcs.maemo.org/svn/numptyphysics (I had a dump of that lying
    around... other repositories might do the job just as well, of course)
  Dump the repository using svnadmin dump --deltas.
  Dump the repository using svnrdump.
  Compare output with diff -u.

Please get rid of all global variables in svnrdump.c: subversion/svnrdump/svnrdump.c:43: warning: declaration of `pool' shadows a glob al declaration subversion/svnrdump/svnrdump.c:33: warning: shadowed declaration is here subversion/svnrdump/svnrdump.c:91: warning: declaration of `pool' shadows a glob al declaration subversion/svnrdump/svnrdump.c:33: warning: shadowed declaration is here

When adding unit tests for svnrdump, please make each and every one of those tests compare with output of svnadmin dump --deltas, so that we will keep them in sync.

Thanks, Stefan