I've noticed that in some cases a working copy can become inoperable
after merge text conflict resolution with `svn_wc_conflict_choose_merged'
option.

The problem can occur in the following scenario:

  1. There is a text conflict, that is resolved with
     `svn_wc_conflict_choose_merged' option and specifying some
     path as MERGED_FILE.  According to the API it can be any file,
     let's say "D:\merged.txt".

  2. Previous step adds a FILE_INSTALL item into workqueue with a
     reference to "D:\merged.txt" and then runs the workqueue.

  3. An error happens when running the workqueue.
     For example a working file cannot be overwritten because
     of 'Access denied' error.

  4. The "D:\merged.txt" file gets deleted for some reason.

  5. At this point the working copy is inoperable and cannot be
     fixed by 'svn cleanup', because workqueue contains absolute
     path to non-existing file ("D:\merged.txt").

I've attached a patch with fix for this problem.  The patch contains a
simplified regression test that reproduces problem by returning
non-existing path from resolver callback.

Log message:
[[[
Make sure resolving a conflict with `svn_wc_conflict_choose_merged' can't
break the workqueue by referencing a non-existing file

* subversion/libsvn_wc/conflicts.c
  (build_text_conflict_resolve_items): Create private copy of MERGED_FILE
   contents in case of `svn_wc_conflict_choose_merged' is specified.
   Doing that to avoid referencing a potentially absent file in FILE_INSTALL
   workqueue record.

Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]]
Index: subversion/libsvn_wc/conflicts.c
===================================================================
--- subversion/libsvn_wc/conflicts.c    (revision 1900114)
+++ subversion/libsvn_wc/conflicts.c    (working copy)
@@ -1706,9 +1706,49 @@ build_text_conflict_resolve_items(svn_skel_t **wor
            good to use". */
       case svn_wc_conflict_choose_merged:
         {
-          install_from_abspath = merged_file
-                                  ? merged_file
-                                  : local_abspath;
+          if (merged_file)
+            {
+              /* Create private copy of merged MERGED_FILE contents to
+                 avoid referencing a potentially absent file in FILE_INSTALL
+                 workqueue record. */
+
+              const char *temp_dir_abspath;
+              const char *temp_file_abspath;
+              svn_stream_t *merged_contents;
+              svn_stream_t *tmp_contents;
+              apr_file_t *file;
+
+              SVN_ERR(svn_io_file_open(&file, merged_file,
+                                       APR_READ, APR_OS_DEFAULT,
+                                       scratch_pool));
+              merged_contents = svn_stream_from_aprfile2(file, FALSE,
+                                                         scratch_pool);
+
+              SVN_ERR(svn_wc__db_temp_wcroot_tempdir(&temp_dir_abspath, db,
+                                                     local_abspath,
+                                                     scratch_pool,
+                                                     scratch_pool));
+
+              SVN_ERR(svn_stream_open_unique(&tmp_contents,
+                                             &temp_file_abspath,
+                                             temp_dir_abspath,
+                                             svn_io_file_del_none,
+                                             scratch_pool, scratch_pool));
+
+              SVN_ERR(svn_stream_copy3(merged_contents, tmp_contents,
+                                       cancel_func, cancel_baton,
+                                       scratch_pool));
+
+              install_from_abspath = temp_file_abspath;
+
+              /* Remove temp file after successful installation. */
+              remove_source = TRUE;
+            }
+          else
+            {
+              /* Just reference the local file itself. */
+              install_from_abspath = local_abspath;
+            }
           break;
         }
       case svn_wc_conflict_choose_postpone:
Index: subversion/tests/libsvn_client/conflicts-test.c
===================================================================
--- subversion/tests/libsvn_client/conflicts-test.c     (revision 1900114)
+++ subversion/tests/libsvn_client/conflicts-test.c     (working copy)
@@ -7459,7 +7459,110 @@ test_update_file_move_vs_file_move_accept_move(con
   return SVN_NO_ERROR;
 }
 
+typedef struct {
+  const char *file_local_abspath;
+  const char *non_exising_file_abspath;
+} choose_merged_non_existing_path_conflict_baton_t;
 
+static svn_error_t *
+choose_merged_non_existing_path_conflict_func(
+  svn_wc_conflict_result_t **result,
+  const svn_wc_conflict_description2_t *description,
+  void *baton,
+  apr_pool_t *result_pool,
+  apr_pool_t *scratch_pool)
+{
+  choose_merged_non_existing_path_conflict_baton_t *b = baton;
+
+  if (strcmp(description->local_abspath, b->file_local_abspath) == 0
+      && description->kind == svn_wc_conflict_kind_text)
+    {
+      /* 'Resolve' conflict by specifying non-existing file. */
+      *result = svn_wc_create_conflict_result(svn_wc_conflict_choose_merged,
+                                              b->non_exising_file_abspath,
+                                              result_pool);
+
+      return SVN_NO_ERROR;
+    }
+  else
+    {
+      /* We should see only text conflict on 'newfile.txt'. */
+      return svn_error_create(SVN_ERR_TEST_FAILED, NULL,
+                              "Unexpected call to conflict resolver func");
+    }
+}
+
+static svn_error_t *
+test_resolve_choose_merged_non_existing_path(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_client_conflict_t *conflict;
+  svn_boolean_t text_conflicted;
+  apr_array_header_t *props_conflicted;
+  svn_boolean_t tree_conflicted;
+  choose_merged_non_existing_path_conflict_baton_t conflict_baton;
+
+  SVN_ERR(svn_test__sandbox_create(
+    b, "test_resolve_choose_merged_non_existing_path", opts, pool));
+
+  SVN_ERR(svn_test__create_client_ctx(&ctx, b, b->pool));
+
+  SVN_ERR(sbox_add_and_commit_greek_tree(b)); /* r1 */
+
+  /* Create a simple text conflict on file. */
+
+  /* Write some content in text file. */
+  SVN_ERR(sbox_file_write(b, "A/mu", "Original content.\n"));
+  SVN_ERR(sbox_wc_commit(b, "")); /* r2 */
+
+  /* Update text file. */
+  SVN_ERR(sbox_file_write(b, "A/mu", "Modified content.\n"));
+  SVN_ERR(sbox_wc_commit(b, "")); /* r3 */
+
+  /* Update working copy to r2 and modify the file locally. */
+  SVN_ERR(sbox_wc_update(b, "", 2));
+  SVN_ERR(sbox_file_write(b, "A/mu", "Local modification.\n"));
+
+  /* Update the working copy. */
+  SVN_ERR(sbox_wc_update(b, "", SVN_INVALID_REVNUM));
+
+  /* Ensure that the expected text conflict is present. */
+  SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, "A/mu"),
+                                ctx, b->pool, b->pool));
+  SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
+                                             &props_conflicted,
+                                             &tree_conflicted,
+                                             conflict, b->pool, b->pool));
+  SVN_TEST_ASSERT(text_conflicted &&
+                  props_conflicted->nelts == 0 &&
+                  !tree_conflicted);
+
+  /* Set-up conflict resolver callback. */
+  conflict_baton.file_local_abspath = sbox_wc_path(b, "A/mu");
+  conflict_baton.non_exising_file_abspath = sbox_wc_path(b, "non-existing");
+  ctx->conflict_func2 = choose_merged_non_existing_path_conflict_func;
+  ctx->conflict_baton2 = &conflict_baton;
+
+  /* 'Resolve' the conflict -- get an error. */
+  SVN_TEST_ASSERT_ERROR(svn_client_resolve(sbox_wc_path(b, "A/mu"),
+                                           svn_depth_empty,
+                                           svn_wc_conflict_choose_unspecified,
+                                           ctx, b->pool),
+                        SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE);
+
+  /* Clear the resolver callback. */
+  ctx->conflict_func2 = NULL;
+  ctx->conflict_baton2 = NULL;
+
+  /* Working copy is still operable; we shouldn't get any error here. */
+  SVN_ERR(sbox_wc_revert(b, b->wc_abspath, svn_depth_infinity));
+
+  return SVN_NO_ERROR;
+}
+
+
 /* ========================================================================== 
*/
 
 
@@ -7588,6 +7691,8 @@ static struct svn_test_descriptor_t test_funcs[] =
                        "file move vs file move during update"),
     SVN_TEST_OPTS_PASS(test_update_file_move_vs_file_move_accept_move,
                        "file move vs file move during update accept move"),
+    SVN_TEST_OPTS_PASS(test_resolve_choose_merged_non_existing_path,
+                       "resolve text conflict with non-existing path"),
     SVN_TEST_NULL
   };
 

Reply via email to