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

Re: multiple merge notifications

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2007-05-17 23:27:32 CEST

I'm getting ready to leave on vacation, I'll be mostly offline through
May 30th. Unfortunately, that means I need to hand off my patch in
progress. I'm posting it to the list here, in the hopes that pburba,
dlr, malcolmr (or anyone else) can pick it up and find the Right
Solution. If you can get tests to pass elegantly, then please commit
this! Don't wait for my approval.

It took one day to write the multi-merge notification. It works just fine.

However, it's taken me a week to mess with our test suite. The output
change basically breaks every test which invokes 'svn merge' on a
single file. Why? Because our tests merge to single files by calling
run_and_verify_svn(), rather than run_and_verify_merge(). Why?
Because run_and_verify_merge() can't handle single-file merges. Thus,
up to now, we've been routing around this problem by passing *literal*
expected-output lines to run_and_verify_svn(..."merge"...). Now we're
being bitten by this workaround, because our tests are no longer
adaptable or flexible to UI changes. We're supposed to be building
parse-trees, rather than passing literal expected-output.

So, I've been experimenting with strategies to fix the core of the
problem: let's teach run_and_verify_merge() to work on single files.
That means teaching build_tree_from_wc() to work on single file
targets. That means fixing our tree comparison functions to stop
assuming everything is a directory. It's a cascade of hairy changes
that has been stumping me.

If someone has a better idea here, please give it a shot!

Here's my patch.

-------------------

Have 'svn merge' notify client when it merges each range.

* subversion/include/svn_wc.h
 (svn_wc_notify_action_t): new 'svn_wc_notify_merge_begin' action.
 (svn_wc_notify_t): new 'merge_range' field to describe range being merged.

* subversion/libsvn_wc/util.c
 (svn_wc_create_notify): have constructor explicitly NULL new field.
 (svn_wc_dup_notify): duplicate the new merge_range_t field.

* subversion/libsvn_client/merge.c
 (do_merge, do_single_file_merge): send new merge_begin notification.

* subversion/svn/notify.c
 (notify): print the merge_begin notification, describe range being merged.

* subversion/tests/cmdline/update_tests.py: make test #36 call
  run_and_verify_merge() on single file target, rather than run_and_verify_svn()

* subversion/tests/cmdline/svntest/tree.py
  (build_tree_from_wc): tweak to work on single files. (???)

Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 25052)
+++ subversion/include/svn_wc.h (working copy)
@@ -733,8 +733,12 @@
   svn_wc_notify_changelist_clear,

   /** Failed to update a path's changelist association. @since New in 1.5. */
- svn_wc_notify_changelist_failed
+ svn_wc_notify_changelist_failed,

+ /** A merge operation (to path) has begun. See @c merge_range in
+ @c svn_wc_notify_t. @since New in 1.5 */
+ svn_wc_notify_merge_begin,
+
 } svn_wc_notify_action_t;

@@ -840,6 +844,9 @@
   /** When @c action is @c svn_wc_notify_changelist_add or name. In all other
    * cases, it is @c NULL. */
   const char *changelist_name;
+ /** When @c action is @c svn_wc_notify_merge_begin. In all other
+ cases, it is @c NULL. */
+ svn_merge_range_t *merge_range;
   /* NOTE: Add new fields at the end to preserve binary compatibility.
      Also, if you add fields here, you have to update svn_wc_create_notify
      and svn_wc_dup_notify. */
Index: subversion/libsvn_wc/util.c
===================================================================
--- subversion/libsvn_wc/util.c (revision 25052)
+++ subversion/libsvn_wc/util.c (working copy)
@@ -127,6 +127,7 @@
   ret->lock_state = svn_wc_notify_lock_state_unknown;
   ret->revision = SVN_INVALID_REVNUM;
   ret->changelist_name = NULL;
+ ret->merge_range = NULL;

   return ret;
 }
@@ -161,6 +162,8 @@
     }
   if (ret->changelist_name)
     ret->changelist_name = apr_pstrdup(pool, ret->changelist_name);
+ if (ret->merge_range)
+ ret->merge_range = svn_merge_range_dup(ret->merge_range, pool);

   return ret;
 }
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 25052)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -1754,6 +1754,7 @@
   svn_boolean_t inherited;
   svn_opt_revision_t assumed_initial_revision1, assumed_initial_revision2;
   apr_size_t target_count, merge_target_count;
+ apr_pool_t *subpool = svn_pool_create(pool);

   /* Establish first RA session to initial_URL1. */
   SVN_ERR(svn_client__open_ra_session_internal(&ra_session, initial_URL1, NULL,
@@ -1890,16 +1891,23 @@
      may create holes in range to merge. Loop over the revision
      ranges we have left to merge, getting an editor for each range,
      and applying its delta. */
- /* ### FIXME: Handle notification callbacks for multiple merges into
- ### a single versioned resource. */
   for (i = 0; i < remaining_ranges->nelts; i++)
     {
+ svn_wc_notify_t *notify;
+
       /* When using this merge range, account for the exclusivity of
          its low value (which is indicated by this operation being a
          merge vs. revert). */
       svn_merge_range_t *r = APR_ARRAY_IDX(remaining_ranges, i,
                                            svn_merge_range_t *);

+ svn_pool_clear(subpool);
+
+ notify = svn_wc_create_notify(target_wcpath, svn_wc_notify_merge_begin,
+ subpool);
+ notify->merge_range = r;
+ notification_receiver(&notify_b, notify, subpool);
+
       /* We must avoid subsequent merges to files which are already in
          conflict, as subsequent merges might overlap with the
          conflict markers in the file (or worse, be completely inside
@@ -1922,7 +1930,7 @@
                                           ctx->cancel_baton,
                                           &diff_editor,
                                           &diff_edit_baton,
- pool));
+ subpool));

       SVN_ERR(svn_ra_do_diff3(ra_session,
                               &reporter, &report_baton,
@@ -1932,11 +1940,11 @@
                               ignore_ancestry,
                               TRUE, /* text_deltas */
                               URL2,
- diff_editor, diff_edit_baton, pool));
+ diff_editor, diff_edit_baton, subpool));

       SVN_ERR(reporter->set_path(report_baton, "",
                                  is_revert ? r->start : r->start - 1,
- depth, FALSE, NULL, pool));
+ depth, FALSE, NULL, subpool));
       if (notify_b.same_urls &&
           children_with_mergeinfo && children_with_mergeinfo->nelts > 0)
         {
@@ -1962,14 +1970,14 @@
                     (target_wcpath_len ? target_wcpath_len + 1 : 0);
                   SVN_ERR(reporter->set_path(report_baton, child_repos_path,
                                              is_revert ? r->end - 1 : r->end,
- depth, FALSE, NULL, pool));
+ depth, FALSE, NULL, subpool));
                 }
             }
         }

       /* ### TODO: Print revision range separator line. */

- SVN_ERR(reporter->finish_report(report_baton, pool));
+ SVN_ERR(reporter->finish_report(report_baton, subpool));

       /* ### LATER: Give the caller a shot at resolving any conflicts
          ### we've detected. If the conflicts are not resolved, abort
@@ -1984,18 +1992,18 @@
                  merges, minus any unresolved conflicts and skips. */
               apr_hash_t *merges;
               SVN_ERR(determine_merges_performed(&merges, target_wcpath, r,
- &notify_b, pool));
+ &notify_b, subpool));

               /* If merge target has inherited mergeinfo set it before
                  recording the first merge range. */
               if (!i && inherited)
                 SVN_ERR(svn_client__record_wc_merge_info(target_wcpath,
                                                          target_mergeinfo,
- adm_access, pool));
+ adm_access, subpool));

               SVN_ERR(update_wc_merge_info(target_wcpath, entry, rel_path,
                                            merges, is_revert, adm_access,
- ctx, pool));
+ ctx, subpool));
             }

           /* Clear the notification counter and list of skipped paths
@@ -2096,6 +2104,7 @@
   svn_boolean_t inherited = FALSE;
   apr_size_t target_count, merge_target_count;
   svn_opt_revision_t assumed_initial_revision1, assumed_initial_revision2;
+ apr_pool_t *subpool = svn_pool_create(pool);

   /* Establish first RA session to URL1. */
   SVN_ERR(svn_client__open_ra_session_internal(&ra_session1,
initial_URL1, NULL,
@@ -2226,25 +2235,33 @@
         APR_ARRAY_PUSH(remaining_ranges, svn_merge_range_t *) = &range;
     }

- /* ### FIXME: Handle notification callbacks for multiple merges into
- ### a single versioned resource. */
   for (i = 0; i < remaining_ranges->nelts; i++)
     {
+ svn_wc_notify_t *n;
+
       /* When using this merge range, account for the exclusivity of
          its low value (which is indicated by this operation being a
          merge vs. revert). */
       svn_merge_range_t *r = APR_ARRAY_IDX(remaining_ranges, i,
                                            svn_merge_range_t *);

+ svn_pool_clear(subpool);
+
+ n= svn_wc_create_notify(target_wcpath,
+ svn_wc_notify_merge_begin,
+ subpool);
+ n->merge_range = r;
+ notification_receiver(&notify_b, n, subpool);
+
       /* While we currently don't allow it, in theory we could be
          fetching two fulltexts from two different repositories here. */
       SVN_ERR(single_file_merge_get_file(&tmpfile1, ra_session1, &props1,
                                          is_revert ? r->start : r->start - 1,
- URL1, target_wcpath, pool));
+ URL1, target_wcpath, subpool));

       SVN_ERR(single_file_merge_get_file(&tmpfile2, ra_session2, &props2,
                                          is_revert ? r->end - 1 : r->end,
- URL2, target_wcpath, pool));
+ URL2, target_wcpath, subpool));

       /* Discover any svn:mime-type values in the proplists */
       pval = apr_hash_get(props1, SVN_PROP_MIME_TYPE,
@@ -2256,7 +2273,7 @@
       mimetype2 = pval ? pval->data : NULL;

       /* Deduce property diffs. */
- SVN_ERR(svn_prop_diffs(&propchanges, props2, props1, pool));
+ SVN_ERR(svn_prop_diffs(&propchanges, props2, props1, subpool));

       SVN_ERR(merge_file_changed(adm_access,
                                  &text_state, &prop_state,
@@ -2270,11 +2287,11 @@
                                  merge_b));

       /* Ignore if temporary file not found. It may have been renamed. */
- err = svn_io_remove_file(tmpfile1, pool);
+ err = svn_io_remove_file(tmpfile1, subpool);
       if (err && ! APR_STATUS_IS_ENOENT(err->apr_err))
         return err;
       svn_error_clear(err);
- err = svn_io_remove_file(tmpfile2, pool);
+ err = svn_io_remove_file(tmpfile2, subpool);
       if (err && ! APR_STATUS_IS_ENOENT(err->apr_err))
         return err;
       svn_error_clear(err);
@@ -2282,11 +2299,11 @@
         {
           svn_wc_notify_t *notify
           = svn_wc_create_notify(target_wcpath, svn_wc_notify_update_update,
- pool);
+ subpool);
           notify->kind = svn_node_file;
           notify->content_state = text_state;
           notify->prop_state = prop_state;
- notification_receiver(&notify_b, notify, pool);
+ notification_receiver(&notify_b, notify, subpool);
         }

       /* ### LATER: Give the caller a shot at resolving any conflicts
@@ -2302,18 +2319,18 @@
                  merges, minus any unresolved conflicts and skips. */
               apr_hash_t *merges;
               SVN_ERR(determine_merges_performed(&merges, target_wcpath, r,
- &notify_b, pool));
+ &notify_b, subpool));

               /* If merge target has inherited mergeinfo set it before
                  recording the first merge range. */
               if (!i && inherited)
                 SVN_ERR(svn_client__record_wc_merge_info(target_wcpath,
                                                          target_mergeinfo,
- adm_access, pool));
+ adm_access, subpool));

               SVN_ERR(update_wc_merge_info(target_wcpath, entry, rel_path,
                                            merges, is_revert, adm_access,
- ctx, pool));
+ ctx, subpool));
             }

           /* Clear the notification counter and list of skipped paths
Index: subversion/tests/cmdline/update_tests.py
===================================================================
--- subversion/tests/cmdline/update_tests.py (revision 25052)
+++ subversion/tests/cmdline/update_tests.py (working copy)
@@ -2938,22 +2938,30 @@
                                         '-r', '5', wc_dir)

   # Merge r2:5 to A/B_COPY/E/alpha
+ short_E_COPY_path = shorten_path_kludge(E_COPY_path)
   short_alpha_COPY_path = shorten_path_kludge(alpha_COPY_path)
- expected_output = wc.State(short_alpha_COPY_path, {
+ expected_output = wc.State(short_E_copy_path, {
     'alpha' : Item(status='U '),
     })
- expected_skip = wc.State(short_alpha_COPY_path, { })
+ expected_output.pprint()
+ expected_merge_status = wc.State(short_E_COPY_path, {
+ 'alpha' : Item(status='MM', wc_rev=5),
+ })
+ expected_merge_disk = wc.State(short_E_COPY_path, {
+ 'alpha' : Item("New content")
+ })
+ expected_skip = wc.State(short_E_COPY_path, { })
   saved_cwd = os.getcwd()
   try:
     os.chdir(svntest.main.work_dir)
- # run_and_verify_merge doesn't support merging to a file WCPATH
- # so use run_and_verify_svn.
- svntest.actions.run_and_verify_svn(None,
- ['U ' + \
- short_alpha_COPY_path + '\n'],
- [], 'merge', '-r2:5',
- sbox.repo_url + '/A/B/E/alpha',
- short_alpha_COPY_path)
+ svntest.actions.run_and_verify_merge(short_alpha_COPY_path, '2', '5',
+ sbox.repo_url + '/A/B/E/alpha',
+ expected_output,
+ expected_merge_disk,
+ expected_merge_status,
+ expected_skip,
+ None, None, None, None,
+ None, 1, 0)
   finally:
     os.chdir(saved_cwd)

@@ -2963,10 +2971,6 @@
   svntest.actions.run_and_verify_status(alpha_COPY_path,
                                         expected_alpha_status)

- svntest.actions.run_and_verify_svn(None, ["/A/B/E/alpha:1,3-5\n"], [],
- 'propget', SVN_PROP_MERGE_INFO,
- alpha_COPY_path)
-
   # Update WC. The local mergeinfo (r1,3-5) on A/B_COPY/E/alpha is
   # identical to that on added to A/B_COPY by the update, so should
   # elide to the latter, leaving no mereginfo on alpha.
Index: subversion/tests/cmdline/svntest/tree.py
===================================================================
--- subversion/tests/cmdline/svntest/tree.py (revision 25052)
+++ subversion/tests/cmdline/svntest/tree.py (working copy)
@@ -768,23 +768,38 @@
 # process for every file and dir in the working copy!

-def build_tree_from_wc(wc_path, load_props=0, ignore_svn=1):
- """Takes WC_PATH as the path to a working copy. Walks the tree below
- that path, and creates the tree based on the actual found
- files. If IGNORE_SVN is true, then exclude SVN admin dirs from the tree.
- If LOAD_PROPS is true, the props will be added to the tree."""
+def build_tree_from_wc(path, load_props=0, ignore_svn=1):
+ """Takes PATH as the path to either a working copy or single
+ versioned file. If PATH is a directory, walk the tree below that
+ path, and create the tree based on the actual files found. If
+ PATH is a single file, return a 'tree' of exactly one file node.

+ If IGNORE_SVN is true, then exclude .svn admin dirs from the tree.
+ If LOAD_PROPS is true, then scan properties when building the tree."""
+
     root = SVNTreeNode(root_node_name, None)
+ is_dir = os.path.isdir(path)

+ # Handle single-file case
+ if not is_dir:
+ fcontents = get_text(path)
+ if load_props:
+ fprops = get_props(path)
+ else:
+ fprops = {}
+ root.add_child(SVNTreeNode(os.path.basename(path), None,
+ fcontents, fprops))
+ return root
+
     # if necessary, store the root dir's props in a new child node '.'.
     if load_props:
- props = get_props(wc_path)
+ props = get_props(path)
       if props:
         root_dir_node = SVNTreeNode(os.path.basename('.'), None, None, props)
         root.add_child(root_dir_node)

     # Walk the tree recursively
- handle_dir(os.path.normpath(wc_path), root, load_props, ignore_svn)
+ handle_dir(os.path.normpath(path), root, load_props, ignore_svn)

     return root

Index: subversion/svn/notify.c
===================================================================
--- subversion/svn/notify.c (revision 25052)
+++ subversion/svn/notify.c (working copy)
@@ -410,6 +410,17 @@
       svn_error_clear(n->err);
       break;

+ case svn_wc_notify_merge_begin:
+ if (n->merge_range->start == n->merge_range->end)
+ err = svn_cmdline_printf(pool, _("--- Merging revision %ld:\n"),
+ n->merge_range->start);
+ else
+ err = svn_cmdline_printf(pool, _("--- Merging revisions %ld-%ld:\n"),
+ n->merge_range->start, n->merge_range->end);
+ if (err)
+ goto print_error;
+ break;
+
     default:
       break;
     }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 17 23:27:49 2007

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