> -----Original Message----- > From: stef...@apache.org [mailto:stef...@apache.org] > Sent: vrijdag 19 juni 2015 20:29 > To: comm...@subversion.apache.org > Subject: svn commit: r1686478 - in /subversion/trunk/subversion: > libsvn_repos/rev_hunt.c tests/libsvn_client/mtcc-test.c > > Author: stefan2 > Date: Fri Jun 19 18:29:01 2015 > New Revision: 1686478 > > URL: http://svn.apache.org/r1686478 > Log: > Workaround for 'svn blame' -g with old clients. > > Old clients rely on receiving a callback whenever the path changes, e.g. > when switching from one branch to another. So, for now, we unconditionally > send a text delta in that case. Future releases should make that backward > compatibility behavior an option that will be controlled be e.g. client > capabilities. > > Found by: philipm > > * subversion/libsvn_repos/rev_hunt.c > (send_path_revision): Always send a text delta when the path changes. > > * subversion/tests/libsvn_client/mtcc-test.c > (handle_rev): Update the expectations. > > Modified: > subversion/trunk/subversion/libsvn_repos/rev_hunt.c > subversion/trunk/subversion/tests/libsvn_client/mtcc-test.c > > Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/rev_hu > nt.c?rev=1686478&r1=1686477&r2=1686478&view=diff > ================================================================ > ============== > --- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original) > +++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Fri Jun 19 18:29:01 > 2015 > @@ -1373,15 +1373,31 @@ send_path_revision(struct path_revision > SVN_ERR(svn_prop_diffs(&prop_diffs, props, sb->last_props, > sb->iterpool)); > > - /* 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_different(&contents_changed, sb->last_root, > - sb->last_path, root, path_rev->path, > - sb->iterpool)); > + /* Check if the contents *may* have changed. */ > + if (! sb->last_root) > + { > + /* Special case: In the first revision, we always provide a delta. */ > + contents_changed = TRUE; > + } > + else if (strcmp(sb->last_path, path_rev->path)) > + { > + /* This is a HACK!!! > + * Blame, in older clients anyways, relies on getting a notification > + * whenever the path changes - even if there was no content change. > + * > + * TODO: A future release should take an extra parameter and depending > + * on that either always send a text delta or only send it if there > + * is a difference. */ > + contents_changed = TRUE;
Is this really always when ther path changed, or only when the path changed *and* we are looking at a -g blame? The fact that we see a path change might be a quite visible side effect on a -g blame (where different branches have different paths), but it can also happen on a rename (without actual change to the file) on a non -g blame. If this fix is only relevant to -g blames we shouldn't apply it to other blame invocations. Bert