On 31.07.2018 13:04, Julian Foad wrote:
> https://issues.apache.org/jira/browse/SVN-4767
>
> Running tests with the '--dump-load-cross-check' option revealed a minor
> discrepancy between the dump files produced by 'svnadmin dump' and 'svnrdump
> dump' in some test cases.
>
> [...]
> K
> svn:date
> V
> - 2015-01-01T00:00:00.000000Z
> + 2015-01-01T00:00:00.0Z
> [...]
>
> "svnadmin dump" "canonicalizes" each svn:date revprop while dumping, in the
> function write_revision_record().
>
> This seems to have been done in r842390 in order to upgrade from pre-0.14
> repository format to the new timestamp format introduced in 0.14 -- see issue
> SVN-614 "DAV:creationdate needs to be an ISO8601 date".
> svn_time_from_cstring() reads either new or old format, and then
> svn_time_to_cstring() writes the new format.
There's another possible interpretation here: that the millisecond field
should be 6 digits wide, but for some reason svn_time_to_cstring doesn't
format it that way when the value is zero. I don't have the actual
standard, so I can't check what it says ... and, of course, in the case
of 0 there is no semantic difference.
> However, this does not only convert old to new format, but could also make
> textual changes to the string if the revprop value is not already canonical.
> Dump should carefully output exactly what is in the repository and not
> gratuitously change it. In retrospect, such a transformation should have been
> done during "svnadmin load" instead of in "dump".
Agreed, this fits with what "svnadmin load" already does with the
--normalize-props option. But if the timestamp is *not* in the right
format, "load" should fix it. Maybe "except with
--bypass-prop-validation"? Another option that controls "load" behaviour
WRT dates is --ignore-dates, though I can't remember why we introduced that.
In any case I think the logic should be changed a bit: Only reformat the
svn:date property if it is not already in ISO8601 format; otherwise,
leave it alone.
> While "svnadmin dump" makes this transformation, "svnrdump dump" and
> "svndumpfilter" do not. This could lead to unintended differences in dump
> output depending on which tool is used.
"svnadmin dump" should always define the canonical behaviour. Both
"svnrdump" and "svndumpfilter" are newer; they should behave the way
"svnadmin dump" does, not the other way around. One _could_ argue that
this is an omission in svnrdump.
> Therefore I will remove this code path. It even has a comment saying "###
> Remove this when it is no longer needed for sure" which referred to being
> needed for pre-0.14 upgrades. We definitely no longer need that.
+0, and I'll explain why:
1. This is not a trivial change and should be documented in 1.11
release notes (hence, it stays on trunk until then). It's possible
that some 3rd-party tools rely on "svnadmin dump" producing
timestamps in the ISO format.
2. Our dumpfile format is documented. With your proposed change, there
is a slight chance that "svnadmin dump" could produce invalid dump
files. This is in effect a corollary of point 1., above. Note that
"svndrump dump" is already "infected" by this potential bug, whereas
"svndumpfilter" is not because it starts with a presumably valid
dumpfile.
-- Brane
