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 2015/09/30 15:01:29 UTC

svn commit: r1706020 - in /subversion/trunk/subversion: libsvn_client/patch.c tests/cmdline/patch_tests.py

Author: rhuijben
Date: Wed Sep 30 13:01:28 2015
New Revision: 1706020

URL: http://svn.apache.org/viewvc?rev=1706020&view=rev
Log:
Consolidate the code that decides to create a new file in 'svn patch' in one
location to fix some cases where a patch couldn't be re-applied. This fix
also fixes some notifications where non changes were reported as changes.

* subversion/libsvn_client/patch.c
  (patch_target_t): Remove replaced boolean. Just use added.
  (init_patch_target): Don't set properties as changed until something is
    applied.
  (send_patch_notification): Simplify check.
  (apply_one_patch): Create files for property only changes if necessary.
  (install_patched_prop_targets): Don't create files here. Let caller handle
    that in an earlier stage.

* subversion/tests/cmdline/patch_tests.py
  (patch_adds_executability_nocontents): Expect mer'g'ed output when already
    applied.
  (patch_empty_vs_delete): Extend test.

Modified:
    subversion/trunk/subversion/libsvn_client/patch.c
    subversion/trunk/subversion/tests/cmdline/patch_tests.py

Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=1706020&r1=1706019&r2=1706020&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Wed Sep 30 13:01:28 2015
@@ -242,10 +242,6 @@ typedef struct patch_target_t {
   /* True if the target ended up being deleted by the patch. */
   svn_boolean_t deleted;
 
-  /* True if the target ended up being replaced by the patch
-   * (i.e. a new file was added on top locally deleted node). */
-  svn_boolean_t replaced;
-
   /* Set if the target is supposed to be moved by the patch.
    * This applies to --git diffs which carry "rename from/to" headers. */
    const char *move_target_abspath;
@@ -1006,11 +1002,6 @@ init_patch_target(patch_target_t **patch
   target->prop_targets = apr_hash_make(result_pool);
   target->operation = patch->operation;
 
-  if (patch->new_executable_p != svn_tristate_unknown
-      && patch->new_executable_p != patch->old_executable_p)
-    /* May also be set by apply_hunk(). */
-    target->has_prop_changes = TRUE;
-
   SVN_ERR(resolve_target_path(target, choose_target_filename(patch),
                               root_abspath, strip_count, has_text_changes,
                               wc_ctx, result_pool, scratch_pool));
@@ -2192,7 +2183,7 @@ send_patch_notification(const patch_targ
     action = svn_wc_notify_skip;
   else if (target->deleted && !target->locally_deleted)
     action = svn_wc_notify_delete;
-  else if (target->added || target->replaced || target->move_target_abspath)
+  else if (target->added || target->move_target_abspath)
     action = svn_wc_notify_add;
   else
     action = svn_wc_notify_patch;
@@ -2713,6 +2704,7 @@ apply_one_patch(patch_target_t **patch_t
     {
       apr_finfo_t working_file;
       apr_finfo_t patched_file;
+      svn_boolean_t ensure_exists = FALSE;
 
       /* Get sizes of the patched temporary file and the working file.
        * We'll need those to figure out whether we should delete the
@@ -2759,14 +2751,49 @@ apply_one_patch(patch_target_t **patch_t
             target->skipped = TRUE;
           }
         }
-      else if (patched_file.size > 0 && working_file.size == 0
-               && patch->operation == svn_diff_op_unchanged)
+      else if (patched_file.size > 0 && working_file.size == 0)
         {
           /* The patch has created a file. */
-          if (target->locally_deleted)
-            target->replaced = TRUE;
-          else if (target->db_kind == svn_node_none)
-            target->added = TRUE;
+          ensure_exists = TRUE;
+        }
+      else if (target->operation == svn_diff_op_added)
+        {
+          /* Can't add something that is already there.
+             Report node as merged or conflict */
+          target->added = FALSE;
+        }
+
+      if (!ensure_exists
+          && !target->skipped
+          && target->has_prop_changes)
+        {
+          for (hash_index = apr_hash_first(scratch_pool, target->prop_targets);
+               hash_index;
+               hash_index = apr_hash_next(hash_index))
+            {
+              prop_patch_target_t *prop_target = apr_hash_this_val(hash_index);
+
+              if (prop_target->operation != svn_diff_op_deleted)
+              {
+                ensure_exists = TRUE;
+              }
+            }
+        }
+
+      if (ensure_exists)
+        {
+          if (patch->operation == svn_diff_op_unchanged
+              || patch->operation == svn_diff_op_added)
+            {
+              if (target->locally_deleted || target->db_kind == svn_node_none)
+                target->added = TRUE;
+            }
+          else if (target->locally_deleted
+                   || target->kind_on_disk == svn_node_none)
+            {
+              /* Can't modify something that isn't there */
+              target->skipped = TRUE;
+            }
         }
     }
 
@@ -2954,7 +2981,7 @@ install_patched_target(patch_target_t *t
   else
     {
       svn_node_kind_t parent_db_kind;
-      if (target->added || target->replaced)
+      if (target->added)
         {
           const char *parent_abspath;
 
@@ -3043,7 +3070,7 @@ install_patched_target(patch_target_t *t
                         ctx->cancel_func, ctx->cancel_baton, pool));
             }
 
-          if (target->added || target->replaced)
+          if (target->added)
             {
               /* The target file didn't exist previously,
                * so add it to version control.
@@ -3156,50 +3183,6 @@ install_patched_prop_targets(patch_targe
           continue;
         }
 
-
-      /* If the patch target doesn't exist yet, the patch wants to add an
-       * empty file with properties set on it. So create an empty file and
-       * add it to version control. But if the patch was in the 'git format'
-       * then the file has already been added.
-       *
-       * We only do this if the patch file didn't set an explicit operation.
-       *
-       * If there are text changes then we already handled these cases when
-       * constructing the patched file.
-       */
-      if (target->operation == svn_diff_op_unchanged
-          && !target->has_text_changes)
-        {
-          if (target->kind_on_disk == svn_node_none
-              && ! target->added
-              && ! target->replaced)
-            {
-              if (! dry_run)
-                {
-                  SVN_ERR(svn_io_file_create_empty(target->local_abspath,
-                                                   scratch_pool));
-                  SVN_ERR(svn_wc_add_from_disk3(ctx->wc_ctx, target->local_abspath,
-                                                NULL /*props*/,
-                                                FALSE /* skip checks */,
-                                                /* suppress notification */
-                                                NULL, NULL,
-                                                iterpool));
-                }
-              if (target->locally_deleted)
-                target->replaced = TRUE;
-              else
-                target->added = TRUE;
-            }
-        }
-      
-      if (target->kind_on_disk == svn_node_none
-          && !target->added
-          && !target->replaced)
-        {
-          target->skipped = TRUE;
-          break;
-        }
-
       /* Attempt to set the property, and reject all hunks if this
          fails.  If the property had a non-empty value, but now has
          an empty one, we'll just delete the property altogether.  */

Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/patch_tests.py?rev=1706020&r1=1706019&r2=1706020&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Wed Sep 30 13:01:28 2015
@@ -5986,6 +5986,7 @@ def patch_adds_executability_nocontents(
 
   # And try it again
   # This may produce different output but must have the same result
+  expected_output.tweak('iota', status=' G')
   svntest.actions.run_and_verify_patch(wc_dir, os.path.abspath(patch_file_path),
                                        expected_output, expected_disk,
                                        expected_status, expected_skip,
@@ -5994,6 +5995,7 @@ def patch_adds_executability_nocontents(
   # And then try it in reverse
   expected_disk.tweak('iota', props={})
   expected_status.tweak('iota', status='  ')
+  expected_output.tweak('iota', status=' U')
   svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
                                        expected_output, expected_disk,
                                        expected_status, expected_skip,
@@ -6001,6 +6003,7 @@ def patch_adds_executability_nocontents(
 
   # And try it again
   # This may produce different output but must have the same result
+  expected_output.tweak('iota', status=' G')
   svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
                                        expected_output, expected_disk,
                                        expected_status, expected_skip,
@@ -6536,6 +6539,39 @@ def patch_empty_vs_delete(sbox):
   svntest.actions.run_and_verify_svn(None, [],
                                      'rm', '--force', sbox.ospath('iota'))
 
+  expected_output.tweak('iota', status='A ')
+  svntest.actions.run_and_verify_patch(wc_dir, del_patch,
+                                       expected_output, expected_disk,
+                                       expected_status, expected_skip,
+                                       [], True, True,
+                                       '--strip', strip_count,
+                                       '--reverse-diff')
+  # And retry
+  expected_output.tweak('iota', status='G ')
+  svntest.actions.run_and_verify_patch(wc_dir, del_patch,
+                                       expected_output, expected_disk,
+                                       expected_status, expected_skip,
+                                       [], True, True,
+                                       '--strip', strip_count,
+                                       '--reverse-diff')
+
+  svntest.actions.run_and_verify_svn(None, [],
+                                     'rm', '--force', sbox.ospath('iota'))
+
+  expected_output.tweak('iota', status='A ')
+  svntest.actions.run_and_verify_patch(wc_dir, del_git_patch,
+                                       expected_output, expected_disk,
+                                       expected_status, expected_skip,
+                                       [], True, True,
+                                       '--reverse-diff')
+  # And retry
+  expected_output.tweak('iota', status='G ')
+  svntest.actions.run_and_verify_patch(wc_dir, del_git_patch,
+                                       expected_output, expected_disk,
+                                       expected_status, expected_skip,
+                                       [], True, True,
+                                       '--reverse-diff')
+
 ########################################################################
 #Run the tests