On Fri, Jun 12, 2015 at 8:38 PM, Branko Čibej <brane_at_wandisco.com> wrote:
> 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.
>
First of all, that commit is neither intended as an optimization
nor an "optimization". It is all about predictable output based
on actual API guarantees. Reverting to the unreliable code
(as in: different backends and different server versions may
give different answers) seems like the least desirable option.
I'll need some time to figure out whether the current response
for "-g" is actuall wrong, i.e. shows revisions that never held
the respective code snippets. Then we need to look into how
we can change the code to produce a better approximation of
what users may "-g" expect to return.
I also do have an idea of what an ideal -g result would look
like but that requires a different API and / or client-side logic.
But that would be a separate topic.
-- Stefan^2.
Received on 2015-06-14 21:35:12 CEST