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

RE: svn commit: r1686478 - in /subversion/trunk/subversion: libsvn_repos/rev_hunt.c tests/libsvn_client/mtcc-test.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Mon, 22 Jun 2015 15:42:25 +0200

> -----Original Message-----
> From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
> Sent: vrijdag 19 juni 2015 20:29
> To: commits_at_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
Received on 2015-06-22 15:42:39 CEST

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