svnrdump: Faking revision 0

[Ramkumar Ramachandra <ar...@gmail.com>]

Hi Stefan,

Stefan Sperling writes:
> If these revision properties aren't set on the original repository,
> you should not include them in the dump.
> 
> synsync creates these revision properties, but svnrdump should not.
> 
> I think you should query the actual revprop values using the RA API 
> (if possible), rather than using fixed values passed into this function. 
> Though maybe a better fix is to make the replay API replay any revision 
> properties that exist on revision 0?

I suspect everyone has different thoughts in their heads. I'll clarify
what I'm thinking as a first step: To answer why does svnsync create
revision 0, we must analyze what revision 0 is; it is created as soon
as `svnsync init` is called and contains the following information:

svn:sync-from-url - it's clear why svnsync needs this. After the
`init`, subsequent calls to `sync` needs this information, but isn't
given it explicitly.

svn:sync-currently-copying and svn:sync-last-merged-rev - `sync` needs
to know how many revisions to copy.
 
svn:sync-from-uuid - I'm not sure why svnsync NEEDS this, but I
suppose it's just for completeness/ elegance.

svn: date - I'm guessing that this is the date the repository was
created, as opposed to the date of the original checkin. This should
be possible to get via the RA API I think, althought I haven't figured
out how.

I think svnrdump should include all these headers as we might want to
feature "resume" dumping to an existing dumpfile without explicitly
specifying the URL. Besides, there's no harm in including them and we
can just re-use the tests from svnsync.

-- Ram

Re: svnrdump: Faking revision 0

[Ramkumar Ramachandra <ar...@gmail.com>]

Hi Philip,

Philip Martin writes:
> They are all the same (REAL?), they are all just unversioned revision
> properties.

I probably misunderstood something somewhere- I'm making deductions
simply based on my limited observation. I've used the word "real" in
the context that those are the only properties fetched when I call
svn_ra_rev_proplist on revision 0. On the ASF, I've observed that
svn:date is the only real property I get. However, when I use svnsync,
I get additional properties- svnsync is cooking those up.

> Like Stefan I don't understand why svnrdump is inventing revsion
> properties.  It should simply dump those that exist, it should not add
> anything that does not exist.

As I pointed out, svnsync cooks it up for resume support. If we want
to implement resume support (or even find out which repository a
certain dumpfile corresponds to), we must cook up atleast
svn:sync-from-url and svn:sync-from-uuid. Is this not justified?

-- Ram

Re: svnrdump: Faking revision 0

[Ramkumar Ramachandra <ar...@gmail.com>]

Hi Daniel,

Daniel Shahaf writes:
> Philip Martin wrote on Fri, Jul 23, 2010 at 12:03:01 +0100:
> > Like Stefan I don't understand why svnrdump is inventing revsion
> > properties.  It should simply dump those that exist, it should not add
> > anything that does not exist.
> > 
> 
> +1.   Folks can use 'svnsync init --allow-non-empty' later (or just set the
> svn:sync-* revprops by hand).

Now fixed (see the other email on the list for an explanation).

-- Ram

Re: svnrdump: Faking revision 0

[Daniel Shahaf <d....@daniel.shahaf.name>]

Philip Martin wrote on Fri, Jul 23, 2010 at 12:03:01 +0100:
> Like Stefan I don't understand why svnrdump is inventing revsion
> properties.  It should simply dump those that exist, it should not add
> anything that does not exist.
> 

+1.   Folks can use 'svnsync init --allow-non-empty' later (or just set the
svn:sync-* revprops by hand).

Re: svnrdump: Faking revision 0

[Philip Martin <ph...@wandisco.com>]

Ramkumar Ramachandra <ar...@gmail.com> writes:

> Ramkumar Ramachandra writes:
>> svn:sync-from-url - it's clear why svnsync needs this. After the
>> `init`, subsequent calls to `sync` needs this information, but isn't
>> given it explicitly.
>
> We can keep this.
>
>> svn:sync-currently-copying and svn:sync-last-merged-rev - `sync` needs
>> to know how many revisions to copy.
>
> I looked more closely and found out that these headers have to be
> updated everytime a revision is written (hooked into revstart and
> revfinish functions). Since we write immediately to the stream, we
> cannot use this technique- to enable resume support, we must actually
> look at the tail of the dumpfile supplied and see which revision was
> dumped last. Hence, I'm dropping these two headers.
>
>> svn:sync-from-uuid - I'm not sure why svnsync NEEDS this, but I
>> suppose it's just for completeness/ elegance.
>
> We can keep this.
>
>> svn: date - I'm guessing that this is the date the repository was
>> created, as opposed to the date of the original checkin. This should
>> be possible to get via the RA API I think, althought I haven't figured
>> out how.
>
> It turns out that this is a REAL revision property that I can get by
> passing a 0 revision argument to svn_ra_rev_proplist.

They are all the same (REAL?), they are all just unversioned revision
properties.

Like Stefan I don't understand why svnrdump is inventing revsion
properties.  It should simply dump those that exist, it should not add
anything that does not exist.

-- 
Philip

Re: svnrdump: Faking revision 0

[Ramkumar Ramachandra <ar...@gmail.com>]

Hi again,

Ramkumar Ramachandra writes:
> svn:sync-from-url - it's clear why svnsync needs this. After the
> `init`, subsequent calls to `sync` needs this information, but isn't
> given it explicitly.

We can keep this.

> svn:sync-currently-copying and svn:sync-last-merged-rev - `sync` needs
> to know how many revisions to copy.

I looked more closely and found out that these headers have to be
updated everytime a revision is written (hooked into revstart and
revfinish functions). Since we write immediately to the stream, we
cannot use this technique- to enable resume support, we must actually
look at the tail of the dumpfile supplied and see which revision was
dumped last. Hence, I'm dropping these two headers.

> svn:sync-from-uuid - I'm not sure why svnsync NEEDS this, but I
> suppose it's just for completeness/ elegance.

We can keep this.

> svn: date - I'm guessing that this is the date the repository was
> created, as opposed to the date of the original checkin. This should
> be possible to get via the RA API I think, althought I haven't figured
> out how.

It turns out that this is a REAL revision property that I can get by
passing a 0 revision argument to svn_ra_rev_proplist.

-- Ram