You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2014/05/17 20:08:50 UTC
svn commit: r1595522 - in /subversion/trunk/subversion: libsvn_wc/props.c
tests/cmdline/merge_tests.py tests/cmdline/move_tests.py
tests/cmdline/prop_tests.py
Author: danielsh
Date: Sat May 17 18:08:49 2014
New Revision: 1595522
URL: http://svn.apache.org/r1595522
Log:
Render property conflicts with diff3.
Thread: http://mail-archives.apache.org/mod_mbox/subversion-dev/201405.mbox/%3C20140506110855.GA1948@tarsus.local2%3E
* subversion/libsvn_wc/props.c
(prop_conflict_from_skel): Always use INCOMING_BASE as the diff3 "older".
Add detailed comment.
* subversion/tests/cmdline/merge_tests.py
(simple_property_merges):
* subversion/tests/cmdline/move_tests.py
(property_merge):
* subversion/tests/cmdline/prop_tests.py
(prop_reject_grind):
Update expectations.
* subversion/tests/cmdline/prop_tests.py
(file_matching_dir_prop_reject):
Add comment. No functional change.
Modified:
subversion/trunk/subversion/libsvn_wc/props.c
subversion/trunk/subversion/tests/cmdline/merge_tests.py
subversion/trunk/subversion/tests/cmdline/move_tests.py
subversion/trunk/subversion/tests/cmdline/prop_tests.py
Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1595522&r1=1595521&r2=1595522&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Sat May 17 18:08:49 2014
@@ -555,7 +555,7 @@ prop_conflict_from_skel(const svn_string
svn_diff_t *diff;
svn_diff_file_options_t *diff_opts;
svn_stringbuf_t *buf;
- svn_boolean_t original_is_binary;
+ svn_boolean_t incoming_base_is_binary;
svn_boolean_t mine_is_binary;
svn_boolean_t incoming_is_binary;
@@ -573,47 +573,72 @@ prop_conflict_from_skel(const svn_string
buf = generate_conflict_message(propname, original, mine, incoming,
incoming_base, scratch_pool);
+ /* Convert deleted or not-yet-added values to empty-string values, for the
+ purposes of diff generation and binary detection. */
if (mine == NULL)
mine = svn_string_create_empty(scratch_pool);
if (incoming == NULL)
incoming = svn_string_create_empty(scratch_pool);
+ if (incoming_base == NULL)
+ incoming_base = svn_string_create_empty(scratch_pool);
- /* Pick a suitable base for the conflict diff.
- * The incoming value is always a change,
- * but the local value might not have changed. */
- if (original == NULL)
- {
- if (incoming_base)
- original = incoming_base;
- else
- original = svn_string_create_empty(scratch_pool);
- }
- else if (incoming_base && svn_string_compare(original, mine))
- original = incoming_base;
+ /* How we render the conflict:
+
+ We have four sides: original, mine, incoming_base, incoming.
+ We render the conflict as a 3-way diff. A diff3 API has three parts,
+ called:
+
+ <<< - original
+ ||| - modified (or "older")
+ === - latest (or "theirs")
+ >>>
+
+ We fill those parts as follows:
+
+ PART FILLED BY SKEL MEMBER USER-FACING ROLE
+ ==== ===================== ================
+ original mine was WORKING tree at conflict creation
+ modified incoming_base left-hand side of merge
+ latest incoming right-hand side of merge
+ (none) original was BASE tree at conflict creation
+
+ An 'update -r rN' is treated like a 'merge -r BASE:rN', i.e., in an
+ 'update' operation skel->original and skel->incoming_base coincide.
+
+ Note that the term "original" is used both in the skel and in diff3
+ with different meanings. Note also that the skel's ORIGINAL value was
+ at some point in the BASE tree, but the BASE tree need not have contained
+ the INCOMING_BASE value.
+
+ Yes, it's confusing. */
/* If any of the property values involved in the diff is binary data,
* do not generate a diff. */
- original_is_binary = svn_io_is_binary_data(original->data, original->len);
+ incoming_base_is_binary = svn_io_is_binary_data(incoming_base->data,
+ incoming_base->len);
mine_is_binary = svn_io_is_binary_data(mine->data, mine->len);
incoming_is_binary = svn_io_is_binary_data(incoming->data, incoming->len);
- if (!(original_is_binary || mine_is_binary || incoming_is_binary))
+ if (!(incoming_base_is_binary || mine_is_binary || incoming_is_binary))
{
diff_opts = svn_diff_file_options_create(scratch_pool);
diff_opts->ignore_space = svn_diff_file_ignore_space_none;
diff_opts->ignore_eol_style = FALSE;
diff_opts->show_c_function = FALSE;
- SVN_ERR(svn_diff_mem_string_diff3(&diff, original, mine, incoming,
+ /* Pass skel member INCOMING_BASE into the formal parameter ORIGINAL.
+ Ignore the skel member ORIGINAL. */
+ SVN_ERR(svn_diff_mem_string_diff3(&diff, incoming_base, mine, incoming,
diff_opts, scratch_pool));
if (svn_diff_contains_conflicts(diff))
{
svn_stream_t *stream;
svn_diff_conflict_display_style_t style;
const char *mine_marker = _("<<<<<<< (local property value)");
- const char *incoming_marker = _(">>>>>>> (incoming property value)");
+ const char *incoming_marker = _(">>>>>>> (incoming 'changed to' value)");
+ const char *incoming_base_marker = _("||||||| (incoming 'changed from' value)");
const char *separator = "=======";
- svn_string_t *original_ascii =
- svn_string_create(svn_utf_cstring_from_utf8_fuzzy(original->data,
+ svn_string_t *incoming_base_ascii =
+ svn_string_create(svn_utf_cstring_from_utf8_fuzzy(incoming_base->data,
scratch_pool),
scratch_pool);
svn_string_t *mine_ascii =
@@ -625,14 +650,14 @@ prop_conflict_from_skel(const svn_string
scratch_pool),
scratch_pool);
- style = svn_diff_conflict_display_modified_latest;
+ style = svn_diff_conflict_display_modified_original_latest;
stream = svn_stream_from_stringbuf(buf, scratch_pool);
SVN_ERR(svn_stream_skip(stream, buf->len));
SVN_ERR(svn_diff_mem_string_output_merge2(stream, diff,
- original_ascii,
+ incoming_base_ascii,
mine_ascii,
incoming_ascii,
- NULL, mine_marker,
+ incoming_base_marker, mine_marker,
incoming_marker, separator,
style, scratch_pool));
SVN_ERR(svn_stream_close(stream));
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1595522&r1=1595521&r2=1595522&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Sat May 17 18:08:49 2014
@@ -645,8 +645,10 @@ def simple_property_merges(sbox):
def error_message(property, old_value, new_value):
return "Trying to change property '%s'\n" \
"but the property has been locally deleted.\n" \
- "<<<<<<< (local property value)\n=======\n" \
- "%s>>>>>>> (incoming property value)\n" % (property, new_value)
+ "<<<<<<< (local property value)\n" \
+ "||||||| (incoming 'changed from' value)\n" \
+ "%s=======\n" \
+ "%s>>>>>>> (incoming 'changed to' value)\n" % (property, old_value, new_value)
# Merge B 3:4 into B2 now causes a conflict
expected_disk.add({
Modified: subversion/trunk/subversion/tests/cmdline/move_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/move_tests.py?rev=1595522&r1=1595521&r2=1595522&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/move_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/move_tests.py Sat May 17 18:08:49 2014
@@ -1140,29 +1140,33 @@ def property_merge(sbox):
"""Trying to add new property 'key1'
but the property already exists.
<<<<<<< (local property value)
-value2=======
-value3>>>>>>> (incoming property value)
+value2||||||| (incoming 'changed from' value)
+=======
+value3>>>>>>> (incoming 'changed to' value)
"""),
'A/C2/D5/dir_conflicts.prej' : Item(contents=
"""Trying to change property 'key1'
but the property has already been locally changed to a different value.
<<<<<<< (local property value)
-value2=======
-value3>>>>>>> (incoming property value)
+value2||||||| (incoming 'changed from' value)
+value1=======
+value3>>>>>>> (incoming 'changed to' value)
"""),
'A/C2/f4.prej' : Item(contents=
"""Trying to add new property 'key1'
but the property already exists.
<<<<<<< (local property value)
-value2=======
-value3>>>>>>> (incoming property value)
+value2||||||| (incoming 'changed from' value)
+=======
+value3>>>>>>> (incoming 'changed to' value)
"""),
'A/C2/f5.prej' : Item(contents=
"""Trying to change property 'key1'
but the property has already been locally changed to a different value.
<<<<<<< (local property value)
-value2=======
-value3>>>>>>> (incoming property value)
+value2||||||| (incoming 'changed from' value)
+value1=======
+value3>>>>>>> (incoming 'changed to' value)
"""),
})
Modified: subversion/trunk/subversion/tests/cmdline/prop_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/prop_tests.py?rev=1595522&r1=1595521&r2=1595522&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/prop_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/prop_tests.py Sat May 17 18:08:49 2014
@@ -1878,8 +1878,9 @@ def prop_reject_grind(sbox):
"Trying to change property 'edit.none'\n"
"but the property does not exist locally.\n"
"<<<<<<< (local property value)\n"
- "=======\n"
- "repos.changed>>>>>>> (incoming property value)\n",
+ "||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ "repos.changed>>>>>>> (incoming 'changed to' value)\n",
"Trying to delete property 'del.del'\n"
"but the property has been locally deleted and had a different value.\n",
@@ -1887,75 +1888,84 @@ def prop_reject_grind(sbox):
"Trying to delete property 'del.edit'\n"
"but the local property value is different.\n"
"<<<<<<< (local property value)\n"
- "local.changed=======\n"
- ">>>>>>> (incoming property value)\n",
+ "local.changed||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ ">>>>>>> (incoming 'changed to' value)\n",
"Trying to change property 'edit.del'\n"
"but the property has been locally deleted.\n"
"<<<<<<< (local property value)\n"
- "=======\n"
- "repos.changed>>>>>>> (incoming property value)\n",
+ "||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ "repos.changed>>>>>>> (incoming 'changed to' value)\n",
"Trying to change property 'edit.edit'\n"
"but the property has already been locally changed to a different value.\n"
"<<<<<<< (local property value)\n"
- "local.changed=======\n"
- "repos.changed>>>>>>> (incoming property value)\n",
+ "local.changed||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ "repos.changed>>>>>>> (incoming 'changed to' value)\n",
"Trying to delete property 'del.edit2'\n"
"but the property has been locally modified.\n"
"<<<<<<< (local property value)\n"
- "repos.changed=======\n"
- ">>>>>>> (incoming property value)\n",
+ "repos.changed||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ ">>>>>>> (incoming 'changed to' value)\n",
"Trying to delete property 'del.add'\n"
"but the property has been locally added.\n"
"<<<<<<< (local property value)\n"
- "local=======\n"
- ">>>>>>> (incoming property value)\n",
+ "local||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ ">>>>>>> (incoming 'changed to' value)\n",
"Trying to delete property 'del.diff'\n"
"but the local property value is different.\n"
"<<<<<<< (local property value)\n"
- "local=======\n"
- ">>>>>>> (incoming property value)\n",
+ "local||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ ">>>>>>> (incoming 'changed to' value)\n",
"Trying to change property 'edit.add'\n"
"but the property has been locally added with a different value.\n"
"<<<<<<< (local property value)\n"
- "local=======\n"
- "repos.changed>>>>>>> (incoming property value)\n",
+ "local||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ "repos.changed>>>>>>> (incoming 'changed to' value)\n",
"Trying to change property 'edit.diff'\n"
"but the local property value conflicts with the incoming change.\n"
"<<<<<<< (local property value)\n"
- "local=======\n"
- "repos.changed>>>>>>> (incoming property value)\n",
+ "local||||||| (incoming 'changed from' value)\n"
+ "repos=======\n"
+ "repos.changed>>>>>>> (incoming 'changed to' value)\n",
"Trying to add new property 'add.add'\n"
"but the property already exists.\n"
"<<<<<<< (local property value)\n"
- "local=======\n"
- "repos>>>>>>> (incoming property value)\n",
+ "local||||||| (incoming 'changed from' value)\n"
+ "=======\n"
+ "repos>>>>>>> (incoming 'changed to' value)\n",
"Trying to add new property 'add.diff'\n"
"but the property already exists.\n"
- "Local property value:\n"
- "local\n"
- "Incoming property value:\n"
- "repos\n",
+ "<<<<<<< (local property value)\n"
+ "local||||||| (incoming 'changed from' value)\n"
+ "=======\n"
+ "repos>>>>>>> (incoming 'changed to' value)\n",
"Trying to add new property 'add.del'\n"
"but the property has been locally deleted.\n"
- "<<<<<<< (local property value)\n"
- "=======\n"
- "repos>>>>>>> (incoming property value)\n",
+ "Incoming property value:\n"
+ "repos\n",
"Trying to add new property 'add.edit'\n"
"but the property already exists.\n"
"<<<<<<< (local property value)\n"
- "local.changed=======\n"
- "repos>>>>>>> (incoming property value)\n",
+ "local.changed||||||| (incoming 'changed from' value)\n"
+ "=======\n"
+ "repos>>>>>>> (incoming 'changed to' value)\n",
]
# Get the contents of mu.prej. The error messages are in the prej file
@@ -2338,6 +2348,7 @@ def file_matching_dir_prop_reject(sbox):
expected_status.tweak(wc_rev=2)
expected_status.tweak('A', 'A/dir_conflicts', status=' C')
+ # Conflict: BASE=val2 WORKING=val3 INCOMING_OLD=val2 INCOMING_NEW=val1
extra_files = ['dir_conflicts.prej', 'dir_conflicts.2.prej']
svntest.actions.run_and_verify_update(wc_dir,
expected_output,