subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <...@daniel.shahaf.name>
Subject Re: Issue SVN-4767: svnadmin dump shouldn't canonicalize svn:date
Date Tue, 31 Jul 2018 11:35:40 GMT
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

Mime
View raw message