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

Re: No-op changes no longer dumped by 'svnadmin dump' in 1.9

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Mon, 26 Oct 2015 19:45:20 +0300

Bert Huijben <bert_at_qqmail.nl> writes:

> Noting everything as a change even if there is no actual delta... still
> looks like a bug we should fix (and we did) instead of something that we
> should restore.
> For the reasoning on why:
> For blame we are really interested if there are deltas... and if there
> aren't any changes the revision is not 'interesting'. (See docs of the api).

The place where we define "interesting" revisions is svn_fs_history_prev2():

  /** Set @a *prev_history_p to an opaque node history object which
   * represents the previous (or "next oldest") interesting history
   * location for the filesystem node represented by @a history, or @c
   * NULL if no such previous history exists. If @a cross_copies is @c
   * FALSE, also return @c NULL if stepping backwards in history to @a
   * *prev_history_p would cross a filesystem copy operation.
   * @note If this is the first call to svn_fs_history_prev() for the @a
   * history object, it could return a history object whose location is
   * the same as the original. This will happen if the original
   * location was an interesting one (where the node was modified, or
   * took place in a copy event). This behavior allows looping callers
   * to avoid the calling svn_fs_history_location() on the object
   * returned by svn_fs_node_history(), and instead go ahead and begin
   * calling svn_fs_history_prev().

Functions like svn_ra_get_file_revs2() that you mentioned just link to it:

   * Retrieve a subset of the interesting revisions of a file @a path
   * as seen in revision @a end (see svn_fs_history_prev() for a
   * definition of "interesting revisions").

The svn_fs_history_prev2() function reports revisions with empty deltas as
interesting, and has been doing that for years (see the attached patch with
a test). This behavior did not change in 1.9.

This means that after r1572363 and r1573111, svn_ra_get_file_revs2() and
svn_repos_get_file_revs2() were skipping some of the "interesting" revisions,
according to the FS API defining the concept. Moreover, this behavior could
be inconsistent even within a single function like svn_ra_get_file_revs2()
that calls svn_ra_get_log2() for old servers, as get-log notices revisions
with empty deltas.

I think that it's another example of where r1572363 and r1573111 introduce an
inconsistent and unwanted behavior change.

Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> writes:

>> As for /trunk, I think that we could do that, so I sketched this option in
>> the attached patch.
> The patch looks o.k.

Thanks for giving this patch a look.

I examined the differences between these functions, svn_repos__compare_files()
and svn_fs_contents_different(), and I think that we also have a problem in
FSFS's implementation of the new svn_fs_contents_different() API in 1.9.x.
The implementation relies on representation_t.expanded_size value when doing
the comparison. If this value is zero, a representation is considered empty,
and two empty representations are considered equal.

The problem is that the expanded_size can be zero for empty files and also for
some of the PLAIN representations, which are not empty. If I am not mistaken,
we have a workaround for this in /trunk — r1673875 [1] and follow-ups, but not
in Subversion 1.9. Hence, the new svn_fs_contents_different() API can fail to
distinguish different contents in 1.9.x under the circumstances described in
issue 4554 [2].

So, we can use it in trunk, but this API may produce invalid results in 1.9.x.

I'll commit the patch, and then I am going to prepare the nomination consisting
of r1709388 and the new dump_no_op_change() test.

[1] https://svn.apache.org/r1673875
[2] https://issues.apache.org/jira/browse/SVN-4554

Evgeny Kotkov

Received on 2015-10-26 17:45:57 CET

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