You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2012/07/04 12:25:00 UTC
svn commit: r1357214 - in /subversion/trunk/subversion:
libsvn_wc/conflicts.c libsvn_wc/merge.c libsvn_wc/props.c
tests/cmdline/merge_tests.py
Author: rhuijben
Date: Wed Jul 4 10:24:58 2012
New Revision: 1357214
URL: http://svn.apache.org/viewvc?rev=1357214&view=rev
Log:
Make it an error to perform a text or property merge into an already conflicted
node. We can't store two conflicts of the same kind on a node.
(And even if we could, it would be very hard to provide a UI to resolve the
conflict).
'svn merge' should probably provide some more user friendly handling for this
case. Just blocking merging into a somewhere conflicted tree currently breaks
a lot of tests, while it is certainly *not* best practice to merge into a
conflicted tree.
* subversion/libsvn_wc/conflicts.c
(svn_wc__conflict_create_markers): Remove ugly corner case. The new
behavior is that a second .prej file is created when this case is hit,
so there is at least some evidence that the property conflict was
deleted.
* subversion/libsvn_wc/merge.c
(svn_wc_merge5): Detect merging into an already conflicted node.
* subversion/libsvn_wc/props.c
(svn_wc__perform_props_merge): Detect merging into an already conflicted
node.
* subversion/tests/cmdline/merge_tests.py
(expected_merge_output,
svn_merge): Allow passing a list of resolved targets.
(merge_two_edits_to_same_prop): Accept the conflict before merging again.
Modified:
subversion/trunk/subversion/libsvn_wc/conflicts.c
subversion/trunk/subversion/libsvn_wc/merge.c
subversion/trunk/subversion/libsvn_wc/props.c
subversion/trunk/subversion/tests/cmdline/merge_tests.py
Modified: subversion/trunk/subversion/libsvn_wc/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/conflicts.c?rev=1357214&r1=1357213&r2=1357214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_wc/conflicts.c Wed Jul 4 10:24:58 2012
@@ -955,6 +955,12 @@ svn_wc__conflict_create_markers(svn_skel
if (prop_conflicted)
{
+ const char *marker_abspath = NULL;
+ svn_node_kind_t kind;
+ const char *marker_dir;
+ const char *marker_name;
+ const char *marker_relpath;
+
/* Ok, currently we have to do a few things for property conflicts:
- Create a marker file
- Create a WQ item that sets the marker name
@@ -962,52 +968,23 @@ svn_wc__conflict_create_markers(svn_skel
This can be simplified once we really store conflict_skel in wc.db */
- const char *marker_abspath = NULL;
- const char *marker_relpath;
-
- /* ### as the legacy code, check if we already have a prejfile.
-
- ### Triggered by merge_tests.py 90 on a double property merge.
- ### Needs further review as we will probably loose the original
- ### conflict by overwriting. (Legacy issue) */
- {
- svn_skel_t *old_conflict;
- SVN_ERR(svn_wc__db_read_conflict(&old_conflict, db, local_abspath,
- scratch_pool, scratch_pool));
-
- if (old_conflict)
- SVN_ERR(svn_wc__conflict_read_prop_conflict(&marker_abspath,
- NULL, NULL, NULL, NULL,
- db, local_abspath,
- old_conflict,
- scratch_pool,
- scratch_pool));
- }
+ SVN_ERR(svn_io_check_path(local_abspath, &kind, scratch_pool));
- if (! marker_abspath)
+ if (kind == svn_node_dir)
{
- svn_node_kind_t kind;
- const char *marker_dir;
- const char *marker_name;
-
- SVN_ERR(svn_io_check_path(local_abspath, &kind, scratch_pool));
-
- if (kind == svn_node_dir)
- {
- marker_dir = local_abspath;
- marker_name = SVN_WC__THIS_DIR_PREJ;
- }
- else
- svn_dirent_split(&marker_dir, &marker_name, local_abspath,
- scratch_pool);
-
- SVN_ERR(svn_io_open_uniquely_named(NULL, &marker_abspath,
- marker_dir,
- marker_name,
- SVN_WC__PROP_REJ_EXT,
- svn_io_file_del_none,
- scratch_pool, scratch_pool));
+ marker_dir = local_abspath;
+ marker_name = SVN_WC__THIS_DIR_PREJ;
}
+ else
+ svn_dirent_split(&marker_dir, &marker_name, local_abspath,
+ scratch_pool);
+
+ SVN_ERR(svn_io_open_uniquely_named(NULL, &marker_abspath,
+ marker_dir,
+ marker_name,
+ SVN_WC__PROP_REJ_EXT,
+ svn_io_file_del_none,
+ scratch_pool, scratch_pool));
SVN_ERR(svn_wc__db_to_relpath(&marker_relpath, db, local_abspath,
marker_abspath, result_pool, result_pool));
Modified: subversion/trunk/subversion/libsvn_wc/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=1357214&r1=1357213&r2=1357214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/merge.c (original)
+++ subversion/trunk/subversion/libsvn_wc/merge.c Wed Jul 4 10:24:58 2012
@@ -1114,12 +1114,13 @@ svn_wc_merge5(enum svn_wc_merge_outcome_
svn_kind_t kind;
svn_boolean_t had_props;
svn_boolean_t props_mod;
+ svn_boolean_t conflicted;
SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, &had_props, &props_mod, NULL, NULL,
- NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL,
+ &conflicted, NULL, &had_props, &props_mod,
+ NULL, NULL, NULL,
wc_ctx->db, target_abspath,
scratch_pool, scratch_pool));
@@ -1132,6 +1133,31 @@ svn_wc_merge5(enum svn_wc_merge_outcome_
return SVN_NO_ERROR;
}
+ if (conflicted)
+ {
+ svn_boolean_t text_conflicted;
+ svn_boolean_t prop_conflicted;
+ svn_boolean_t tree_conflicted;
+
+ SVN_ERR(svn_wc__internal_conflicted_p(&text_conflicted,
+ &prop_conflicted,
+ &tree_conflicted,
+ wc_ctx->db, target_abspath,
+ scratch_pool));
+
+ /* We can't install two prop conflicts on a single node, so
+ avoid even checking that we have to merge it */
+ if (text_conflicted || prop_conflicted || tree_conflicted)
+ {
+ return svn_error_createf(
+ SVN_ERR_WC_PATH_UNEXPECTED_STATUS, NULL,
+ _("Can't merge into conflicted node '%s'"),
+ svn_dirent_local_style(target_abspath,
+ scratch_pool));
+ }
+ /* else: Conflict was resolved by removing markers */
+ }
+
if (merge_props_outcome && had_props)
{
SVN_ERR(svn_wc__db_read_pristine_props(&pristine_props,
Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1357214&r1=1357213&r2=1357214&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Wed Jul 4 10:24:58 2012
@@ -204,6 +204,7 @@ svn_wc__perform_props_merge(svn_wc_notif
apr_hash_t *new_actual_props;
svn_boolean_t had_props, props_mod;
svn_boolean_t have_base;
+ svn_boolean_t conflicted;
svn_skel_t *work_items = NULL;
svn_skel_t *conflict_skel = NULL;
@@ -212,7 +213,7 @@ svn_wc__perform_props_merge(svn_wc_notif
SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, &conflicted, NULL,
&had_props, &props_mod, &have_base, NULL, NULL,
db, local_abspath,
scratch_pool, scratch_pool));
@@ -236,6 +237,30 @@ svn_wc__perform_props_merge(svn_wc_notif
_("The node '%s' does not have properties in this state."),
svn_dirent_local_style(local_abspath, scratch_pool));
}
+ else if (conflicted)
+ {
+ svn_boolean_t text_conflicted;
+ svn_boolean_t prop_conflicted;
+ svn_boolean_t tree_conflicted;
+
+ SVN_ERR(svn_wc__internal_conflicted_p(&text_conflicted,
+ &prop_conflicted,
+ &tree_conflicted,
+ db, local_abspath,
+ scratch_pool));
+
+ /* We can't install two text/prop conflicts on a single node, so
+ avoid even checking that we have to merge it */
+ if (text_conflicted || prop_conflicted || tree_conflicted)
+ {
+ return svn_error_createf(
+ SVN_ERR_WC_PATH_UNEXPECTED_STATUS, NULL,
+ _("Can't merge into conflicted node '%s'"),
+ svn_dirent_local_style(local_abspath,
+ scratch_pool));
+ }
+ /* else: Conflict was resolved by removing markers */
+ }
/* The PROPCHANGES may not have non-"normal" properties in it. If entry
or wc props were allowed, then the following code would install them
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1357214&r1=1357213&r2=1357214&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Wed Jul 4 10:24:58 2012
@@ -50,7 +50,8 @@ from svntest.actions import inject_confl
def expected_merge_output(rev_ranges, additional_lines=None, foreign=False,
elides=False, two_url=False, target=None,
- text_conflicts=0, prop_conflicts=0, tree_conflicts=0):
+ text_conflicts=0, prop_conflicts=0, tree_conflicts=0,
+ resolved=[]):
"""Generate an (inefficient) regex representing the expected merge
output and mergeinfo notifications from REV_RANGES and ADDITIONAL_LINES.
@@ -72,7 +73,10 @@ def expected_merge_output(rev_ranges, ad
notifications; if None, it is not checked.
TEXT_CONFLICTS, PROP_CONFLICTS and TREE_CONFLICTS specify the number of
- each kind of conflict to expect."""
+ each kind of conflict to expect.
+
+ RESOLVED contains a list of target paths of which conflicts are resolved
+ during merging"""
if rev_ranges is None:
lines = [svntest.main.merge_notify_line(None, None, False, foreign)]
@@ -111,6 +115,9 @@ def expected_merge_output(rev_ranges, ad
additional_lines = additional_lines.replace("\\", "\\\\")
lines.append(str(additional_lines))
+ for rslv in resolved:
+ lines.append("Resolved conflicted state of '%s'" % re.escape(rslv))
+
if text_conflicts or prop_conflicts or tree_conflicts:
lines.append("Summary of conflicts:\n")
if text_conflicts:
@@ -12532,7 +12539,8 @@ def svn_copy(s_rev, path1, path2):
'-r', s_rev, path1, path2)
def svn_merge(rev_range, source, target, lines=None, elides=[],
- text_conflicts=0, prop_conflicts=0, tree_conflicts=0, args=[]):
+ text_conflicts=0, prop_conflicts=0, tree_conflicts=0, args=[],
+ resolved=[]):
"""Merge a single change from path SOURCE to path TARGET and verify the
output and that there is no error. (The changes made are not verified.)
@@ -12547,7 +12555,10 @@ def svn_merge(rev_range, source, target,
TEXT_CONFLICTS, PROP_CONFLICTS and TREE_CONFLICTS specify the number of
each kind of conflict to expect.
- ARGS are additional arguments passed to svn merge."""
+ ARGS are additional arguments passed to svn merge.
+
+ RESOLVED contains a list of targets of which conflicts are resolved
+ during merging"""
source = local_path(source)
target = local_path(target)
@@ -12568,7 +12579,8 @@ def svn_merge(rev_range, source, target,
elides=elides,
text_conflicts=text_conflicts,
prop_conflicts=prop_conflicts,
- tree_conflicts=tree_conflicts)
+ tree_conflicts=tree_conflicts,
+ resolved=resolved)
svntest.actions.run_and_verify_svn(None, exp_out, [],
'merge', rev_arg, source, target, *args)
@@ -13110,7 +13122,10 @@ def merge_two_edits_to_same_prop(sbox):
# Merge the first change, then the second, to source.
svn_merge(rev3, A_COPY_path, A_path, [
" C %s\n" % mu_path,
- ], prop_conflicts=1, args=['--allow-mixed-revisions'])
+ ], prop_conflicts=1,
+ args=['--allow-mixed-revisions',
+ '--accept=theirs-conflict'],
+ resolved=[mu_path])
svn_merge(rev4, A_COPY_path, A_path, [
" C %s\n" % mu_path,
], prop_conflicts=1, args=['--allow-mixed-revisions'])
RE: svn commit: r1357214 - in /subversion/trunk/subversion: libsvn_wc/conflicts.c libsvn_wc/merge.c libsvn_wc/props.c tests/cmdline/merge_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: woensdag 4 juli 2012 12:39
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1357214 - in /subversion/trunk/subversion:
> libsvn_wc/conflicts.c libsvn_wc/merge.c libsvn_wc/props.c
> tests/cmdline/merge_tests.py
>
> On Wed, Jul 04, 2012 at 10:25:00AM -0000, rhuijben@apache.org wrote:
> > Author: rhuijben
> > Date: Wed Jul 4 10:24:58 2012
> > New Revision: 1357214
> >
> > URL: http://svn.apache.org/viewvc?rev=1357214&view=rev
> > Log:
> > Make it an error to perform a text or property merge into an already
> conflicted
> > node. We can't store two conflicts of the same kind on a node.
> > (And even if we could, it would be very hard to provide a UI to resolve
the
> > conflict).
> >
> > 'svn merge' should probably provide some more user friendly handling for
> this
> > case. Just blocking merging into a somewhere conflicted tree currently
> breaks
> > a lot of tests, while it is certainly *not* best practice to merge into
a
> > conflicted tree.
>
> Can we add an --allow-conflicts switch that overrides this new error,
> like we did for mixed-revisions? That might also make it easier to keep
> the merge tests passing by adding the new flag where appropriate.
That would work for the high-level check, that I couldn't commit because it
would break all kinds of other scenarios.
Low level (where this change was) we should really error on this case and
ask the merge code to resolve the conflict before calling into the merge
code. (Like how it currently handles tree conflicts)
Before this change was applied we just ignored the conflict and wrote a new
conflict right over it. (And we probably did that for several versions).
I do think we should check for conflicts on merging like we do for mixed
revision working copies, but that is outside my current scope of the
conflict handling.
(The status walk in the merge code already does all the hard work, it is
just passing the information to the user. The patch to do that was so small
that I just deleted it, instead of keeping it for future work)
Bert
Re: svn commit: r1357214 - in /subversion/trunk/subversion:
libsvn_wc/conflicts.c libsvn_wc/merge.c libsvn_wc/props.c
tests/cmdline/merge_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jul 04, 2012 at 10:25:00AM -0000, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Wed Jul 4 10:24:58 2012
> New Revision: 1357214
>
> URL: http://svn.apache.org/viewvc?rev=1357214&view=rev
> Log:
> Make it an error to perform a text or property merge into an already conflicted
> node. We can't store two conflicts of the same kind on a node.
> (And even if we could, it would be very hard to provide a UI to resolve the
> conflict).
>
> 'svn merge' should probably provide some more user friendly handling for this
> case. Just blocking merging into a somewhere conflicted tree currently breaks
> a lot of tests, while it is certainly *not* best practice to merge into a
> conflicted tree.
Can we add an --allow-conflicts switch that overrides this new error,
like we did for mixed-revisions? That might also make it easier to keep
the merge tests passing by adding the new flag where appropriate.