You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2017/08/16 16:22:56 UTC
svn commit: r1805216 - in /subversion/trunk/subversion:
include/private/svn_wc_private.h libsvn_client/conflicts.c
libsvn_wc/translate.c tests/libsvn_client/conflicts-test.c
Author: kotkov
Date: Wed Aug 16 16:22:56 2017
New Revision: 1805216
URL: http://svn.apache.org/viewvc?rev=1805216&view=rev
Log:
Prevent the tree conflict resolver from creating spurious text conflicts
when merging files with non-LF line endings.
(For example, on Windows, assuming no svn:eol-style, this has been causing
unexpected text conflicts due to mismatching EOLs in the majority of
cases.)
The issue is that the conflict resolver has been unconditionally translating
the contents to LF endings, whereas the actual repository-normal form can
be different.
* subversion/include/private/svn_wc_private.h
(svn_wc__translated_stream): Declare new function.
* subversion/libsvn_wc/translate.c
(svn_wc__translated_stream): Implement this new function by calling
its internal counterpart, svn_wc__internal_translated_stream().
* subversion/libsvn_client/conflicts.c
(get_keywords): Remove this helper, as it's no longer required.
(resolve_incoming_move_file_text_merge,
resolve_merge_incoming_added_file_text_update,
resolve_merge_incoming_added_file_replace_and_merge):
Call svn_wc__translated_stream() for the optional detranslation to
the repository-normal form.
* subversion/tests/libsvn_client/conflicts-test.c
(test_merge_incoming_move_file_text_merge_crlf): New test, fails
without the fix.
(test_merge_incoming_move_file_text_merge_native_eol): New test.
(test_funcs): Add new tests.
Modified:
subversion/trunk/subversion/include/private/svn_wc_private.h
subversion/trunk/subversion/libsvn_client/conflicts.c
subversion/trunk/subversion/libsvn_wc/translate.c
subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_wc_private.h?rev=1805216&r1=1805215&r2=1805216&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_wc_private.h Wed Aug 16 16:22:56 2017
@@ -2062,6 +2062,17 @@ svn_wc__read_conflict_descriptions2_t(co
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+/* Internal version of svn_wc_translated_stream(), accepting a working
+ copy context. */
+svn_error_t *
+svn_wc__translated_stream(svn_stream_t **stream,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ const char *versioned_abspath,
+ apr_uint32_t flags,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1805216&r1=1805215&r2=1805216&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Aug 16 16:22:56 2017
@@ -5917,58 +5917,6 @@ filter_props(apr_hash_t *props, apr_pool
}
}
-/* Get KEYWORDS for LOCAL_ABSPATH.
- * WC_CTX is a context for the working copy the patch is applied to.
- * Use RESULT_POOL for allocations of fields in TARGET.
- * Use SCRATCH_POOL for all other allocations. */
-static svn_error_t *
-get_keywords(apr_hash_t **keywords,
- svn_wc_context_t *wc_ctx,
- const char *local_abspath,
- apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
-{
- apr_hash_t *props;
- svn_string_t *keywords_val;
-
- *keywords = NULL;
- SVN_ERR(svn_wc_prop_list2(&props, wc_ctx, local_abspath,
- scratch_pool, scratch_pool));
- keywords_val = svn_hash_gets(props, SVN_PROP_KEYWORDS);
- if (keywords_val)
- {
- svn_revnum_t changed_rev;
- apr_time_t changed_date;
- const char *rev_str;
- const char *author;
- const char *url;
- const char *repos_root_url;
- const char *repos_relpath;
-
- SVN_ERR(svn_wc__node_get_changed_info(&changed_rev,
- &changed_date,
- &author, wc_ctx,
- local_abspath,
- scratch_pool,
- scratch_pool));
- rev_str = apr_psprintf(scratch_pool, "%ld", changed_rev);
- SVN_ERR(svn_wc__node_get_repos_info(NULL, &repos_relpath, &repos_root_url,
- NULL,
- wc_ctx, local_abspath,
- scratch_pool, scratch_pool));
- url = svn_path_url_add_component2(repos_root_url, repos_relpath,
- scratch_pool);
-
- SVN_ERR(svn_subst_build_keywords3(keywords,
- keywords_val->data,
- rev_str, url, repos_root_url,
- changed_date,
- author, result_pool));
- }
-
- return SVN_NO_ERROR;
-}
-
/* Implements conflict_option_resolve_func_t. */
static svn_error_t *
resolve_merge_incoming_added_file_text_update(
@@ -5986,11 +5934,9 @@ resolve_merge_incoming_added_file_text_u
const char *working_file_tmp_abspath;
svn_stream_t *working_file_stream;
svn_stream_t *working_file_tmp_stream;
- svn_stream_t *normalized_stream;
apr_hash_t *working_props;
apr_array_header_t *propdiffs;
svn_error_t *err;
- apr_hash_t *keywords;
local_abspath = svn_client_conflict_get_local_abspath(conflict);
@@ -6003,16 +5949,12 @@ resolve_merge_incoming_added_file_text_u
svn_io_file_del_none,
scratch_pool, scratch_pool));
- /* Copy the working file to temporary storage. */
- SVN_ERR(svn_stream_open_readonly(&working_file_stream, local_abspath,
- scratch_pool, scratch_pool));
- SVN_ERR(get_keywords(&keywords, ctx->wc_ctx, local_abspath,
- scratch_pool, scratch_pool));
- normalized_stream = svn_subst_stream_translated(working_file_stream,
- "\n", TRUE,
- keywords, FALSE,
- scratch_pool);
- SVN_ERR(svn_stream_copy3(normalized_stream, working_file_tmp_stream,
+ /* Copy the detranslated working file to temporary storage. */
+ SVN_ERR(svn_wc__translated_stream(&working_file_stream, ctx->wc_ctx,
+ local_abspath, local_abspath,
+ SVN_WC_TRANSLATE_TO_NF,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_stream_copy3(working_file_stream, working_file_tmp_stream,
ctx->cancel_func, ctx->cancel_baton,
scratch_pool));
@@ -6254,10 +6196,8 @@ resolve_merge_incoming_added_file_replac
const char *wc_tmpdir;
svn_stream_t *working_file_tmp_stream;
const char *working_file_tmp_abspath;
- svn_stream_t *normalized_stream;
svn_stream_t *working_file_stream;
apr_hash_t *working_props;
- apr_hash_t *keywords;
svn_error_t *err;
svn_wc_merge_outcome_t merge_content_outcome;
svn_wc_notify_state_t merge_props_outcome;
@@ -6275,16 +6215,12 @@ resolve_merge_incoming_added_file_replac
svn_io_file_del_on_pool_cleanup,
scratch_pool, scratch_pool));
- /* Copy the working file to temporary storage. */
- SVN_ERR(svn_stream_open_readonly(&working_file_stream, local_abspath,
- scratch_pool, scratch_pool));
- SVN_ERR(get_keywords(&keywords, ctx->wc_ctx, local_abspath,
- scratch_pool, scratch_pool));
- normalized_stream = svn_subst_stream_translated(working_file_stream,
- "\n", TRUE,
- keywords, FALSE,
- scratch_pool);
- SVN_ERR(svn_stream_copy3(normalized_stream, working_file_tmp_stream,
+ /* Copy the detranslated working file to temporary storage. */
+ SVN_ERR(svn_wc__translated_stream(&working_file_stream, ctx->wc_ctx,
+ local_abspath, local_abspath,
+ SVN_WC_TRANSLATE_TO_NF,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_stream_copy3(working_file_stream, working_file_tmp_stream,
ctx->cancel_func, ctx->cancel_baton,
scratch_pool));
@@ -7746,8 +7682,6 @@ resolve_incoming_move_file_text_merge(sv
{
svn_stream_t *working_stream;
svn_stream_t *incoming_stream;
- svn_stream_t *normalized_stream;
- apr_hash_t *keywords;
/* Create a temporary copy of the working file in repository-normal form.
* Set up this temporary file to be automatically removed. */
@@ -7758,21 +7692,14 @@ resolve_incoming_move_file_text_merge(sv
if (err)
goto unlock_wc;
- err = svn_stream_open_readonly(&working_stream, local_abspath,
- scratch_pool, scratch_pool);
- if (err)
- goto unlock_wc;
-
- err = get_keywords(&keywords, ctx->wc_ctx, local_abspath,
- scratch_pool, scratch_pool);
+ err = svn_wc__translated_stream(&working_stream, ctx->wc_ctx,
+ local_abspath, local_abspath,
+ SVN_WC_TRANSLATE_TO_NF,
+ scratch_pool, scratch_pool);
if (err)
goto unlock_wc;
- normalized_stream = svn_subst_stream_translated(working_stream,
- "\n", TRUE,
- keywords, FALSE,
- scratch_pool);
- err = svn_stream_copy3(normalized_stream, incoming_stream,
+ err = svn_stream_copy3(working_stream, incoming_stream,
NULL, NULL, /* no cancellation */
scratch_pool);
if (err)
@@ -7782,8 +7709,6 @@ resolve_incoming_move_file_text_merge(sv
{
svn_stream_t *incoming_stream;
svn_stream_t *move_target_stream;
- svn_stream_t *normalized_stream;
- apr_hash_t *keywords;
/* Set aside the current move target file. This is required to apply
* the move, and only then perform a three-way text merge between
@@ -7798,21 +7723,14 @@ resolve_incoming_move_file_text_merge(sv
if (err)
goto unlock_wc;
- err = svn_stream_open_readonly(&move_target_stream, moved_to_abspath,
- scratch_pool, scratch_pool);
- if (err)
- goto unlock_wc;
-
- err = get_keywords(&keywords, ctx->wc_ctx, moved_to_abspath,
- scratch_pool, scratch_pool);
+ err = svn_wc__translated_stream(&move_target_stream, ctx->wc_ctx,
+ moved_to_abspath, moved_to_abspath,
+ SVN_WC_TRANSLATE_TO_NF,
+ scratch_pool, scratch_pool);
if (err)
goto unlock_wc;
- normalized_stream = svn_subst_stream_translated(move_target_stream,
- "\n", TRUE,
- keywords, FALSE,
- scratch_pool);
- err = svn_stream_copy3(normalized_stream, incoming_stream,
+ err = svn_stream_copy3(move_target_stream, incoming_stream,
NULL, NULL, /* no cancellation */
scratch_pool);
if (err)
Modified: subversion/trunk/subversion/libsvn_wc/translate.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/translate.c?rev=1805216&r1=1805215&r2=1805216&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/translate.c (original)
+++ subversion/trunk/subversion/libsvn_wc/translate.c Wed Aug 16 16:22:56 2017
@@ -440,3 +440,20 @@ svn_wc__sync_flags_with_props(svn_boolea
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_wc__translated_stream(svn_stream_t **stream,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ const char *versioned_abspath,
+ apr_uint32_t flags,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ return svn_error_trace(
+ svn_wc__internal_translated_stream(stream, wc_ctx->db,
+ local_abspath,
+ versioned_abspath,
+ flags, result_pool,
+ scratch_pool));
+}
Modified: subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c?rev=1805216&r1=1805215&r2=1805216&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c Wed Aug 16 16:22:56 2017
@@ -4682,6 +4682,217 @@ test_cherry_pick_moved_file_with_propdel
return SVN_NO_ERROR;
}
+static svn_error_t *
+test_merge_incoming_move_file_text_merge_crlf(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_test__sandbox_t *b = apr_palloc(pool, sizeof(*b));
+ svn_client_ctx_t *ctx;
+ svn_opt_revision_t opt_rev;
+ svn_client_conflict_t *conflict;
+ svn_boolean_t text_conflicted;
+ apr_array_header_t *props_conflicted;
+ svn_boolean_t tree_conflicted;
+ svn_stringbuf_t *buf;
+
+ SVN_ERR(svn_test__sandbox_create(
+ b, "merge_incoming_move_file_text_merge_crlf", opts, pool));
+
+ SVN_ERR(sbox_add_and_commit_greek_tree(b));
+ /* Edit the file to have CRLF line endings. */
+ SVN_ERR(sbox_file_write(b, "A/mu", "Original content.\r\n"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+ /* Create a copy of node "A". */
+ SVN_ERR(sbox_wc_update(b, "", SVN_INVALID_REVNUM));
+ SVN_ERR(sbox_wc_copy(b, "A", "A1"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+ /* On "trunk", move the file. */
+ SVN_ERR(sbox_wc_move(b, "A/mu", "A/mu-moved"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+ /* On "branch", edit the file. */
+ SVN_ERR(sbox_file_write(b, "A1/mu", "Modified content.\r\n"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+
+ SVN_ERR(sbox_wc_update(b, "", SVN_INVALID_REVNUM));
+ opt_rev.kind = svn_opt_revision_head;
+ opt_rev.value.number = SVN_INVALID_REVNUM;
+ SVN_ERR(svn_test__create_client_ctx(&ctx, b, pool));
+
+ /* Merge "A" to "A1". */
+ SVN_ERR(svn_client_merge_peg5(svn_path_url_add_component2(b->repos_url, "A",
+ pool),
+ NULL, &opt_rev, sbox_wc_path(b, "A1"),
+ svn_depth_infinity,
+ FALSE, FALSE, FALSE, FALSE, FALSE, FALSE,
+ NULL, ctx, pool));
+
+ /* We should have a tree conflict in the file "mu". */
+ SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, "A1/mu"), ctx,
+ pool, pool));
+ SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
+ &props_conflicted,
+ &tree_conflicted,
+ conflict, pool, pool));
+ SVN_TEST_ASSERT(!text_conflicted);
+ SVN_TEST_INT_ASSERT(props_conflicted->nelts, 0);
+ SVN_TEST_ASSERT(tree_conflicted);
+
+ /* Check available tree conflict resolution options. */
+ {
+ svn_client_conflict_option_id_t expected_opts[] = {
+ svn_client_conflict_option_postpone,
+ svn_client_conflict_option_accept_current_wc_state,
+ svn_client_conflict_option_incoming_delete_ignore,
+ svn_client_conflict_option_incoming_delete_accept,
+ -1 /* end of list */
+ };
+ SVN_ERR(assert_tree_conflict_options(conflict, ctx, expected_opts, pool));
+ }
+
+ SVN_ERR(svn_client_conflict_tree_get_details(conflict, ctx, pool));
+
+ {
+ svn_client_conflict_option_id_t expected_opts[] = {
+ svn_client_conflict_option_postpone,
+ svn_client_conflict_option_accept_current_wc_state,
+ svn_client_conflict_option_incoming_move_file_text_merge,
+ -1 /* end of list */
+ };
+ SVN_ERR(assert_tree_conflict_options(conflict, ctx, expected_opts, pool));
+ }
+
+ /* Resolve the tree conflict by moving "mu" to "mu-moved". */
+ SVN_ERR(svn_client_conflict_tree_resolve_by_id(
+ conflict, svn_client_conflict_option_incoming_move_file_text_merge,
+ ctx, pool));
+
+ /* The file should no longer be in conflict, and should not have a
+ * text conflict, because the contents are identical in "trunk" and
+ * in the "branch". */
+ SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, "A1/mu-moved"),
+ ctx, pool, pool));
+ SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
+ &props_conflicted,
+ &tree_conflicted,
+ conflict, pool, pool));
+ SVN_TEST_ASSERT(!text_conflicted);
+ SVN_TEST_INT_ASSERT(props_conflicted->nelts, 0);
+ SVN_TEST_ASSERT(!tree_conflicted);
+
+ /* And it should have expected contents. */
+ SVN_ERR(svn_stringbuf_from_file2(&buf, sbox_wc_path(b, "A1/mu-moved"),
+ pool));
+ SVN_TEST_STRING_ASSERT(buf->data, "Modified content.\r\n");
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_merge_incoming_move_file_text_merge_native_eol(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_test__sandbox_t *b = apr_palloc(pool, sizeof(*b));
+ svn_client_ctx_t *ctx;
+ svn_opt_revision_t opt_rev;
+ svn_client_conflict_t *conflict;
+ svn_boolean_t text_conflicted;
+ apr_array_header_t *props_conflicted;
+ svn_boolean_t tree_conflicted;
+ svn_stringbuf_t *buf;
+
+ SVN_ERR(svn_test__sandbox_create(
+ b, "merge_incoming_move_file_text_merge_native_eol", opts, pool));
+
+ SVN_ERR(sbox_add_and_commit_greek_tree(b));
+ /* Set svn:eol-style on a file and edit it. */
+ SVN_ERR(sbox_wc_propset(b, SVN_PROP_EOL_STYLE, "native", "A/mu"));;
+ SVN_ERR(sbox_file_write(b, "A/mu", "Original content.\n"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+ /* Create a copy of node "A". */
+ SVN_ERR(sbox_wc_update(b, "", SVN_INVALID_REVNUM));
+ SVN_ERR(sbox_wc_copy(b, "A", "A1"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+ /* On "trunk", move the file. */
+ SVN_ERR(sbox_wc_move(b, "A/mu", "A/mu-moved"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+ /* On "branch", edit the file. */
+ SVN_ERR(sbox_file_write(b, "A1/mu", "Modified content.\n"));
+ SVN_ERR(sbox_wc_commit(b, ""));
+
+ SVN_ERR(sbox_wc_update(b, "", SVN_INVALID_REVNUM));
+ opt_rev.kind = svn_opt_revision_head;
+ opt_rev.value.number = SVN_INVALID_REVNUM;
+ SVN_ERR(svn_test__create_client_ctx(&ctx, b, pool));
+
+ /* Merge "A" to "A1". */
+ SVN_ERR(svn_client_merge_peg5(svn_path_url_add_component2(b->repos_url, "A",
+ pool),
+ NULL, &opt_rev, sbox_wc_path(b, "A1"),
+ svn_depth_infinity,
+ FALSE, FALSE, FALSE, FALSE, FALSE, FALSE,
+ NULL, ctx, pool));
+
+ /* We should have a tree conflict in the file "mu". */
+ SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, "A1/mu"), ctx,
+ pool, pool));
+ SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
+ &props_conflicted,
+ &tree_conflicted,
+ conflict, pool, pool));
+ SVN_TEST_ASSERT(!text_conflicted);
+ SVN_TEST_INT_ASSERT(props_conflicted->nelts, 0);
+ SVN_TEST_ASSERT(tree_conflicted);
+
+ /* Check available tree conflict resolution options. */
+ {
+ svn_client_conflict_option_id_t expected_opts[] = {
+ svn_client_conflict_option_postpone,
+ svn_client_conflict_option_accept_current_wc_state,
+ svn_client_conflict_option_incoming_delete_ignore,
+ svn_client_conflict_option_incoming_delete_accept,
+ -1 /* end of list */
+ };
+ SVN_ERR(assert_tree_conflict_options(conflict, ctx, expected_opts, pool));
+ }
+
+ SVN_ERR(svn_client_conflict_tree_get_details(conflict, ctx, pool));
+
+ {
+ svn_client_conflict_option_id_t expected_opts[] = {
+ svn_client_conflict_option_postpone,
+ svn_client_conflict_option_accept_current_wc_state,
+ svn_client_conflict_option_incoming_move_file_text_merge,
+ -1 /* end of list */
+ };
+ SVN_ERR(assert_tree_conflict_options(conflict, ctx, expected_opts, pool));
+ }
+
+ /* Resolve the tree conflict by moving "mu" to "mu-moved". */
+ SVN_ERR(svn_client_conflict_tree_resolve_by_id(
+ conflict, svn_client_conflict_option_incoming_move_file_text_merge,
+ ctx, pool));
+
+ /* The file should no longer be in conflict, and should not have a
+ * text conflict, because the contents are identical in "trunk" and
+ * in the "branch". */
+ SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, "A1/mu-moved"),
+ ctx, pool, pool));
+ SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
+ &props_conflicted,
+ &tree_conflicted,
+ conflict, pool, pool));
+ SVN_TEST_ASSERT(!text_conflicted);
+ SVN_TEST_INT_ASSERT(props_conflicted->nelts, 0);
+ SVN_TEST_ASSERT(!tree_conflicted);
+
+ /* And it should have expected contents. */
+ SVN_ERR(svn_stringbuf_from_file2(&buf, sbox_wc_path(b, "A1/mu-moved"),
+ pool));
+ SVN_TEST_STRING_ASSERT(buf->data, "Modified content." APR_EOL_STR);
+
+ return SVN_NO_ERROR;
+}
+
/* ========================================================================== */
@@ -4764,6 +4975,10 @@ static struct svn_test_descriptor_t test
"update incoming add dir merge with obstructions"),
SVN_TEST_OPTS_PASS(test_cherry_pick_moved_file_with_propdel,
"cherry-pick with moved file and propdel"),
+ SVN_TEST_OPTS_PASS(test_merge_incoming_move_file_text_merge_crlf,
+ "merge incoming move file merge with CRLF eols"),
+ SVN_TEST_OPTS_PASS(test_merge_incoming_move_file_text_merge_native_eol,
+ "merge incoming move file merge with native eols"),
SVN_TEST_NULL
};