Re: [PATCH v2] Add svnrdump

From: Stefan Sperling <stsp@elego.de>
Date: Jul 14, 2010, 17:24 UTC
Message-ID: <20100714172429.GC25861@ted.stsp.name>
In-Reply-To: <20100714160149.GA7561@debian>

On Wed, Jul 14, 2010 at 06:01:49PM +0200, Ramkumar Ramachandra wrote:

> Yeah, I forgot to ask about this: passing 0 as an argument to the
> replay API doesn't seem to work. Why? How do I dump revision 0 then?

Indeed. This seems to be a problem in the replay API.
This is not a problem for svnsync itself because svnsync manually
sets the revision properties while doing a sync.
We can fix the replay API to allow svnrdump to get revprops for r0.

Show 7 quoted lines

> >  - You're missing a couple of fields:
> >    The UUID of the repository.
> >    Text-content-sha1
> >    Text-delta-base-md5
> >    Text-delta-base-sha1
> 
> Yes, I'm aware.

OK.

Show 6 quoted lines

> >  - I've seen a "Prop-delta: true" line which svnadmin dump does not print.
> 
> Correct. `svnadmin dump` has a logic for determining when the prop is
> really a delta (as opposed to a delta against /dev/null). Since
> there's no harm printing extra Prop-delta headers, I decided not to
> implement this logic.

We can fix this later.

Show 8 quoted lines

> >  - You're missing some newlines that svnadmin dump prints (cosmetic,
> >    but it would be nice if both produced matching output).
> 
> This isn't in the dump-load-format spec document (atleast afaik), and
> it's very hard to get this right (yes, I tried). Moreover, it's very
> ungratifying to have a few extra newlines (reverse engineered from
> `svnadmin dump`) printed at the end of 10+ hrs of work; yes, that's
> what I estimate it'll take to fix this.

Well, it would be really nice to have. Details like this are time sinks, I know. But it pays off. You don't have to do it right away. We can file an issue so we don't forget about fixing it before 1.7 release. If necessary, feel free to adjust the output of svnadmin dump a little if that makes it easier for svnrdump to produce matching output.

> gawk '$0 !~ "Prop-delta: true|Text-delta-base-|sha1|Text-copy-source-|^-$" && $0 ~ "^+|^-" { print; }'

Fine for testing. But I still think the end-result should look just like svnadmin dump, if possible. That would make testing even easier.

> > Please get rid of all global variables in svnrdump.c:
> Will do. I'm waiting for commit access, because I don't want to make
> un-versioned edits to the file that I cannot track or revert in
> future.

What about using git until then? It does not matter which state you
initially import into the Subversion repository. But well, whatever
works for you is best.

> Please see the current `validate.sh` for an example of the
> functionality I'll write into the unit tests.

Thanks, I'll take a look.

Stefan