[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 31 Jul 2018 11:35:40 +0000

Julian Foad wrote on Tue, Jul 31, 2018 at 12:04:23 +0100:
> 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.
>

One clarification:

Both "2015-01-01T00:00:00.000000Z" and "2015-01-01T00:00:00.0Z" are "new
format" timestamps. (I assume they are handled identically, but perhaps some
piece of code somewhere malfunctions when the number of decimal places is
!=á6.) The "old format" which that issue and comment refer to is this one:

[[[
/* Our old timestamp strings looked like this:
 *
 * "Tue 3 Oct 2000 HH:MM:SS.UUU (day 277, dst 1, gmt_off -18000)"
 *
 * The idea is that they are conventionally human-readable for the
 * first part, and then in parentheses comes everything else required
 * to completely fill in an apr_time_exp_t: tm_yday, tm_isdst,
 * and tm_gmtoff.
 *
 * This format is still recognized on input, for backward
 * compatibility, but no longer generated.
 */
static const char * const old_timestamp_format =
"%s %d %s %d %02d:%02d:%02d.%06d (day %03d, dst %d, gmt_off %06d)";
]]]

That string constant is still present in HEAD, but it's been converted to a macro
named OLD_TIMESTAMP_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 prop_tests.py 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.

+1 to having dump write the data verbatim.

Cheers,

Daniel
Received on 2018-07-31 13:35:59 CEST

This is an archived mail posted to the Subversion Dev mailing list.