Re: [PATCH] New client diff callbacks

On Wed, Feb 27, 2008 at 05:12:42PM +0100, Stephen Butler wrote:
> Hi Julian and other conflict-resolvers,
>
> Here is a diff that is useful for tree-conflict detection, but which
> is not strictly part of the tree-conflict code. Can it be merged into
> the Subversion trunk?

I'd like to bump this long-sleeping thread.

AFAIK the patch has not been reviewed yet.
Would anyone volunteer reviewing it?

> We want to do two things that will rely on this patch.
>
> 1. Stop an update/switch/merge operation if it tries to enter a
> directory containing old, unresolved conflicts, including text or
> property conflicts. This would resolve issue #2202. Requires a
> callback for opening a directory.

This point is now invalid, since Mike Pilato pointed out that
this check should rather be done in the initial working copy
crawl preceding a commit. He said since we check for conflicts
during that crawl anyway, we might as well adapt that code instead
of adding open_dir() callbacks to the editor.

However, Mike also said that open_dir() callbacks might be nice
to have anyway. So we might still want to leave them in.

I myself would in principle object to adding dead code, but if
changing the editor interface really requires a big ceremony every
time, we might as well give open_dir() callbacks a free piggyback
ride into the tree and hope they get used by clients at some point.
Any client API consumers around for comments?

> 2. Notify the user if (new) tree conflicts have been detected in a
> directory. Requires a callback for closing a directory.

This point is still valid.

Since the tree-conflicts branch is already carrying a lot of changes,
and there are a few more to come, it would be nice for us to put this
helper functionality into trunk straight away. The farther the branches
divert the more headaches we'll have to face when merging tree-conflicts
into trunk. We already have a few libsvn_wc API changes in the tree-conflicts
branch.

But given that a different patch I submitted (which only changed white
space!) was rejected because there is still backporting to 1.5 going on,
I guess there will be objections committing the new diff callbacks to
trunk straight away, right?

If so, I will have to add these callbacks to the tree-conflicts
branch before they get into trunk (and I can live with that, it's
not that big of an issue, really).

Opinions?

 
> For update/switch, appropriate callbacks already exist in the struct
> svn_delta_editor_t.
>
> For merge, the code in libsvn_client implements a smaller set of
> callbacks, declared in svn_wc_diff_callbacks2_t. This patch adds the
> two missing callbacks.
>
> Due to client code shared by merge and diff, adding the new callbacks
> for the merge-specific code requires a slew of changes to the
> diff-specific code in order to compile. If there's a better way,
> please point me in the right direction.
>
> Regards,
> Steve
>
> [[[
>
> Extend the diff-callbacks struct to handle the opening and closing of
> a directory explicitly. For backward compatibility, name the new
> struct svn_wc_diff_callbacks3_t.
>
> In libsvn_wc, create the new struct, adjust all relevant function args
> and comments to track the new version, and add code to call the new
> callbacks (dir_opened() and dir_closed()).
>
> In libsvn_client, add new callback implementations for diff and merge,
> and track the version change.
>
> For merge, the new callbacks don't do anything yet, but we plan to use
> them to improve conflict detection. The dir_opened() function will be
> useful in blocking merge into a conflicted directory. The
> dir_closed() function will be useful in reporting tree conflicts.
>
> For diff, the new callbacks exist for compatibility only.
>
> * subversion/include/svn_wc.h
> (svn_wc_diff_callbacks3_t): New struct.
> (svn_wc_diff_callbacks2_t): Moved comments to the new struct,
> editing slightly to avoid repetition. Added deprecation comments.
> (svn_wc_get_diff_editor5): New function.
> (svn_wc_get_diff_editor4): Deprecated (edit to comment only).
> (svn_wc_diff5): New function.
> (svn_wc_diff4): Deprecated (edit to comment only).
>
> * subversion/libsvn_wc/diff.c
> (edit_baton): Track diff-callbacks version in struct field.
> (make_editor_baton,
> svn_wc_get_diff_editor4): Track diff-callbacks version.
> (file_changed,
> file_added,
> file_deleted,
> dir_added,
> dir_deleted,
> dir_props_changed): Track diff-callbacks version in comments only.
> (dir_opened,
> dir_closed,
> svn_wc_diff5): New functions.
> (svn_wc_diff4): Deprecated (edit to comment only).
> (callbacks2_wrapper): New struct.
>
> * subversion/libsvn_client/repos_diff.c
> (edit_baton): Track diff-callbacks version bump in struct.
> (close_directory): Move the call to get_path_access() so that it is
> done unconditionally, because a valid adm_access is always needed
> by the call to the new dir_closed() callback.
> (svn_client__get_diff_editor): Track diff-callbacks version.
>
> * subversion/libsvn_client/client.h
> (svn_client__get_diff_editor): Track diff-callbacks version.
>
> * subversion/libsvn_client/diff.c
> (diff_cmd_baton,
> diff_props_changed,
> diff_file_changed,
> diff_file_added,
> diff_file_deleted_with_diff,
> diff_file_deleted_no_diff,
> diff_dir_added,
> diff_dir_deleted: Track diff-callbacks version in comment only.
> (diff_dir_opened,
> diff_dir_closed): New functions.
> (diff_wc_wc,
> diff_repos_repos,
> diff_repos_wc): Track diff-callbacks version in args; track new
> public function names.
> (svn_client_diff4): Track diff-callbacks version internally.
> Include the new functions in the diff-callback initialization.
>
> * subversion/libsvn_client/diff.c
> (merge_props_changed,
> merge_file_changed,
> merge_file_added,
> merge_file_deleted_with_diff,
> merge_file_deleted_no_diff,
> merge_dir_added,
> merge_dir_deleted: Track diff-callbacks version in comment only.
> (merge_dir_opened,
> merge_dir_closed): New functions.
> (drive_merge_report_editor): Track diff-callbacks version in args
>
> ]]]

-- 
Stefan Sperling <stsp_at_elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner