You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Neels J. Hofmeyr" <ne...@elego.de> on 2008/10/29 06:05:34 UTC

Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

Hey tree-conflicts folks,

please review and/or jump right in and fix stuff.

Thanks!
~Neels

neels@tigris.org wrote:
> Author: neels
> Date: Tue Oct 28 23:02:46 2008
> New Revision: 33944
> 
> Log:
> On lightweight branch tc-merge-notify:
> 
> Start off implementing per-victim notification of tree-conflicts during merge.
> There are still a lot of lines saying "Skipped" although it's inside a newly
> raised tree-conflict (should be omitted).
> 
> The cmdline tests are not adapted yet. This stuff is not fully tested yet.
> 
> 
> * subversion/include/svn_wc.h (svn_wc_diff_callbacks3_t): Add a svn_boolean_t
>     *TREE_CONFLICTED argument to all of the callbacks functions, which
>     returns whether the callback wants this node to be notified as
>     tree-conflicted.
> 
> * subversion/libsvn_wc/diff.c
>   (file_diff, directory_elements_diff, report_wc_file_as_added,
>    report_wc_directory_as_added, delete_entry, close_directory,
>    close_file): Pass NULL for new *TREE_CONFLICTED parameter upon calling
>     the svn_wc_diff_callbacks3_t callback functions.
>     ### This needs mending as in repos_diff.c in order to get
>     ### tree-conflicts through to the notifier.
> 
> * subversion/libsvn_client/repos_diff.c 
>   (kind_action_state_t): Add a boolean TREE_CONFLICTED field, obsoleting the
>     name kind_action_state_t which says "it has a kind, and action and a
>     state".
>   (dir_baton, file_baton): Add a boolean TREE_CONFLICTED.
>   (make_dir_baton, make_file_baton): Default TREE_CONFLICTED to FALSE.
>   (delete_entry, add_directory, open_directory, close_file, close_directory):
>     Pass through the tree-conflict state to the notify_func, and change some
>     of the logic around the notify action to not show tree-conflicts as
>     skipped paths.
>   (absent_directory, absent_file): Comment.
>   
> * subversion/libsvn_client/merge.c
>   (merge_props_changed, merge_file_changed, merge_file_added,
>    merge_file_deleted, merge_dir_added, merge_dir_deleted, merge_dir_opened,
>    merge_dir_closed): Apply addition of new callbacks argument called
>     *TREE_CONFLICTED and use it.
>   (single_file_merge_notify, do_file_merge): Use the new callbacks argument
>     called *TREE_CONFLICTED.
>   (merge_cmd_baton_t): Remove obsoleted field TREE_CONFLICTED_DIRS which
>     used to list all dirs that contain tree-conflicts.
>   (tree_conflict): Remove use of TREE_CONFLICTED_DIRS.
>   (add_parent_to_tree_conflicted_dirs, is_tree_conflicted_dir_p): Remove
>     obsoleted functions related to TREE_CONFLICTED_DIRS.
>   (do_merge): Remove use of TREE_CONFLICTED_DIRS.
> 
> * subversion/libsvn_client/diff.c
>   (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, diff_dir_opened, diff_dir_closed):
>     Apply addition of callbacks parameter *TREE_CONFLICTED and ignore it.
>     These are the callbacks for a plain diff, they don't need to report
>     tree-conflicts.
> 
> Modified:
>    branches/tc-merge-notify/subversion/include/svn_wc.h
>    branches/tc-merge-notify/subversion/libsvn_client/diff.c
>    branches/tc-merge-notify/subversion/libsvn_client/merge.c
>    branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c
>    branches/tc-merge-notify/subversion/libsvn_wc/diff.c
> 
> Modified: branches/tc-merge-notify/subversion/include/svn_wc.h
> URL: http://svn.collab.net/viewvc/svn/branches/tc-merge-notify/subversion/include/svn_wc.h?pathrev=33944&r1=33943&r2=33944
> ==============================================================================
> --- branches/tc-merge-notify/subversion/include/svn_wc.h	Tue Oct 28 22:17:24 2008	(r33943)
> +++ branches/tc-merge-notify/subversion/include/svn_wc.h	Tue Oct 28 23:02:46 2008	(r33944)
> @@ -1461,6 +1461,7 @@ typedef struct svn_wc_diff_callbacks3_t
>    svn_error_t *(*file_changed)(svn_wc_adm_access_t *adm_access,
>                                 svn_wc_notify_state_t *contentstate,
>                                 svn_wc_notify_state_t *propstate,
> +                               svn_boolean_t *tree_conflicted,
>                                 const char *path,
>                                 const char *tmpfile1,
>                                 const char *tmpfile2,
> @@ -1491,6 +1492,7 @@ typedef struct svn_wc_diff_callbacks3_t
>    svn_error_t *(*file_added)(svn_wc_adm_access_t *adm_access,
>                               svn_wc_notify_state_t *contentstate,
>                               svn_wc_notify_state_t *propstate,
> +                             svn_boolean_t *tree_conflicted,
>                               const char *path,
>                               const char *tmpfile1,
>                               const char *tmpfile2,
> @@ -1514,6 +1516,7 @@ typedef struct svn_wc_diff_callbacks3_t
>     */
>    svn_error_t *(*file_deleted)(svn_wc_adm_access_t *adm_access,
>                                 svn_wc_notify_state_t *state,
> +                               svn_boolean_t *tree_conflicted,
>                                 const char *path,
>                                 const char *tmpfile1,
>                                 const char *tmpfile2,
> @@ -1528,6 +1531,7 @@ typedef struct svn_wc_diff_callbacks3_t
>     */
>    svn_error_t *(*dir_added)(svn_wc_adm_access_t *adm_access,
>                              svn_wc_notify_state_t *state,
> +                            svn_boolean_t *tree_conflicted,
>                              const char *path,
>                              svn_revnum_t rev,
>                              void *diff_baton);
> @@ -1537,6 +1541,7 @@ typedef struct svn_wc_diff_callbacks3_t
>     */
>    svn_error_t *(*dir_deleted)(svn_wc_adm_access_t *adm_access,
>                                svn_wc_notify_state_t *state,
> +                              svn_boolean_t *tree_conflicted,
>                                const char *path,
>                                void *diff_baton);
>  
> @@ -1552,6 +1557,7 @@ typedef struct svn_wc_diff_callbacks3_t
>     */
>    svn_error_t *(*dir_props_changed)(svn_wc_adm_access_t *adm_access,
>                                      svn_wc_notify_state_t *propstate,
> +                                    svn_boolean_t *tree_conflicted,
>                                      const char *path,
>                                      const apr_array_header_t *propchanges,
>                                      apr_hash_t *original_props,
> @@ -1563,6 +1569,7 @@ typedef struct svn_wc_diff_callbacks3_t
>     *
>     */
>    svn_error_t *(*dir_opened)(svn_wc_adm_access_t *adm_access,
> +                             svn_boolean_t *tree_conflicted,
>                               const char *path,
>                               svn_revnum_t rev,
>                               void *diff_baton);
> @@ -1578,6 +1585,7 @@ typedef struct svn_wc_diff_callbacks3_t
>     */
>    svn_error_t *(*dir_closed)(svn_wc_adm_access_t *adm_access,
>                               svn_wc_notify_state_t *state,
> +                             svn_boolean_t *tree_conflicted,
>                               const char *path,
>                               void *diff_baton);
>  
> 
> Modified: branches/tc-merge-notify/subversion/libsvn_client/diff.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc-merge-notify/subversion/libsvn_client/diff.c?pathrev=33944&r1=33943&r2=33944
> ==============================================================================
> --- branches/tc-merge-notify/subversion/libsvn_client/diff.c	Tue Oct 28 22:17:24 2008	(r33943)
> +++ branches/tc-merge-notify/subversion/libsvn_client/diff.c	Tue Oct 28 23:02:46 2008	(r33944)
> @@ -374,6 +374,7 @@ diff_label(const char *path,
>  static svn_error_t *
>  diff_props_changed(svn_wc_adm_access_t *adm_access,
>                     svn_wc_notify_state_t *state,
> +                   svn_boolean_t *tree_conflicted,
>                     const char *path,
>                     const apr_array_header_t *propchanges,
>                     apr_hash_t *original_props,
> @@ -394,6 +395,8 @@ diff_props_changed(svn_wc_adm_access_t *
>  
>    if (state)
>      *state = svn_wc_notify_state_unknown;
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
>  
>    svn_pool_destroy(subpool);
>    return SVN_NO_ERROR;
> @@ -604,6 +607,7 @@ static svn_error_t *
>  diff_file_changed(svn_wc_adm_access_t *adm_access,
>                    svn_wc_notify_state_t *content_state,
>                    svn_wc_notify_state_t *prop_state,
> +                  svn_boolean_t *tree_conflicted,
>                    const char *path,
>                    const char *tmpfile1,
>                    const char *tmpfile2,
> @@ -620,12 +624,15 @@ diff_file_changed(svn_wc_adm_access_t *a
>                                   tmpfile1, tmpfile2, rev1, rev2,
>                                   mimetype1, mimetype2, diff_baton));
>    if (prop_changes->nelts > 0)
> -    SVN_ERR(diff_props_changed(adm_access, prop_state, path, prop_changes,
> +    SVN_ERR(diff_props_changed(adm_access, prop_state, tree_conflicted,
> +                               path, prop_changes,
>                                 original_props, diff_baton));
>    if (content_state)
>      *content_state = svn_wc_notify_state_unknown;
>    if (prop_state)
>      *prop_state = svn_wc_notify_state_unknown;
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
>    return SVN_NO_ERROR;
>  }
>  
> @@ -638,6 +645,7 @@ static svn_error_t *
>  diff_file_added(svn_wc_adm_access_t *adm_access,
>                  svn_wc_notify_state_t *content_state,
>                  svn_wc_notify_state_t *prop_state,
> +                svn_boolean_t *tree_conflicted,
>                  const char *path,
>                  const char *tmpfile1,
>                  const char *tmpfile2,
> @@ -658,7 +666,8 @@ diff_file_added(svn_wc_adm_access_t *adm
>       user see that *something* happened. */
>    diff_cmd_baton->force_empty = TRUE;
>  
> -  SVN_ERR(diff_file_changed(adm_access, content_state, prop_state, path,
> +  SVN_ERR(diff_file_changed(adm_access, content_state, prop_state,
> +                            tree_conflicted, path,
>                              tmpfile1, tmpfile2,
>                              rev1, rev2,
>                              mimetype1, mimetype2,
> @@ -673,6 +682,7 @@ diff_file_added(svn_wc_adm_access_t *adm
>  static svn_error_t *
>  diff_file_deleted_with_diff(svn_wc_adm_access_t *adm_access,
>                              svn_wc_notify_state_t *state,
> +                            svn_boolean_t *tree_conflicted,
>                              const char *path,
>                              const char *tmpfile1,
>                              const char *tmpfile2,
> @@ -684,7 +694,7 @@ diff_file_deleted_with_diff(svn_wc_adm_a
>    struct diff_cmd_baton *diff_cmd_baton = diff_baton;
>  
>    /* We don't list all the deleted properties. */
> -  return diff_file_changed(adm_access, state, NULL, path,
> +  return diff_file_changed(adm_access, state, NULL, tree_conflicted, path,
>                             tmpfile1, tmpfile2,
>                             diff_cmd_baton->revnum1, diff_cmd_baton->revnum2,
>                             mimetype1, mimetype2,
> @@ -697,6 +707,7 @@ diff_file_deleted_with_diff(svn_wc_adm_a
>  static svn_error_t *
>  diff_file_deleted_no_diff(svn_wc_adm_access_t *adm_access,
>                            svn_wc_notify_state_t *state,
> +                          svn_boolean_t *tree_conflicted,
>                            const char *path,
>                            const char *tmpfile1,
>                            const char *tmpfile2,
> @@ -709,6 +720,8 @@ diff_file_deleted_no_diff(svn_wc_adm_acc
>  
>    if (state)
>      *state = svn_wc_notify_state_unknown;
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
>  
>    return file_printf_from_utf8
>            (diff_cmd_baton->outfile,
> @@ -717,21 +730,22 @@ diff_file_deleted_no_diff(svn_wc_adm_acc
>             path, equal_string);
>  }
>  
> -/* An svn_wc_diff_callbacks3_t function.
> -   For now, let's have 'svn diff' send feedback to the top-level
> -   application, so that something reasonable about directories and
> -   propsets gets printed to stdout. */
> +/* An svn_wc_diff_callbacks3_t function. */
>  static svn_error_t *
>  diff_dir_added(svn_wc_adm_access_t *adm_access,
>                 svn_wc_notify_state_t *state,
> +               svn_boolean_t *tree_conflicted,
>                 const char *path,
>                 svn_revnum_t rev,
>                 void *diff_baton)
>  {
>    if (state)
>      *state = svn_wc_notify_state_unknown;
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
> +  /* Do nothing. */
>  
> -  /* ### todo:  send feedback to app */
>    return SVN_NO_ERROR;
>  }
>  
> @@ -739,11 +753,16 @@ diff_dir_added(svn_wc_adm_access_t *adm_
>  static svn_error_t *
>  diff_dir_deleted(svn_wc_adm_access_t *adm_access,
>                   svn_wc_notify_state_t *state,
> +                 svn_boolean_t *tree_conflicted,
>                   const char *path,
>                   void *diff_baton)
>  {
>    if (state)
>      *state = svn_wc_notify_state_unknown;
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
> +  /* Do nothing. */
>  
>    return SVN_NO_ERROR;
>  }
> @@ -751,10 +770,16 @@ diff_dir_deleted(svn_wc_adm_access_t *ad
>  /* An svn_wc_diff_callbacks3_t function. */
>  static svn_error_t *
>  diff_dir_opened(svn_wc_adm_access_t *adm_access,
> +                svn_boolean_t *tree_conflicted,
>                  const char *path,
>                  svn_revnum_t rev,
>                  void *diff_baton)
>  {
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
> +  /* Do nothing. */
> +
>    return SVN_NO_ERROR;
>  }
>  
> @@ -762,11 +787,16 @@ diff_dir_opened(svn_wc_adm_access_t *adm
>  static svn_error_t *
>  diff_dir_closed(svn_wc_adm_access_t *adm_access,
>                  svn_wc_notify_state_t *state,
> +                svn_boolean_t *tree_conflicted,
>                  const char *path,
>                  void *diff_baton)
>  {
>    if (state)
>      *state = svn_wc_notify_state_unknown;
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
> +  /* Do nothing. */
>  
>    return SVN_NO_ERROR;
>  }
> 
> Modified: branches/tc-merge-notify/subversion/libsvn_client/merge.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc-merge-notify/subversion/libsvn_client/merge.c?pathrev=33944&r1=33943&r2=33944
> ==============================================================================
> --- branches/tc-merge-notify/subversion/libsvn_client/merge.c	Tue Oct 28 22:17:24 2008	(r33943)
> +++ branches/tc-merge-notify/subversion/libsvn_client/merge.c	Tue Oct 28 23:02:46 2008	(r33944)
> @@ -280,9 +280,6 @@ typedef struct merge_cmd_baton_t {
>    svn_ra_session_t *ra_session1;
>    svn_ra_session_t *ra_session2;
>  
> -  /* A list of directories containing tree conflicts. */
> -  apr_array_header_t *tree_conflicted_dirs;
> -
>    /* During the merge, *USE_SLEEP is set to TRUE if a sleep will be required
>       afterwards to ensure timestamp integrity, or unchanged if not. */
>    svn_boolean_t *use_sleep;
> @@ -323,28 +320,6 @@ is_path_conflicted_by_merge(merge_cmd_ba
>            apr_hash_count(merge_b->conflicted_paths) > 0);
>  }
>  
> -/* Add the parent dir of VICTIM_PATH to the merge baton's list of 
> -   tree-conflicted directories, if it isn't already in the list. */
> -static void
> -add_parent_to_tree_conflicted_dirs(merge_cmd_baton_t *merge_b,
> -                                   const char *victim_path)
> -{
> -  const char *dir_path, *old_path;
> -  int i;
> -
> -  dir_path = svn_path_dirname(victim_path, merge_b->pool);
> -
> -  for (i = 0; i < merge_b->tree_conflicted_dirs->nelts; i++)
> -    {
> -      old_path = APR_ARRAY_IDX(merge_b->tree_conflicted_dirs, i,
> -                               const char *);
> -      if (strcmp(old_path, dir_path) == 0)
> -        return;
> -    }
> -
> -  APR_ARRAY_PUSH(merge_b->tree_conflicted_dirs, const char *) = dir_path;
> -}
> -
>  /* Cause a tree conflict notification, and if the merge is not
>   * a dry run, also make the tree conflict persistent. Do nothing
>   * if the merge is record-only.
> @@ -368,12 +343,7 @@ tree_conflict(merge_cmd_baton_t *merge_b
>  {
>    svn_wc_conflict_description_t *conflict;
>  
> -  if (merge_b->record_only)
> -    return SVN_NO_ERROR;
> - 
> -  add_parent_to_tree_conflicted_dirs(merge_b, victim_path);
> -
> -  if (merge_b->dry_run)
> +  if (merge_b->record_only || merge_b->dry_run)
>      return SVN_NO_ERROR;
>  
>    conflict = svn_wc_conflict_description_create_tree(
> @@ -384,26 +354,6 @@ tree_conflict(merge_cmd_baton_t *merge_b
>    return SVN_NO_ERROR;
>  }
>  
> -/* TRUE iff DIR_PATH is in the merge baton's list of tree-conflicted
> -   directories. */
> -static svn_boolean_t
> -is_tree_conflicted_dir_p(merge_cmd_baton_t *merge_b,
> -                         const char *dir_path)
> -{
> -  const char *old_path;
> -  int i;
> -
> -  for (i = 0; i < merge_b->tree_conflicted_dirs->nelts; i++)
> -    {
> -      old_path = APR_ARRAY_IDX(merge_b->tree_conflicted_dirs, i,
> -                               const char *);
> -      if (strcmp(old_path, dir_path) == 0)
> -        return TRUE;
> -    }
> -
> -  return FALSE;
> -}
> -
>  /* Set *HONOR_MERGEINFO and *RECORD_MERGEINFO (if non-NULL) based on the
>     merge being performed as described in MERGE_B.
>  
> @@ -820,6 +770,7 @@ filter_self_referential_mergeinfo(apr_ar
>  static svn_error_t *
>  merge_props_changed(svn_wc_adm_access_t *adm_access,
>                      svn_wc_notify_state_t *state,
> +                    svn_boolean_t *tree_conflicted,
>                      const char *path,
>                      const apr_array_header_t *propchanges,
>                      apr_hash_t *original_props,
> @@ -969,6 +920,7 @@ static svn_error_t *
>  merge_file_changed(svn_wc_adm_access_t *adm_access,
>                     svn_wc_notify_state_t *content_state,
>                     svn_wc_notify_state_t *prop_state,
> +                   svn_boolean_t *tree_conflicted,
>                     const char *mine,
>                     const char *older,
>                     const char *yours,
> @@ -985,6 +937,9 @@ merge_file_changed(svn_wc_adm_access_t *
>    svn_boolean_t merge_required = TRUE;
>    enum svn_wc_merge_outcome_t merge_outcome;
>  
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
>    /* Easy out:  no access baton means there ain't no merge target */
>    if (adm_access == NULL)
>      {
> @@ -992,6 +947,10 @@ merge_file_changed(svn_wc_adm_access_t *
>          *content_state = svn_wc_notify_state_missing;
>        if (prop_state)
>          *prop_state = svn_wc_notify_state_missing;
> +      /* Trying to change a file at a non-existing path.
> +       * Although this is a tree-conflict, it will already have been
> +       * raised by the merge_dir_opened() callback. Not raising additional tree
> +       * conflicts for the child nodes inside. */
>        svn_pool_destroy(subpool);
>        return SVN_NO_ERROR;
>      }
> @@ -1019,6 +978,8 @@ merge_file_changed(svn_wc_adm_access_t *
>                                svn_node_file,
>                                svn_wc_conflict_action_edit,
>                                svn_wc_conflict_reason_missing));
> +        if (tree_conflicted)
> +          *tree_conflicted = TRUE;
>          if (content_state)
>            *content_state = svn_wc_notify_state_missing;
>          if (prop_state)
> @@ -1049,8 +1010,8 @@ merge_file_changed(svn_wc_adm_access_t *
>    /* Do property merge before text merge so that keyword expansion takes
>       into account the new property values. */
>    if (prop_changes->nelts > 0)
> -    SVN_ERR(merge_props_changed(adm_access, prop_state, mine, prop_changes,
> -                                original_props, baton));
> +    SVN_ERR(merge_props_changed(adm_access, prop_state, tree_conflicted,
> +                                mine, prop_changes, original_props, baton));
>    else
>      if (prop_state)
>        *prop_state = svn_wc_notify_state_unchanged;
> @@ -1141,6 +1102,7 @@ static svn_error_t *
>  merge_file_added(svn_wc_adm_access_t *adm_access,
>                   svn_wc_notify_state_t *content_state,
>                   svn_wc_notify_state_t *prop_state,
> +                 svn_boolean_t *tree_conflicted,
>                   const char *mine,
>                   const char *older,
>                   const char *yours,
> @@ -1164,6 +1126,9 @@ merge_file_added(svn_wc_adm_access_t *ad
>    if (prop_state)
>      *prop_state = svn_wc_notify_state_unknown;
>  
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
>    /* Apply the prop changes to a new hash table. */
>    new_props = apr_hash_copy(subpool, original_props);
>    for (i = 0; i < prop_changes->nelts; ++i)
> @@ -1197,6 +1162,10 @@ merge_file_added(svn_wc_adm_access_t *ad
>          }
>        else
>          *content_state = svn_wc_notify_state_missing;
> +      /* Trying to add a file at a non-existing path.
> +       * Although this is a tree-conflict, it will already have been
> +       * raised by the merge_dir_opened() callback. Not raising additional tree
> +       * conflicts for the child nodes inside. */
>        svn_pool_destroy(subpool);
>        return SVN_NO_ERROR;
>      }
> @@ -1223,6 +1192,8 @@ merge_file_added(svn_wc_adm_access_t *ad
>                                    svn_node_file,
>                                    svn_wc_conflict_action_add,
>                                    svn_wc_conflict_reason_obstructed));
> +            if (tree_conflicted)
> +              *tree_conflicted = TRUE;
>              if (content_state)
>                *content_state = svn_wc_notify_state_obstructed;
>              svn_pool_destroy(subpool);
> @@ -1284,6 +1255,8 @@ merge_file_added(svn_wc_adm_access_t *ad
>                              svn_node_file,
>                              svn_wc_conflict_action_add,
>                              svn_wc_conflict_reason_obstructed));
> +      if (tree_conflicted)
> +        *tree_conflicted = TRUE;
>        if (content_state)
>          {
>            /* directory already exists, is it under version control? */
> @@ -1317,6 +1290,8 @@ merge_file_added(svn_wc_adm_access_t *ad
>                                    svn_node_file,
>                                    svn_wc_conflict_action_add,
>                                    svn_wc_conflict_reason_obstructed));
> +            if (tree_conflicted)
> +              *tree_conflicted = TRUE;
>  
>              /* this will make the repos_editor send a 'skipped' message */
>              if (content_state)
> @@ -1340,26 +1315,8 @@ merge_file_added(svn_wc_adm_access_t *ad
>                                        svn_node_file,
>                                        svn_wc_conflict_action_add,
>                                        svn_wc_conflict_reason_obstructed));
> -                /*
> -                 * FIXME: The above doesn't seem to correspond to the
> -                 *        following code which seems to be handling this
> -                 *        as a non-conflict!
> -                 */
> -
> -                /* Indicate that we merge because of an add to handle a
> -                   special case for binary files with no local mods. */
> -                  merge_b->add_necessitated_merge = TRUE;
> -
> -                  SVN_ERR(merge_file_changed(adm_access, content_state,
> -                                             prop_state, mine, older, yours,
> -                                             rev1, rev2,
> -                                             mimetype1, mimetype2,
> -                                             prop_changes, original_props,
> -                                             baton));
> -
> -                /* Reset the state so that the baton can safely be reused
> -                   in subsequent ops occurring during this merge. */
> -                  merge_b->add_necessitated_merge = FALSE;
> +                if (tree_conflicted)
> +                  *tree_conflicted = TRUE;
>                }
>            }
>          break;
> @@ -1440,6 +1397,7 @@ files_same_p(svn_boolean_t *same,
>  static svn_error_t *
>  merge_file_deleted(svn_wc_adm_access_t *adm_access,
>                     svn_wc_notify_state_t *state,
> +                   svn_boolean_t *tree_conflicted,
>                     const char *mine,
>                     const char *older,
>                     const char *yours,
> @@ -1495,6 +1453,8 @@ merge_file_deleted(svn_wc_adm_access_t *
>                                    svn_node_file,
>                                    svn_wc_conflict_action_delete,
>                                    svn_wc_conflict_reason_edited));
> +            if (tree_conflicted)
> +              *tree_conflicted = TRUE;
>  
>              if (state)
>                *state = svn_wc_notify_state_obstructed;
> @@ -1511,6 +1471,8 @@ merge_file_deleted(svn_wc_adm_access_t *
>                              svn_node_file,
>                              svn_wc_conflict_action_delete,
>                              svn_wc_conflict_reason_obstructed));
> +      if (tree_conflicted)
> +        *tree_conflicted = TRUE;
>        if (state)
>          *state = svn_wc_notify_state_obstructed;
>        break;
> @@ -1524,6 +1486,8 @@ merge_file_deleted(svn_wc_adm_access_t *
>                              svn_node_file,
>                              svn_wc_conflict_action_delete,
>                              svn_wc_conflict_reason_deleted));
> +      if (tree_conflicted)
> +        *tree_conflicted = TRUE;
>        if (state)
>          *state = svn_wc_notify_state_missing;
>        break;
> @@ -1541,6 +1505,7 @@ merge_file_deleted(svn_wc_adm_access_t *
>  static svn_error_t *
>  merge_dir_added(svn_wc_adm_access_t *adm_access,
>                  svn_wc_notify_state_t *state,
> +                svn_boolean_t *tree_conflicted,
>                  const char *path,
>                  svn_revnum_t rev,
>                  void *baton)
> @@ -1566,10 +1531,17 @@ merge_dir_added(svn_wc_adm_access_t *adm
>            else
>              *state = svn_wc_notify_state_missing;
>          }
> +      /* Trying to add a directory at a non-existing path.
> +       * Although this is a tree-conflict, it will already have been
> +       * raised by the merge_dir_opened() callback. Not raising additional tree
> +       * conflicts for the child nodes inside. */
>        svn_pool_destroy(subpool);
>        return SVN_NO_ERROR;
>      }
>  
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +
>    child = svn_path_is_child(merge_b->target, path, subpool);
>    SVN_ERR_ASSERT(child != NULL);
>  
> @@ -1650,6 +1622,8 @@ merge_dir_added(svn_wc_adm_access_t *adm
>                                      svn_node_dir,
>                                      svn_wc_conflict_action_add,
>                                      svn_wc_conflict_reason_added));
> +              if (tree_conflicted)
> +                *tree_conflicted = TRUE;
>                if (state)
>                  *state = svn_wc_notify_state_obstructed;
>              }
> @@ -1675,6 +1649,8 @@ merge_dir_added(svn_wc_adm_access_t *adm
>                                  svn_node_dir,
>                                  svn_wc_conflict_action_add,
>                                  svn_wc_conflict_reason_obstructed));
> +          if (tree_conflicted)
> +            *tree_conflicted = TRUE;
>            if (state)
>              *state = svn_wc_notify_state_obstructed;
>          }
> @@ -1695,6 +1671,7 @@ merge_dir_added(svn_wc_adm_access_t *adm
>  static svn_error_t *
>  merge_dir_deleted(svn_wc_adm_access_t *adm_access,
>                    svn_wc_notify_state_t *state,
> +                  svn_boolean_t *tree_conflicted,
>                    const char *path,
>                    void *baton)
>  {
> @@ -1714,10 +1691,17 @@ merge_dir_deleted(svn_wc_adm_access_t *a
>      {
>        if (state)
>          *state = svn_wc_notify_state_missing;
> +      /* Trying to delete a directory at a non-existing path.
> +       * Although this is a tree-conflict, it will already have been
> +       * raised by the merge_dir_opened() callback. Not raising additional tree
> +       * conflicts for the child nodes inside. */
>        svn_pool_destroy(subpool);
>        return SVN_NO_ERROR;
>      }
>  
> +  if (tree_conflicted)
> +    *tree_conflicted = TRUE;
> +
>    /* Find the version-control state of this path */
>    SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, subpool));
>  
> @@ -1765,6 +1749,8 @@ merge_dir_deleted(svn_wc_adm_access_t *a
>                                    svn_node_dir,
>                                    svn_wc_conflict_action_delete,
>                                    svn_wc_conflict_reason_deleted));
> +            if (tree_conflicted)
> +              *tree_conflicted = TRUE;
>            }
>        }
>        break;
> @@ -1780,6 +1766,8 @@ merge_dir_deleted(svn_wc_adm_access_t *a
>                              svn_node_dir,
>                              svn_wc_conflict_action_delete,
>                              svn_wc_conflict_reason_deleted));
> +      if (tree_conflicted)
> +        *tree_conflicted = TRUE;
>        if (state)
>          *state = svn_wc_notify_state_missing;
>        break;
> @@ -1796,55 +1784,52 @@ merge_dir_deleted(svn_wc_adm_access_t *a
>  /* An svn_wc_diff_callbacks3_t function. */
>  static svn_error_t *
>  merge_dir_opened(svn_wc_adm_access_t *adm_access,
> +                 svn_boolean_t *tree_conflicted,
>                   const char *path,
>                   svn_revnum_t rev,
>                   void *baton)
>  {
> -  /* If adm_access == NULL, the tree conflict detection can be skipped,
> -   * because:
> -   *
> -   * adm_access refers to the parent(!) directory of the directory that
> -   * is to be opened. If adm_access == NULL, it means that the parent
> -   * of const char *path does not exist in the current working copy.
> -   *
> -   * We are at arbitrary depth in a directory subtree that does not exist
> -   * in the working copy, but nevertheless in a subtree off an existing
> -   * working copy directory (at least off the working copy "root").
> -   *
> -   * This function has already been called on the first non-existent
> -   * path element of this subtree, which has an existing parent (adm_access
> -   * != NULL), and a tree conflict has been triggered there.
> -   *
> -   * Even if we wanted to report another tree-conflict, there'd be no
> -   * working copy to mark the conflict in. Since the nearest existing parent
> -   * directory is already marked tree-conflicted, we can rest at that.
> -   */
> -  if (adm_access != NULL)
> +  if (tree_conflicted)
> +    *tree_conflicted = FALSE;
> +  
> +  if (adm_access == NULL)
>      {
> -      /* adm_access is not NULL, detect a tree-conflict, if any. */
> +      /* Trying to open a directory at a non-existing path.
> +       * Although this is a tree-conflict, it will already have been
> +       * raised by the merge_dir_opened() callback on the topmost nonexisting
> +       * ancestor, where an adm_access was still present. Not raising
> +       * additional tree conflicts for the child nodes inside. */
> +      /* ### TODO: Verify that this holds true for explicit targets that
> +       * # point deep into a nonexisting subtree. */
> +      return SVN_NO_ERROR;
> +    }
>  
> -      merge_cmd_baton_t *merge_b = baton;
> -      apr_pool_t *subpool = svn_pool_create(merge_b->pool);
> -      svn_node_kind_t kind;
> -      const svn_wc_entry_t *entry;
> -
> -      /* Find the version-control and on-disk states of this path */
> -      SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, subpool));
> -      SVN_ERR(svn_io_check_path(path, &kind, subpool));
> -
> -      /* If we're trying to open a directory that's not a directory,
> -       * raise a tree conflict. */
> -      if (!entry || entry->schedule == svn_wc_schedule_delete
> -          || kind != svn_node_dir)
> -        {
> -          SVN_ERR(tree_conflict(merge_b, adm_access, path,
> -                                svn_node_dir,
> -                                svn_wc_conflict_action_edit,
> -                                svn_wc_conflict_reason_deleted));
> -        }
> +  /* Detect a tree-conflict, if any. */
> +  {
> +    merge_cmd_baton_t *merge_b = baton;
> +    apr_pool_t *subpool = svn_pool_create(merge_b->pool);
> +    svn_node_kind_t kind;
> +    const svn_wc_entry_t *entry;
>  
> -      svn_pool_destroy(subpool);
> -    }
> +    /* Find the version-control and on-disk states of this path */
> +    SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, subpool));
> +    SVN_ERR(svn_io_check_path(path, &kind, subpool));
> +
> +    /* If we're trying to open a directory that's not a directory,
> +     * raise a tree conflict. */
> +    if (!entry || entry->schedule == svn_wc_schedule_delete
> +        || kind != svn_node_dir)
> +      {
> +        SVN_ERR(tree_conflict(merge_b, adm_access, path,
> +                              svn_node_dir,
> +                              svn_wc_conflict_action_edit,
> +                              svn_wc_conflict_reason_deleted));
> +        if (tree_conflicted)
> +          *tree_conflicted = TRUE;
> +      }
> +
> +    svn_pool_destroy(subpool);
> +  }
>  
>    return SVN_NO_ERROR;
>  }
> @@ -1853,21 +1838,13 @@ merge_dir_opened(svn_wc_adm_access_t *ad
>  static svn_error_t *
>  merge_dir_closed(svn_wc_adm_access_t *adm_access,
>                   svn_wc_notify_state_t *state,
> +                 svn_boolean_t *tree_conflicted,
>                   const char *path,
>                   void *baton)
>  {
> -  merge_cmd_baton_t *merge_b = baton;
> -
> -  if (state)
> -    {
> -      /* Check if we encountered any tree conflicts
> -       * in this directory while visiting it.
> -       */
> -      if (is_tree_conflicted_dir_p(merge_b, path))
> -        *state = svn_wc_notify_state_conflicted;
> -      else
> -        *state = svn_wc_notify_state_unknown;
> -    }
> +  /* Nothing to be done.
> +   * The reason why this callback was created is no more.
> +   * Maybe this callback should be removed. */
>  
>    return SVN_NO_ERROR;
>  }
> @@ -3882,6 +3859,7 @@ single_file_merge_notify(void *notify_ba
>                           svn_wc_notify_action_t action,
>                           svn_wc_notify_state_t text_state,
>                           svn_wc_notify_state_t prop_state,
> +                         svn_boolean_t tree_conflicted,
>                           svn_wc_notify_t *header_notification,
>                           svn_boolean_t *header_sent,
>                           apr_pool_t *pool)
> @@ -3890,6 +3868,7 @@ single_file_merge_notify(void *notify_ba
>    notify->kind = svn_node_file;
>    notify->content_state = text_state;
>    notify->prop_state = prop_state;
> +  notify->tree_conflicted = tree_conflicted;
>    if (notify->content_state == svn_wc_notify_state_missing)
>      notify->action = svn_wc_notify_skip;
>  
> @@ -5096,6 +5075,7 @@ do_file_merge(const char *url1,
>    apr_array_header_t *propchanges, *remaining_ranges;
>    svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
>    svn_wc_notify_state_t text_state = svn_wc_notify_state_unknown;
> +  svn_boolean_t tree_conflicted = FALSE;
>    svn_client_ctx_t *ctx = merge_b->ctx;
>    const char *mergeinfo_path;
>    svn_merge_range_t range;
> @@ -5273,6 +5253,7 @@ do_file_merge(const char *url1,
>                /* Delete... */
>                SVN_ERR(merge_file_deleted(adm_access,
>                                           &text_state,
> +                                         &tree_conflicted,
>                                           target_wcpath,
>                                           tmpfile1,
>                                           tmpfile2,
> @@ -5281,12 +5262,14 @@ do_file_merge(const char *url1,
>                                           merge_b));
>                single_file_merge_notify(notify_b, target_wcpath,
>                                         svn_wc_notify_update_delete, text_state,
> -                                       svn_wc_notify_state_unknown, n,
> +                                       svn_wc_notify_state_unknown,
> +                                       tree_conflicted, n,
>                                         &header_sent, subpool);
>  
>                /* ...plus add... */
>                SVN_ERR(merge_file_added(adm_access,
>                                         &text_state, &prop_state,
> +                                       &tree_conflicted,
>                                         target_wcpath,
>                                         tmpfile1,
>                                         tmpfile2,
> @@ -5297,13 +5280,15 @@ do_file_merge(const char *url1,
>                                         merge_b));
>                single_file_merge_notify(notify_b, target_wcpath,
>                                         svn_wc_notify_update_add, text_state,
> -                                       prop_state, n, &header_sent, subpool);
> +                                       prop_state, tree_conflicted,
> +                                       n, &header_sent, subpool);
>                /* ... equals replace. */
>              }
>            else
>              {
>                SVN_ERR(merge_file_changed(adm_access,
>                                           &text_state, &prop_state,
> +                                         &tree_conflicted,
>                                           target_wcpath,
>                                           tmpfile1,
>                                           tmpfile2,
> @@ -5314,7 +5299,8 @@ do_file_merge(const char *url1,
>                                           merge_b));
>                single_file_merge_notify(notify_b, target_wcpath,
>                                         svn_wc_notify_update_update, text_state,
> -                                       prop_state, n, &header_sent, subpool);
> +                                       prop_state, tree_conflicted,
> +                                       n, &header_sent, subpool);
>              }
>  
>            /* Ignore if temporary file not found. It may have been renamed. */
> @@ -6292,8 +6278,6 @@ do_merge(apr_array_header_t *merge_sourc
>        merge_cmd_baton.paths_with_new_mergeinfo = NULL;
>        merge_cmd_baton.ra_session1 = ra_session1;
>        merge_cmd_baton.ra_session2 = ra_session2;
> -      merge_cmd_baton.tree_conflicted_dirs =
> -        apr_array_make(pool, 0, sizeof(const char *));
>  
>        /* Populate the portions of the merge context baton that require
>           an RA session to set, but shouldn't be reset for each iteration. */
> 
> Modified: branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c?pathrev=33944&r1=33943&r2=33944
> ==============================================================================
> --- branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c	Tue Oct 28 22:17:24 2008	(r33943)
> +++ branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c	Tue Oct 28 23:02:46 2008	(r33944)
> @@ -88,6 +88,7 @@ typedef struct kind_action_state_t
>    svn_node_kind_t kind;
>    svn_wc_notify_action_t action;
>    svn_wc_notify_state_t state;
> +  svn_boolean_t tree_conflicted;
>  } kind_action_state_t;
>  
>  /* Directory level baton.
> @@ -96,6 +97,10 @@ struct dir_baton {
>    /* Gets set if the directory is added rather than replaced/unchanged. */
>    svn_boolean_t added;
>  
> +  /* Gets set if this operation caused a tree-conflict on this directory
> +   * (does not show tree-conflicts persisting from before this operation). */
> +  svn_boolean_t tree_conflicted;
> +
>    /* The path of the directory within the repository */
>    const char *path;
>  
> @@ -115,6 +120,7 @@ struct dir_baton {
>    /* The pristine-property list attached to this directory. */
>    apr_hash_t *pristine_props;
>  
> +
>    /* The pool passed in by add_dir, open_dir, or open_root.
>       Also, the pool this dir baton is allocated in. */
>    apr_pool_t *pool;
> @@ -126,6 +132,10 @@ struct file_baton {
>    /* Gets set if the file is added rather than replaced. */
>    svn_boolean_t added;
>  
> +  /* Gets set if this operation caused a tree-conflict on this file
> +   * (does not show tree-conflicts persisting from before this operation). */
> +  svn_boolean_t tree_conflicted;
> +
>    /* The path of the file within the repository */
>    const char *path;
>  
> @@ -181,6 +191,7 @@ make_dir_baton(const char *path,
>    dir_baton->dir_baton = parent_baton;
>    dir_baton->edit_baton = edit_baton;
>    dir_baton->added = added;
> +  dir_baton->tree_conflicted = FALSE;
>    dir_baton->pool = pool;
>    dir_baton->path = apr_pstrdup(pool, path);
>    dir_baton->wcpath = svn_path_join(edit_baton->target, path, pool);
> @@ -205,6 +216,7 @@ make_file_baton(const char *path,
>  
>    file_baton->edit_baton = edit_baton;
>    file_baton->added = added;
> +  file_baton->tree_conflicted = FALSE;
>    file_baton->pool = pool;
>    file_baton->path = apr_pstrdup(pool, path);
>    file_baton->wcpath = svn_path_join(eb->target, path, pool);
> @@ -440,6 +452,7 @@ delete_entry(const char *path,
>    svn_wc_adm_access_t *adm_access;
>    svn_wc_notify_state_t state = svn_wc_notify_state_inapplicable;
>    svn_wc_notify_action_t action = svn_wc_notify_skip;
> +  svn_boolean_t tree_conflicted = FALSE;
>  
>    /* We need to know if this is a directory or a file */
>    SVN_ERR(svn_ra_check_path(eb->ra_session, path, eb->revision, &kind, pool));
> @@ -462,7 +475,7 @@ delete_entry(const char *path,
>              get_file_mime_types(&mimetype1, &mimetype2, b);
>  
>              SVN_ERR(eb->diff_callbacks->file_deleted
> -                    (adm_access, &state, b->wcpath,
> +                    (adm_access, &state, &tree_conflicted, b->wcpath,
>                       b->path_start_revision,
>                       b->path_end_revision,
>                       mimetype1, mimetype2,
> @@ -474,7 +487,7 @@ delete_entry(const char *path,
>          case svn_node_dir:
>            {
>              SVN_ERR(eb->diff_callbacks->dir_deleted
> -                    (adm_access, &state,
> +                    (adm_access, &state, &tree_conflicted,
>                       svn_path_join(eb->target, path, pool),
>                       eb->diff_cmd_baton));
>              break;
> @@ -484,7 +497,8 @@ delete_entry(const char *path,
>          }
>  
>        if ((state != svn_wc_notify_state_missing)
> -          && (state != svn_wc_notify_state_obstructed))
> +          && (state != svn_wc_notify_state_obstructed)
> +          && !tree_conflicted)
>          {
>            action = svn_wc_notify_update_delete;
>            if (eb->dry_run)
> @@ -507,8 +521,9 @@ delete_entry(const char *path,
>        kind_action_state_t *kas = apr_palloc(eb->pool, sizeof(*kas));
>        deleted_path = svn_path_join(eb->target, path, eb->pool);
>        kas->kind = kind;
> -      kas->action = action;
> +      kas->action = tree_conflicted ? svn_wc_notify_update_update : action;
>        kas->state = state;
> +      kas->tree_conflicted = tree_conflicted;
>        apr_hash_set(eb->deleted_paths, deleted_path, APR_HASH_KEY_STRING, kas);
>      }
>    return SVN_NO_ERROR;
> @@ -540,9 +555,12 @@ add_directory(const char *path,
>                            pool));
>  
>    SVN_ERR(eb->diff_callbacks->dir_added
> -          (adm_access, &state, b->wcpath, eb->target_revision,
> -           eb->diff_cmd_baton));
> +          (adm_access, &state, &b->tree_conflicted, b->wcpath,
> +           eb->target_revision, eb->diff_cmd_baton));
>  
> +  if (b->tree_conflicted)
> +    action = svn_wc_notify_update_update;
> +  else
>    if ((state == svn_wc_notify_state_missing)
>        || (state == svn_wc_notify_state_obstructed))
>      action = svn_wc_notify_skip;
> @@ -558,7 +576,8 @@ add_directory(const char *path,
>        if (kas)
>          {
>            svn_wc_notify_action_t new_action;
> -          if (kas->action == svn_wc_notify_update_delete
> +          if ((! kas->tree_conflicted)
> +              && kas->action == svn_wc_notify_update_delete
>                && action == svn_wc_notify_update_add)
>              {
>                is_replace = TRUE;
> @@ -566,10 +585,11 @@ add_directory(const char *path,
>              }
>            else
>              new_action = kas->action;
> -          notify  = svn_wc_create_notify(b->wcpath, new_action, pool);
> +          notify = svn_wc_create_notify(b->wcpath, new_action, pool);
>            notify->kind = kas->kind;
>            notify->content_state = notify->prop_state = kas->state;
>            notify->lock_state = svn_wc_notify_lock_state_inapplicable;
> +          notify->tree_conflicted = kas->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>            apr_hash_set(eb->deleted_paths, b->wcpath,
>                         APR_HASH_KEY_STRING, NULL);
> @@ -579,6 +599,7 @@ add_directory(const char *path,
>          {
>            notify = svn_wc_create_notify(b->wcpath, action, pool);
>            notify->kind = svn_node_dir;
> +          notify->tree_conflicted = b->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>          }
>      }
> @@ -608,7 +629,7 @@ open_directory(const char *path,
>                            pool));
>  
>    return eb->diff_callbacks->dir_opened
> -         (adm_access, b->wcpath, base_revision,
> +         (adm_access, &b->tree_conflicted, b->wcpath, base_revision,
>            b->edit_baton->diff_cmd_baton);
>  }
>  
> @@ -740,9 +761,8 @@ close_file(void *file_baton,
>    svn_wc_adm_access_t *adm_access;
>    svn_error_t *err;
>    svn_wc_notify_action_t action;
> -  svn_wc_notify_state_t
> -    content_state = svn_wc_notify_state_unknown,
> -    prop_state = svn_wc_notify_state_unknown;
> +  svn_wc_notify_state_t content_state = svn_wc_notify_state_unknown;
> +  svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
>  
>    err = get_parent_access(&adm_access, eb->adm_access,
>                            b->wcpath, eb->dry_run, b->pool);
> @@ -753,12 +773,16 @@ close_file(void *file_baton,
>        /* If the file path doesn't exist, then send a 'skipped' notification. */
>        if (eb->notify_func)
>          {
> -          svn_wc_notify_t *notify = svn_wc_create_notify(b->wcpath,
> -                                                         svn_wc_notify_skip,
> -                                                         pool);
> +          svn_wc_notify_t *notify = svn_wc_create_notify(
> +                                      b->wcpath,
> +                                      b->tree_conflicted
> +                                       ? svn_wc_notify_update_update
> +                                       : svn_wc_notify_skip,
> +                                      pool);
>            notify->kind = svn_node_file;
>            notify->content_state = svn_wc_notify_state_missing;
>            notify->prop_state = prop_state;
> +          notify->tree_conflicted = b->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>          }
>  
> @@ -775,7 +799,7 @@ close_file(void *file_baton,
>  
>        if (b->added)
>          SVN_ERR(eb->diff_callbacks->file_added
> -                (adm_access, &content_state, &prop_state,
> +                (adm_access, &content_state, &prop_state, &b->tree_conflicted,
>                   b->wcpath,
>                   b->path_end_revision ? b->path_start_revision : NULL,
>                   b->path_end_revision,
> @@ -786,7 +810,7 @@ close_file(void *file_baton,
>                   b->edit_baton->diff_cmd_baton));
>        else
>          SVN_ERR(eb->diff_callbacks->file_changed
> -                (adm_access, &content_state, &prop_state,
> +                (adm_access, &content_state, &prop_state, &b->tree_conflicted,
>                   b->wcpath,
>                   b->path_end_revision ? b->path_start_revision : NULL,
>                   b->path_end_revision,
> @@ -798,8 +822,10 @@ close_file(void *file_baton,
>      }
>  
>  
> -  if ((content_state == svn_wc_notify_state_missing)
> -      || (content_state == svn_wc_notify_state_obstructed))
> +  if (b->tree_conflicted)
> +    action = svn_wc_notify_update_update;
> +  else if ((content_state == svn_wc_notify_state_missing)
> +            || (content_state == svn_wc_notify_state_obstructed))
>      action = svn_wc_notify_skip;
>    else if (b->added)
>      action = svn_wc_notify_update_add;
> @@ -815,7 +841,8 @@ close_file(void *file_baton,
>        if (kas)
>          {
>            svn_wc_notify_action_t new_action;
> -          if (kas->action == svn_wc_notify_update_delete
> +          if ((! kas->tree_conflicted)
> +              && kas->action == svn_wc_notify_update_delete
>                && action == svn_wc_notify_update_add)
>              {
>                is_replace = TRUE;
> @@ -827,6 +854,7 @@ close_file(void *file_baton,
>            notify->kind = kas->kind;
>            notify->content_state = notify->prop_state = kas->state;
>            notify->lock_state = svn_wc_notify_lock_state_inapplicable;
> +          notify->tree_conflicted = kas->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>            apr_hash_set(eb->deleted_paths, b->wcpath,
>                         APR_HASH_KEY_STRING, NULL);
> @@ -838,6 +866,7 @@ close_file(void *file_baton,
>            notify->kind = svn_node_file;
>            notify->content_state = content_state;
>            notify->prop_state = prop_state;
> +          notify->tree_conflicted = b->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>          }
>      }
> @@ -870,10 +899,15 @@ close_directory(void *dir_baton,
>        if (eb->notify_func)
>          {
>            svn_wc_notify_t *notify
> -            = svn_wc_create_notify(b->wcpath, svn_wc_notify_skip, pool);
> +            = svn_wc_create_notify(b->wcpath,
> +                                   b->tree_conflicted
> +                                     ? svn_wc_notify_update_update
> +                                     : svn_wc_notify_skip,
> +                                   pool);
>            notify->kind = svn_node_dir;
>            notify->content_state = notify->prop_state
>              = svn_wc_notify_state_missing;
> +          notify->tree_conflicted = b->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>          }
>        svn_error_clear(err);
> @@ -887,13 +921,13 @@ close_directory(void *dir_baton,
>       have been recognised as added, in which case they cannot conflict. */
>    if ((b->propchanges->nelts > 0) && (! eb->dry_run || adm_access))
>      SVN_ERR(eb->diff_callbacks->dir_props_changed
> -            (adm_access, &prop_state,
> +            (adm_access, &prop_state, &b->tree_conflicted,
>               b->wcpath,
>               b->propchanges, b->pristine_props,
>               b->edit_baton->diff_cmd_baton));
>  
>    SVN_ERR(eb->diff_callbacks->dir_closed
> -          (adm_access, &content_state,
> +          (adm_access, &content_state, &b->tree_conflicted,
>             b->wcpath, b->edit_baton->diff_cmd_baton));
>  
>    /* ### Don't notify added directories as they triggered notification
> @@ -910,10 +944,11 @@ close_directory(void *dir_baton,
>            const void *deleted_path;
>            kind_action_state_t *kas;
>            apr_hash_this(hi, &deleted_path, NULL, (void *)&kas);
> -          notify  = svn_wc_create_notify(deleted_path, kas->action, pool);
> +          notify = svn_wc_create_notify(deleted_path, kas->action, pool);
>            notify->kind = kas->kind;
>            notify->content_state = notify->prop_state = kas->state;
>            notify->lock_state = svn_wc_notify_lock_state_inapplicable;
> +          notify->tree_conflicted = kas->tree_conflicted;
>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>            apr_hash_set(eb->deleted_paths, deleted_path,
>                         APR_HASH_KEY_STRING, NULL);
> @@ -930,6 +965,7 @@ close_directory(void *dir_baton,
>        
>        notify->prop_state = prop_state;
>        notify->lock_state = svn_wc_notify_lock_state_inapplicable;
> +      notify->tree_conflicted = b->tree_conflicted;
>        (*eb->notify_func)(eb->notify_baton, notify, pool);
>      }
>  
> @@ -993,6 +1029,8 @@ absent_directory(const char *path,
>    struct dir_baton *pb = parent_baton;
>    struct edit_baton *eb = pb->edit_baton;
>  
> +  /* ### TODO: Raise a tree-conflict?? I sure hope not.*/
> +
>    if (eb->notify_func)
>      {
>        svn_wc_notify_t *notify
> @@ -1019,6 +1057,8 @@ absent_file(const char *path,
>    struct dir_baton *pb = parent_baton;
>    struct edit_baton *eb = pb->edit_baton;
>  
> +  /* ### TODO: Raise a tree-conflict?? I sure hope not.*/
> +
>    if (eb->notify_func)
>      {
>        svn_wc_notify_t *notify
> 
> Modified: branches/tc-merge-notify/subversion/libsvn_wc/diff.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc-merge-notify/subversion/libsvn_wc/diff.c?pathrev=33944&r1=33943&r2=33944
> ==============================================================================
> --- branches/tc-merge-notify/subversion/libsvn_wc/diff.c	Tue Oct 28 22:17:24 2008	(r33943)
> +++ branches/tc-merge-notify/subversion/libsvn_wc/diff.c	Tue Oct 28 23:02:46 2008	(r33944)
> @@ -586,7 +586,7 @@ file_diff(struct dir_baton *dir_baton,
>                                  adm_access, path, pool));
>  
>        SVN_ERR(dir_baton->edit_baton->callbacks->file_deleted
> -              (NULL, NULL, path,
> +              (NULL, NULL, NULL, path,
>                 textbase,
>                 empty_file,
>                 base_mimetype,
> @@ -610,7 +610,7 @@ file_diff(struct dir_baton *dir_baton,
>                 pool));
>  
>        SVN_ERR(dir_baton->edit_baton->callbacks->file_added
> -              (NULL, NULL, NULL, path,
> +              (NULL, NULL, NULL, NULL, path,
>                 empty_file,
>                 translated,
>                 0, entry->revision,
> @@ -648,7 +648,7 @@ file_diff(struct dir_baton *dir_baton,
>                                         adm_access, path, pool));
>  
>            SVN_ERR(dir_baton->edit_baton->callbacks->file_changed
> -                  (NULL, NULL, NULL,
> +                  (NULL, NULL, NULL, NULL,
>                     path,
>                     modified ? textbase : NULL,
>                     translated,
> @@ -727,7 +727,7 @@ directory_elements_diff(struct dir_baton
>                                          dir_baton->pool));
>  
>            SVN_ERR(dir_baton->edit_baton->callbacks->dir_props_changed
> -                  (adm_access, NULL,
> +                  (adm_access, NULL, NULL,
>                     dir_baton->path,
>                     propchanges, baseprops,
>                     dir_baton->edit_baton->callback_baton));
> @@ -893,7 +893,7 @@ report_wc_file_as_added(struct dir_baton
>             pool));
>  
>    return eb->callbacks->file_added
> -          (adm_access, NULL, NULL,
> +          (adm_access, NULL, NULL, NULL,
>             path,
>             empty_file, translated_file,
>             0, entry->revision,
> @@ -947,7 +947,7 @@ report_wc_directory_as_added(struct dir_
>  
>        if (propchanges->nelts > 0)
>          SVN_ERR(eb->callbacks->dir_props_changed
> -                (adm_access, NULL,
> +                (adm_access, NULL, NULL,
>                   dir_baton->path,
>                   propchanges, emptyprops,
>                   eb->callback_baton));
> @@ -1104,7 +1104,7 @@ delete_entry(const char *path,
>                                      adm_access, full_path, pool));
>  
>            SVN_ERR(pb->edit_baton->callbacks->file_deleted
> -                  (NULL, NULL, full_path,
> +                  (NULL, NULL, NULL, full_path,
>                     textbase,
>                     empty_file,
>                     base_mimetype,
> @@ -1243,7 +1243,7 @@ close_directory(void *dir_baton,
>          reverse_propchanges(originalprops, b->propchanges, b->pool);
>  
>        SVN_ERR(b->edit_baton->callbacks->dir_props_changed
> -              (NULL, NULL,
> +              (NULL, NULL, NULL,
>                 b->path,
>                 b->propchanges,
>                 originalprops,
> @@ -1453,7 +1453,7 @@ close_file(void *file_baton,
>      {
>        if (eb->reverse_order)
>          return b->edit_baton->callbacks->file_added
> -                (NULL, NULL, NULL, b->path,
> +                (NULL, NULL, NULL, NULL, b->path,
>                   empty_file,
>                   temp_file_path,
>                   0,
> @@ -1465,7 +1465,7 @@ close_file(void *file_baton,
>                   b->edit_baton->callback_baton);
>        else
>            return b->edit_baton->callbacks->file_deleted
> -                  (NULL, NULL, b->path,
> +                  (NULL, NULL, NULL, b->path,
>                     temp_file_path,
>                     empty_file,
>                     repos_mimetype,
> @@ -1524,7 +1524,7 @@ close_file(void *file_baton,
>          reverse_propchanges(originalprops, b->propchanges, b->pool);
>  
>        SVN_ERR(b->edit_baton->callbacks->file_changed
> -              (NULL, NULL, NULL,
> +              (NULL, NULL, NULL, NULL,
>                 b->path,
>                 eb->reverse_order ? localfile : temp_file_path,
>                 eb->reverse_order ? temp_file_path : localfile,
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
> 


Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

Posted by Neels J Hofmeyr <ne...@elego.de>.
> Quoting Julian Foad <ju...@btopenworld.com>:
> 
>> On Thu, 2008-10-30 at 03:10 +0100, Neels J. Hofmeyr wrote:
>>>
>>> Stephen Butler wrote:
>>> > I propose that we remove the (new) tree_conflicted field from
>>> > svn_wc_notify_t, and add one or more notify-actions instead.
>>> > At the same time, change the notify() function in the client to
>>> > accept the new tree conflict notifications.
>>>
>>> Hey, that's actually a very good idea. I did this thinking that it
>>> would be
>>> good to have the usual notification alongside the conflict. But that is
>>> better solved with two new columns, as we discussed elsewhere.

As discussed, I have a patch simmering that, in svn_wc_notify_t, changes the
separate boolean TREE_CONFLICTED to a new svn_wc_notify_action_t called
svn_wc_notify_tree_conflicted.

I was hoping get it through quickly but it's bedtime now and it so far only
breaks everything. Haven't investigated thoroughly why that is so, yet.


>> There is already a "skip" notification defined:
>>
>>   /** The type of action occurring. */
>>   typedef enum svn_wc_notify_action_t
>>   {
>>   [...]
>>     /** Skipping a path. */
>>     svn_wc_notify_skip,
>>   [...]
>>
>> Shouldn't we be using svn_wc_notify_skip as the "action occurring", and
>> set the "content_state" to "conflicted" or the "tree_conflicted" flag to
>> true to indicate that the reason is a conflict?

Hmm... I overlooked this. We don't have consensus on using the action to
notify a tree-conflict? Well, it doesn't make much difference really. But it
looks so far as if using the action fits pretty well and avoids some
conditions going true even though nothing happened as they expected
(comparing action to *_add to record something in a list about added items).
We could adapt the conditions and stuff, but it looks like most stuff
becomes simpler with a separate svn_wc_notify_tree_conflict action.

~Neels

Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

Posted by Stephen Butler <sb...@elego.de>.
Quoting Julian Foad <ju...@btopenworld.com>:

> On Thu, 2008-10-30 at 03:10 +0100, Neels J. Hofmeyr wrote:
>>
>> Stephen Butler wrote:
>> > Quoting "Neels J. Hofmeyr" <ne...@elego.de>:
>> >
>> >> Hey tree-conflicts folks,
>> >>
>> >> please review and/or jump right in and fix stuff.
>> >>
>> >> Thanks!
>> >> ~Neels
>> [...]
>> > But I think there's some extra changes needed to support
>> > skipping the victims.  We need to separate tree conflict
>> > notifications from all of the others.
>> >
>> > I propose that we remove the (new) tree_conflicted field from
>> > svn_wc_notify_t, and add one or more notify-actions instead.
>> > At the same time, change the notify() function in the client to
>> > accept the new tree conflict notifications.
>>
>> Hey, that's actually a very good idea. I did this thinking that it would be
>> good to have the usual notification alongside the conflict. But that is
>> better solved with two new columns, as we discussed elsewhere.

>> > Comments?
>>
>> About this
>> >     case svn_wc_notify_conflict_skip:
>> that prints
>> >            (pool, _("Skipped conflicted path '%s'\n"), path_local)))
>>
>> Where is this going to be used, exactly? For persisting tree-conflicts? Not
>> for nodes inside a newly tree-conflicted directory, I presume.

That's correct.  We'll be silent inside a tree conflicted dir that
we've already notified the user about.

>
> There is already a "skip" notification defined:
>
>   /** The type of action occurring. */
>   typedef enum svn_wc_notify_action_t
>   {
>   [...]
>     /** Skipping a path. */
>     svn_wc_notify_skip,
>   [...]
>
> Shouldn't we be using svn_wc_notify_skip as the "action occurring", and
> set the "content_state" to "conflicted" or the "tree_conflicted" flag to
> true to indicate that the reason is a conflict?

Yes, it would be clearer to have just one skip action.  BTW we
should get rid of the "Skipped missing target" output, because
that will always be a tree conflict.

Steve

-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

Posted by Julian Foad <ju...@btopenworld.com>.
On Thu, 2008-10-30 at 03:10 +0100, Neels J. Hofmeyr wrote:
> 
> Stephen Butler wrote:
> > Quoting "Neels J. Hofmeyr" <ne...@elego.de>:
> > 
> >> Hey tree-conflicts folks,
> >>
> >> please review and/or jump right in and fix stuff.
> >>
> >> Thanks!
> >> ~Neels
> [...]
> > But I think there's some extra changes needed to support
> > skipping the victims.  We need to separate tree conflict
> > notifications from all of the others.
> > 
> > I propose that we remove the (new) tree_conflicted field from
> > svn_wc_notify_t, and add one or more notify-actions instead.
> > At the same time, change the notify() function in the client to
> > accept the new tree conflict notifications.
> 
> Hey, that's actually a very good idea. I did this thinking that it would be
> good to have the usual notification alongside the conflict. But that is
> better solved with two new columns, as we discussed elsewhere.
> 
> > 
> > Specifically, in notify(), we should add a couple of new cases,
> > and remove the 'C' in the fourth column from all other cases
> > that have it.  E.g.:
> > 
> >   switch (n->action)
> >     {
> >     case svn_wc_notify_tree_conflict:
> >       nb->in_external ? nb->ext_tree_conflicts++
> >                       : nb->tree_conflicts++;
> >       if ((err = svn_cmdline_printf(pool, "   C %s\n", path_local)))
> >         goto print_error;
> >       break;
> > 
> >     case svn_wc_notify_conflict_skip:
> >       nb->in_external ? nb->ext_skipped_paths++
> >                       : nb->skipped_paths++;
> >       if ((err = svn_cmdline_printf
> >            (pool, _("Skipped conflicted path '%s'\n"), path_local)))
> >         goto print_error;
> >       break;
> > 
> > 
> > The current code (for update, too) mixes tree conflict output
> > with other output.
> > 
> > This will affect repos_diff.c.  Callbacks like the following
> > should do tree conflict notification earlier, before skipping
> > the rest of the callback (including the notify code below).
> > 
> > The notify code at the end of the repos_diff callbacks
> > shouldn't set the tree_conflicted field, because it won't
> > be in svn_wc_notify_t anymore.
> > 
> >   >> +          notify->tree_conflicted = b->tree_conflicted;
> 
> Huh? Where is that?
> Oh, you mean when removing that field. Naturally.
> 
> > 
> > Comments?
> 
> About this
> >     case svn_wc_notify_conflict_skip:
> that prints
> >            (pool, _("Skipped conflicted path '%s'\n"), path_local)))
> 
> Where is this going to be used, exactly? For persisting tree-conflicts? Not
> for nodes inside a newly tree-conflicted directory, I presume.

There is already a "skip" notification defined:

  /** The type of action occurring. */
  typedef enum svn_wc_notify_action_t
  {
  [...]
    /** Skipping a path. */
    svn_wc_notify_skip,
  [...]

Shouldn't we be using svn_wc_notify_skip as the "action occurring", and
set the "content_state" to "conflicted" or the "tree_conflicted" flag to
true to indicate that the reason is a conflict?

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

Posted by "Neels J. Hofmeyr" <ne...@elego.de>.

Stephen Butler wrote:
> Quoting "Neels J. Hofmeyr" <ne...@elego.de>:
> 
>> Hey tree-conflicts folks,
>>
>> please review and/or jump right in and fix stuff.
>>
>> Thanks!
>> ~Neels
[...]
> But I think there's some extra changes needed to support
> skipping the victims.  We need to separate tree conflict
> notifications from all of the others.
> 
> I propose that we remove the (new) tree_conflicted field from
> svn_wc_notify_t, and add one or more notify-actions instead.
> At the same time, change the notify() function in the client to
> accept the new tree conflict notifications.

Hey, that's actually a very good idea. I did this thinking that it would be
good to have the usual notification alongside the conflict. But that is
better solved with two new columns, as we discussed elsewhere.

> 
> Specifically, in notify(), we should add a couple of new cases,
> and remove the 'C' in the fourth column from all other cases
> that have it.  E.g.:
> 
>   switch (n->action)
>     {
>     case svn_wc_notify_tree_conflict:
>       nb->in_external ? nb->ext_tree_conflicts++
>                       : nb->tree_conflicts++;
>       if ((err = svn_cmdline_printf(pool, "   C %s\n", path_local)))
>         goto print_error;
>       break;
> 
>     case svn_wc_notify_conflict_skip:
>       nb->in_external ? nb->ext_skipped_paths++
>                       : nb->skipped_paths++;
>       if ((err = svn_cmdline_printf
>            (pool, _("Skipped conflicted path '%s'\n"), path_local)))
>         goto print_error;
>       break;
> 
> 
> The current code (for update, too) mixes tree conflict output
> with other output.
> 
> This will affect repos_diff.c.  Callbacks like the following
> should do tree conflict notification earlier, before skipping
> the rest of the callback (including the notify code below).
> 
> The notify code at the end of the repos_diff callbacks
> shouldn't set the tree_conflicted field, because it won't
> be in svn_wc_notify_t anymore.
> 
>   >> +          notify->tree_conflicted = b->tree_conflicted;

Huh? Where is that?
Oh, you mean when removing that field. Naturally.

> 
> Comments?

About this
>     case svn_wc_notify_conflict_skip:
that prints
>            (pool, _("Skipped conflicted path '%s'\n"), path_local)))

Where is this going to be used, exactly? For persisting tree-conflicts? Not
for nodes inside a newly tree-conflicted directory, I presume.

~Neels


Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

Posted by Stephen Butler <sb...@elego.de>.
Quoting "Neels J. Hofmeyr" <ne...@elego.de>:

> Hey tree-conflicts folks,
>
> please review and/or jump right in and fix stuff.
>
> Thanks!
> ~Neels
>
> neels@tigris.org wrote:
>> Author: neels
>> Date: Tue Oct 28 23:02:46 2008
>> New Revision: 33944
>>
>> Log:
>> On lightweight branch tc-merge-notify:
>>
>> Start off implementing per-victim notification of tree-conflicts   
>> during merge.

Hi Neels,

The changes to merge.c and diff.c look good to me, on a quick
overview.

But I think there's some extra changes needed to support
skipping the victims.  We need to separate tree conflict
notifications from all of the others.

I propose that we remove the (new) tree_conflicted field from
svn_wc_notify_t, and add one or more notify-actions instead.
At the same time, change the notify() function in the client to
accept the new tree conflict notifications.

Specifically, in notify(), we should add a couple of new cases,
and remove the 'C' in the fourth column from all other cases
that have it.  E.g.:

   switch (n->action)
     {
     case svn_wc_notify_tree_conflict:
       nb->in_external ? nb->ext_tree_conflicts++
                       : nb->tree_conflicts++;
       if ((err = svn_cmdline_printf(pool, "   C %s\n", path_local)))
         goto print_error;
       break;

     case svn_wc_notify_conflict_skip:
       nb->in_external ? nb->ext_skipped_paths++
                       : nb->skipped_paths++;
       if ((err = svn_cmdline_printf
            (pool, _("Skipped conflicted path '%s'\n"), path_local)))
         goto print_error;
       break;


The current code (for update, too) mixes tree conflict output
with other output.

This will affect repos_diff.c.  Callbacks like the following
should do tree conflict notification earlier, before skipping
the rest of the callback (including the notify code below).

The notify code at the end of the repos_diff callbacks
shouldn't set the tree_conflicted field, because it won't
be in svn_wc_notify_t anymore.

   >> +          notify->tree_conflicted = b->tree_conflicted;

Comments?

Steve


==============================================================================
>> ---   
>> branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c	Tue   
>> Oct 28 22:17:24 2008	(r33943)
>> +++   
>> branches/tc-merge-notify/subversion/libsvn_client/repos_diff.c	Tue   
>> Oct 28 23:02:46 2008	(r33944)

[...]

>> @@ -566,10 +585,11 @@ add_directory(const char *path,
>>              }
>>            else
>>              new_action = kas->action;
>> -          notify  = svn_wc_create_notify(b->wcpath, new_action, pool);
>> +          notify = svn_wc_create_notify(b->wcpath, new_action, pool);
>>            notify->kind = kas->kind;
>>            notify->content_state = notify->prop_state = kas->state;
>>            notify->lock_state = svn_wc_notify_lock_state_inapplicable;
>> +          notify->tree_conflicted = kas->tree_conflicted;
>>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>>            apr_hash_set(eb->deleted_paths, b->wcpath,
>>                         APR_HASH_KEY_STRING, NULL);
>> @@ -579,6 +599,7 @@ add_directory(const char *path,
>>          {
>>            notify = svn_wc_create_notify(b->wcpath, action, pool);
>>            notify->kind = svn_node_dir;
>> +          notify->tree_conflicted = b->tree_conflicted;
>>            (*eb->notify_func)(eb->notify_baton, notify, pool);
>>          }
>>      }


-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org