git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Sperling <stsp@elego.de>
To: Ramkumar Ramachandra <artagnon@gmail.com>
Cc: "dev@subversion.apache.org" <dev@subversion.apache.org>,
	Bert Huijben <rhuijben@collab.net>,
	Daniel Shahaf <d.s@daniel.shahaf.name>,
	Will Palmer <wmpalmer@gmail.com>,
	David Michael Barr <david.barr@cordelta.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Sverre Rabbelier <srabbelier@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH v2] Add svnrdump
Date: Wed, 14 Jul 2010 19:24:29 +0200	[thread overview]
Message-ID: <20100714172429.GC25861@ted.stsp.name> (raw)
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.
 
> >  - 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.
 
> >  - 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.

> >  - 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

  parent reply	other threads:[~2010-07-14 17:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-09 14:29 [PATCH v2] Add svnrdump Ramkumar Ramachandra
2010-07-13 20:11 ` Stefan Sperling
2010-07-14 15:32   ` Stefan Sperling
2010-07-14 16:01     ` Ramkumar Ramachandra
2010-07-14 16:48       ` C. Michael Pilato
2010-07-15 10:28         ` Ramkumar Ramachandra
2010-07-14 17:24       ` Stefan Sperling [this message]
2010-07-14 17:31         ` C. Michael Pilato
2010-07-14 17:34           ` Stefan Sperling
2010-07-14 17:47           ` Jonathan Nieder
2010-07-14 17:56             ` C. Michael Pilato
2010-07-15 12:01         ` Ramkumar Ramachandra
2010-07-14 19:25       ` Bert Huijben
2010-07-15 12:07         ` Ramkumar Ramachandra
2010-07-15 19:02   ` Ramkumar Ramachandra
2010-07-15 19:23     ` Stefan Sperling
2010-07-21 11:46       ` Ramkumar Ramachandra
2010-07-21 13:29         ` Daniel Shahaf
2010-07-21 19:03   ` Ramkumar Ramachandra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100714172429.GC25861@ted.stsp.name \
    --to=stsp@elego.de \
    --cc=artagnon@gmail.com \
    --cc=d.s@daniel.shahaf.name \
    --cc=david.barr@cordelta.com \
    --cc=dev@subversion.apache.org \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=rhuijben@collab.net \
    --cc=srabbelier@gmail.com \
    --cc=wmpalmer@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).