You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2006/06/30 22:46:27 UTC
[PATCH] Fix issue 1914: Update runs diff3 twice
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
Re: [PATCH] Fix issue 1914: Update runs diff3 twice
Posted by Erik Huelsmann <eh...@gmail.com>.
On 7/1/06, Erik Huelsmann <eh...@gmail.com> wrote:
> More digging turned up ...
>
> On 7/1/06, Erik Huelsmann <eh...@gmail.com> wrote:
> > 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 jpieper added the dry-run merge in r9868 to be able to do
> per-file progress reporting when he switched libsvn_wc from per-file
> to per-directory log running (to make libsvn_wc more efficient at
> entries rewriting).
>
> My change enables per-file progress reporting while still doing
> per-directory log-running without a dry-run. So, I guess it's OK to
> commit, apart from any other comments...
Committed in r20340.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Fix issue 1914: Update runs diff3 twice
Posted by Erik Huelsmann <eh...@gmail.com>.
More digging turned up ...
On 7/1/06, Erik Huelsmann <eh...@gmail.com> wrote:
> 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 jpieper added the dry-run merge in r9868 to be able to do
per-file progress reporting when he switched libsvn_wc from per-file
to per-directory log running (to make libsvn_wc more efficient at
entries rewriting).
My change enables per-file progress reporting while still doing
per-directory log-running without a dry-run. So, I guess it's OK to
commit, apart from any other comments...
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org