subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <>
Subject Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
Date Tue, 31 Jul 2018 11:04:23 GMT

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.

- 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.

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".

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.
(I made some progress in unifying the output logic for those three dump producers a couple
of years ago, but I left this part alone because I did not know what to do with it.)

The discrepancy I noticed above comes from 41 and 42 which explicitly propset
svn:date to a value such as '2015-01-01T00:00:00.0Z'. "svnadmin dump" was munging this non-canonical
value on output, which meant it was not a true dump. (Whether a non-canonical format should
have been allowed into the repository in the first place is a separate issue.)

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.

- Julian

View raw message