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:
Log:
[[[
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 @@
e->revision);
newrev_str = apr_psprintf(pool, ".r%ld",
new_revision);
-
+
/* 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