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
Regards,
Evgeny Kotkov
Received on 2015-10-26 17:45:57 CET