+1.
Just noting that only -g appears to be affected, while I don’t know if the server side change also affects non -g. If non -g was optimized while it didn't affect clients we should try to keep that part.
Instead of announcing a client side capabilty that is transfered for every ra operation just passing a new flag would work as well… and it would allow keeping the old ra api behavior stable.
Bert
Sent from Surface
From: Branko Čibej
Sent: Friday, June 12, 2015 8:38 PM
To: dev_at_subversion.apache.org
On 12.06.2015 16:55, Philip Martin wrote:
> Philip Martin <philip.martin_at_wandisco.com> writes:
>
>> Philip Martin <philip.martin_at_wandisco.com> writes:
>>
>>> 1.9 has a more accurate svn_fs_contents_changed() that doesn't report as
>>> many false positives. This means that some (or all?) of the -g revisions
>>> reported by svn_repos_get_file_revs2() do not include a textdelta that
>>> was included by 1.8. It appears at some level the 1.9 client interprets
>>> the revision without a textdelta as being the same as a revision with a
>>> textdelta that changes nothing, while the 1.8 client relies on the
>>> missing textdelta. If I force the 1.9 server to send textdeltas between
>>> identical texts the 1.8 client then behaves as the test expects.
>> Looking at r1573111
>>
>> /* Check if the contents *may* have changed. (Allow false positives,
>> for now, as the blame implementation currently depends on them.) */
>> /* Special case: In the first revision, we always provide a delta. */
>> if (sb->last_root)
>> - SVN_ERR(svn_fs_contents_changed2(&contents_changed, sb->last_root,
>> - sb->last_path, root, path_rev->path,
>> - FALSE, sb->iterpool));
>> + SVN_ERR(svn_fs_contents_different(&contents_changed, sb->last_root,
>> + sb->last_path, root, path_rev->path,
>> + sb->iterpool));
>> else
>> contents_changed = TRUE;
>>
>> That looks like an error since svn_fs_contents_different() passes TRUE
>> for strict, not FALSE. I think the intention was that r1573111 should
>> call svn_fs_contents_changed().
>>
>> However the implementation of root_vtable_t.contents_changed() has
>> changed in 1.9 and it no longer reports as many changes as it used to
>> report so even replacing the _different() call with a _changed() call
>> does not solve the problem. The only way I can see to fix the problem
>> is to simply ignore/remove the call altogether and to hard-code
>> contents_changed to TRUE.
> This revision is the client-side magic:
>
> r1570936 | stefan2 | 2014-02-22 22:27:06 +0000 (Sat, 22 Feb 2014) | 10 lines
>
> Fix the blame -g implemenation to no longer depend on false positives
> in the file contents change detection.
>
> A 1.8 server works with all clients whether they have the fix or not. A
> 1.9 server only works when the client has the fix.
>
> That client-side fix is only in 1.9, so at the moment, only 1.9 clients
> work with 1.9 servers.
IMO, the server-side "optimization" should either be reverted, or made
dependent on a — probably new — client capability that 1.9 clients (and
possibly patched 1.8 clients) can advertise.
-- Brane
Received on 2015-06-13 20:19:32 CEST