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

[PATCH] Fix issue 1914: Update runs diff3 twice

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2006-07-01 00:46:27 CEST

For performance and correctness reasons, I think we should stop
running merge twice in update/switch.

In the past, arguments were made that we don't run the actual
(possibly interactive) diff3 in the middle of a network connection to
be sure that the network doesn't time out. After I started
investigating this issue last week, I found

* That we actually run the external (possibly interactive) diff3 in
the close_directory phase of the editor drive, which is as much 'in
the middle of the connection' as close_file is (from which merge_file
is called).

* That ra_dav actually reads the entire response from the connection
and only processes it only after having received it completely.

* Now that merge itself is a loggy operation, there is no need for a
'merge' log command anymore: merge induced commands can be added to
any log file when it's being generated.

Because of the 3 reasons above, I think we should stop merging twice.

There may be 1 problem in the current code though which the patch
doesn't fix: our update/switch code doesn't seem to provide a
merge_options argument...

I've got a patch attached which does make it stop running merge twice:

Fix issue 1914: Stop running diff3 twice.

* subversion/libsvn_wc/update_editor.c
  (merge_file): Take advantage of recently added
   svn_wc__merge_internal to create a log
   which updates the working copy with the
   merge results.

Index: subversion/libsvn_wc/update_editor.c
--- subversion/libsvn_wc/update_editor.c (revision 20312)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -2069,36 +2069,22 @@
               newrev_str = apr_psprintf(pool, ".r%ld",
               /* Merge the changes from the old-textbase (TXTB) to
                  new-textbase (TMP_TXTB) into the file we're
- updating (BASE_NAME). Either the merge will
- happen smoothly, or a conflict will result.
- Luckily, this routine will take care of all eol
- and keyword translation, and diff3 will insert
- conflict markers for us. It also deals with binary
- files appropriately. */
- SVN_ERR(svn_wc__loggy_merge(&log_accum, adm_access,
- base_name, txtb, tmp_txtb,
- oldrev_str, newrev_str, ".mine",
- pool));
- /* Run a dry-run of the merge to see if a conflict will
- occur. This is needed so we can report back to the
- client as the changes come in. */
+ updating (BASE_NAME). Append commands to update the
+ working copy to LOG_ACCUM. */
               base = svn_wc_adm_access_path(adm_access);
+ SVN_ERR(svn_wc__merge_internal
+ (&log_accum,
+ svn_path_join(base, txtb, pool),
+ svn_path_join(base, tmp_txtb, pool),
+ svn_path_join(base, base_name, pool),
+ adm_access,
+ oldrev_str, newrev_str, ".mine",
+ FALSE, &merge_outcome, diff3_cmd, NULL,
+ pool));

- /* ### FIXME: We force use of the internal diff3 here
- because we don't want to run a graphical external
- diff3 twice. See issue 1914. */
- SVN_ERR(svn_wc_merge2(svn_path_join(base, txtb, pool),
- svn_path_join(base, tmp_txtb, pool),
- svn_path_join(base, base_name, pool),
- adm_access,
- oldrev_str, newrev_str, ".mine",
- TRUE, &merge_outcome, NULL, NULL,
- pool));
             } /* end: working file exists and has mods */
         } /* end: working file has mods */
     } /* end: "textual" merging process */

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jul 1 00:46:53 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.