http://subversion.tigris.org/issues/show_bug.cgi?id=4476

There's another part to this issue: 'svnadmin load'.

When mergeinfo refers to r0, not only does 'svnadmin dump' fail (fixed in 
r1574868) and 'svnsync' fail, but 'svnadmin load' also fails, as I found when I 
started writing a test for the 'svnsync' problem.

I think 'svnadmin load' should always be able to load a dump file in order to 
restore a backup. There is a 'validate properties' option, so I would say the 
loading should fail if that is enabled, and succeed if not enabled. When 
loading 'succeeds' it should load exactly what was in the dump file, and not 
try to 'correct' it, for any data that could in principle have been dumped out 
of a repository that was not utterly broken.

(If we want to consider automatically 'correcting' properties during loading, 
that should be a separate development and should be optional, in other words 
there should always be a way to load exactly what is in the dump file. That 
feature should probably be implemented at a higher level than libsvn_repos, 
too.)

But 'svnadmin load' can do more than just restore revisions exactly how they 
were: it can also renumber revisions and move all loaded paths into a 
subdirectory, and when doing either or both of these it attempts to adjust 
mergeinfo accordingly. In that case if mergeinfo is invalid then we have three 
options:

  - libsvn_repos throws an error
  - libsvn_repos warns and does not adjust that mergeinfo property
  - svnadmin 'corrects' the mergeinfo, and then libsvn_repos adjusts it

I think warning and not adjusting is better than throwing an error, but I am 
not sure if 'svnadmin load' should be in the business of correcting bad 
mergeinfo or if that would be better left to an external tool. I would say this 
mode of operation (adjusting revision numbers and/or paths) is a lower priority.


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'll come to 'svnsync' later, but basically my current thought is it should 
'correct' the mergeinfo by removing the r0 reference.

- Julian

Reply via email to