Re: Issue #4476 - Mergeinfo containing r0 makes svnsync and svnadmin dump fail

Julian Foad wrote:
> http://subversion.tigris.org/issues/show_bug.cgi?id=4476
[...]
> PROPOSAL
>
> In the presence of a mergeinfo property that refers to r0, at the libsvn_repos
> API:
>
> 'dump'
>   shall dump the property verbatim (with nowarning). (Done.)
>
> 'load', with 'validate properties' OFF and not adjusting
> revision numbers or paths,
>   shall load theproperty verbatim, and warn.
>
> 'load', with 'validate properties' OFF and adjusting revision
> numbers or paths,
>   shall load the property verbatim (unadjusted), and warn.
>
> 'load', with 'validate properties' ON,
>   shall fail.
>
> And I would say the same rules should apply to any property that fails the
> validation rules.
>
> At the application layer:
>
> 'svnadmin dump' and 'svnadmin load'
>   should behave the same as the repos layer.
>
> Does that sound good as a fix that I can do now and back-port to 1.8.x and
> 1.7.x?

I have done this, and proposed it for backport to 1.7 and 1.8.

(My fix adds a new enumeration constant to an enumeration type, for the warning. For back-porting, I renamed this constant to a double-underscore name to indicate it's not for public use in the 1.7/1.8 APIs.)

> I'll come to 'svnsync' later, but basically my current thought is it
> should 'correct' the mergeinfo by removing the r0 reference.

I have written a test for this, and hacked up code to do this by textual substitution. It isn't quite right -- for example it would go wrong if a path contained the character sequence ":0", among other issues. I ought to rewrite it in the form of a proper mergeinfo parser but one that is more lenient than the regular parser.

I also noticed that loading from a dump file also modifies mergeinfo in other ways such as removing references to r1. I don't think is a good idea: I think there should be a way to load exactly what is in the dump. But that's a different, though related, issue.

- Julian
Received on 2014-12-04 18:59:31 CET