You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2014/02/04 23:13:12 UTC
[PATCH][RFC] svnadmin load --ignore-dates
Didn't want to commit this directly just yet, given my somewhat
disconnected attention level in the project as of late. Mostly looking
for general feedback on the idea of the "sounds useful if it works" or
"you *really* don't want to go there" variety, but deeper review is of
course welcome, too.
Log message:
{{{
Introduce '--ignore-dates' option for 'svnadmin load', which causes
the load process to ignore the revision datestamps found in the
dumpstream. This allows folks to more easily use dumpfiles as
repository templates which appear (datestamp-wise) as normal commits
would.
* subversion/include/svn_repos.h
(svn_repos_load_fs5): New version of this API which accepts
'ignore_dates' flag.
* subversion/libsvn_repos/deprecated.c
(svn_repos_load_fs4): Moved here from load-fs-vtable.c, and is now
just a wrapper around svn_repos_load_fs5().
* subversion/libsvn_repos/load-fs-vtable.c
(parse_baton): Add 'ignore_dates' member.
(set_revision_property, close_revision): Handle ignore_dates flag.
(svn_repos_load_fs5): New version of this API which accepts
'ignore_dates' flag.
* subversion/svnadmin/svnadmin.c
(svnadmin__ignore_dates): New enum value.
(options_table): Define --ignore-dates option.
(cmd_table): Allow 'load' to accept --ignore-dates.
(svnadmin_opt_state): Add 'ignore_dates' member.
(subcommand_load): Now use svn_repos_load_fs5().
(main): Handle --ignore-dates option.
}}}
--Mike
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/05/2014 05:45 AM, Julian Foad wrote:
> C. Michael Pilato wrote:
>
>> Mostly looking
>> for general feedback on the idea of the "sounds useful if it works" or
>> "you *really* don't want to go there" variety, but deeper review is of
>> course welcome, too.
>
> It seems a reasonable enhancement to me. Simple and to the point.
>
> (A couple of style nits:
>
> - is_date = strcmp(name, SVN_PROP_REVISION_DATE) ? FALSE : TRUE;
> + is_date = strcmp(name, SVN_PROP_REVISION_DATE) == 0;
Noted.
> and you unnecessarily moved the block of code that stores the loaded date stamp into the baton.)
Oops! Good catch.
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Julian Foad <ju...@btopenworld.com>.
C. Michael Pilato wrote:
> Mostly looking
> for general feedback on the idea of the "sounds useful if it works" or
> "you *really* don't want to go there" variety, but deeper review is of
> course welcome, too.
It seems a reasonable enhancement to me. Simple and to the point.
(A couple of style nits:
- is_date = strcmp(name, SVN_PROP_REVISION_DATE) ? FALSE : TRUE;
+ is_date = strcmp(name, SVN_PROP_REVISION_DATE) == 0;
and you unnecessarily moved the block of code that stores the loaded date stamp into the baton.)
- Julian
RE: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Bert Huijben <be...@qqmail.nl>.
The ra layer function to do a date lookup currently doesn’t have a path, so we do a date lookup on the repository level and then use it like a revision number.
To make date useful without the current limitations we should first design a few new ra level functions that allow resolving a date (range) to something that would be useful as input. Just fixing the backend to return some other revision when we pass a date won’t help in any way.
Operations like
$ svn merge -r {2014-01-01}:{2014-01-31} trunk .
don’t make any sense if the first date resolves to r789 and the second to r2, because ^/second-product /trunk and ^/dependencies/other-project/trunk just happens to have the best match for those exact dates.
The whole idea of our current date resolving assumes that we have continuously growing timestamps over the entire repository. If we want to change that we should change the scope of the date lookup as the first step.
Perhaps we should try to add a path to the resolve function and see where that helps for specific cases?
After that we can look at the problems this will introduce in the backend.
Bert
From: Stefan Fuhrmann [mailto:stefan.fuhrmann@wandisco.com]
Sent: woensdag 5 februari 2014 06:08
To: Branko Čibej
Cc: Subversion Development
Subject: Re: [PATCH][RFC] svnadmin load --ignore-dates
On Wed, Feb 5, 2014 at 6:03 AM, Stefan Fuhrmann <stefan.fuhrmann@wandisco.com <ma...@wandisco.com> > wrote:
On Wed, Feb 5, 2014 at 4:02 AM, Branko Čibej <brane@wandisco.com <ma...@wandisco.com> > wrote:
On 05.02.2014 03:10, Daniel Shahaf wrote:
However, 'load --ignore-dates' has another use-case: preventing the "-r {DATE} doesn't work as intended" syndrome when loading a project's history into an existing repository.
I would argue that this is an implementation bug, pure and simple. The fact that it has persisted since svn-1.0 doesn't change anything. There is no reason why -r {DATE} couldn't be made to work regardless of the ordering of the dates; it's just that no-one has taken the trouble to actually fix the filesystem to return valid results in all cases.
That would be extremely expensive to do in the existing backends:
We would need to effectively read all revision properties of all revs.
If we added an index ("revision order by time stamp"), we could do
better but would need to update the index upon every (rev) propset.
One more thing. Even with such index, we can only get -r {DATE} to
work but handling the date range {DATE1}:{DATE2} requires massive
changes in the underlying logic and potentially APIs. The reason is
that the date range potentially translates into a sequence of revision
ranges which brakes assumptions of path continuity in e.g. 'svn log'.
-- Stefan^2.
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Feb 5, 2014 at 6:03 AM, Stefan Fuhrmann <
stefan.fuhrmann@wandisco.com> wrote:
> On Wed, Feb 5, 2014 at 4:02 AM, Branko Čibej <br...@wandisco.com> wrote:
>
>> On 05.02.2014 03:10, Daniel Shahaf wrote:
>>
>> However, 'load --ignore-dates' has another use-case: preventing the "-r
>> {DATE} doesn't work as intended" syndrome when loading a project's history
>> into an existing repository.
>>
>>
>> I would argue that this is an implementation bug, pure and simple. The
>> fact that it has persisted since svn-1.0 doesn't change anything. There is
>> no reason why -r {DATE} couldn't be made to work regardless of the ordering
>> of the dates; it's just that no-one has taken the trouble to actually fix
>> the filesystem to return valid results in all cases.
>>
>
> That would be extremely expensive to do in the existing backends:
> We would need to effectively read all revision properties of all revs.
> If we added an index ("revision order by time stamp"), we could do
> better but would need to update the index upon every (rev) propset.
>
One more thing. Even with such index, we can only get -r {DATE} to
work but handling the date range {DATE1}:{DATE2} requires massive
changes in the underlying logic and potentially APIs. The reason is
that the date range potentially translates into a sequence of revision
ranges which brakes assumptions of path continuity in e.g. 'svn log'.
-- Stefan^2.
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Feb 5, 2014 at 4:02 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 05.02.2014 03:10, Daniel Shahaf wrote:
>
> However, 'load --ignore-dates' has another use-case: preventing the "-r
> {DATE} doesn't work as intended" syndrome when loading a project's history
> into an existing repository.
>
>
> I would argue that this is an implementation bug, pure and simple. The
> fact that it has persisted since svn-1.0 doesn't change anything. There is
> no reason why -r {DATE} couldn't be made to work regardless of the ordering
> of the dates; it's just that no-one has taken the trouble to actually fix
> the filesystem to return valid results in all cases.
>
That would be extremely expensive to do in the existing backends:
We would need to effectively read all revision properties of all revs.
If we added an index ("revision order by time stamp"), we could do
better but would need to update the index upon every (rev) propset.
-- Stefan^2.
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Branko Čibej <br...@wandisco.com>.
On 05.02.2014 03:10, Daniel Shahaf wrote:
> However, 'load --ignore-dates' has another use-case: preventing the
> "-r {DATE} doesn't work as intended" syndrome when loading a project's
> history into an existing repository.
I would argue that this is an implementation bug, pure and simple. The
fact that it has persisted since svn-1.0 doesn't change anything. There
is no reason why -r {DATE} couldn't be made to work regardless of the
ordering of the dates; it's just that no-one has taken the trouble to
actually fix the filesystem to return valid results in all cases.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Shahaf wrote:
> C. Michael Pilato wrote:
>> Introduce '--ignore-dates' option for 'svnadmin load',
>
> [...] However, 'load --ignore-dates' has another use-case: preventing
> the "-r {DATE} doesn't work as intended" syndrome when loading a
> project's history into an existing repository. [...]
Ignoring dates upon load would only *keep* '-r {DATE}' working for the previous history that was in the repo before the load, while *losing* the date history of the newly loaded part. In some cases that might be useful but it's not a solution to the problem in general.
- Julian
Re: [PATCH][RFC] svnadmin load --ignore-dates
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Tue, Feb 04, 2014 at 17:13:12 -0500:
> Didn't want to commit this directly just yet, given my somewhat
> disconnected attention level in the project as of late. Mostly looking
> for general feedback on the idea of the "sounds useful if it works" or
> "you *really* don't want to go there" variety, but deeper review is of
> course welcome, too.
>
> Log message:
>
> {{{
> Introduce '--ignore-dates' option for 'svnadmin load', which causes
> the load process to ignore the revision datestamps found in the
> dumpstream. This allows folks to more easily use dumpfiles as
> repository templates which appear (datestamp-wise) as normal commits
> would.
In the case of template repositories, the most important thing is
resetting the UUID (and we have 'load --ignore-uuid'). Changing the
date values of the "mkdir trunk"? I don't know that I would care either
way about what they were. Particularly if the log message of the
template's final commit explained what was going on.
However, 'load --ignore-dates' has another use-case: preventing the
"-r {DATE} doesn't work as intended" syndrome when loading a project's
history into an existing repository. In that scenario, it might be
useful to preserve the original date values (and we already have an
SVN_PROP_REVISION_ORIG_DATE macro defined).
Daniel
(the last sentence might be an instance of feature creep)