You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2021/01/23 04:00:16 UTC

svn commit: r1885826 - in /subversion/branches/1.10.x: ./ STATUS subversion/libsvn_client/merge.c subversion/libsvn_client/mergeinfo.h subversion/tests/cmdline/merge_tests.py

Author: svn-role
Date: Sat Jan 23 04:00:16 2021
New Revision: 1885826

URL: http://svn.apache.org/viewvc?rev=1885826&view=rev
Log:
Merge the 1.10.x-issue4859 branch:

 * r1878997, r1879192, r1879474, r1879959
   Fix issue #4859, Merge removing a folder with non-inheritable mergeinfo
   -> E155023: can't set properties: invalid status for updating properties
   Justification:
     Annoying failure, encountered in real use.
   Branch: ^/subversion/branches/1.10.x-issue4859
   Votes:
     +1: julianfoad, jcorvel, stsp

Modified:
    subversion/branches/1.10.x/   (props changed)
    subversion/branches/1.10.x/STATUS
    subversion/branches/1.10.x/subversion/libsvn_client/merge.c
    subversion/branches/1.10.x/subversion/libsvn_client/mergeinfo.h
    subversion/branches/1.10.x/subversion/tests/cmdline/merge_tests.py

Propchange: subversion/branches/1.10.x/
------------------------------------------------------------------------------
  Merged /subversion/branches/1.10.x-issue4859:r1879482-1885825
  Merged /subversion/trunk:r1878997,1879192,1879474,1879959

Modified: subversion/branches/1.10.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/STATUS?rev=1885826&r1=1885825&r2=1885826&view=diff
==============================================================================
--- subversion/branches/1.10.x/STATUS (original)
+++ subversion/branches/1.10.x/STATUS Sat Jan 23 04:00:16 2021
@@ -89,12 +89,3 @@ Veto-blocked changes:
 
 Approved changes:
 =================
-
- * r1878997, r1879192, r1879474, r1879959
-   Fix issue #4859, Merge removing a folder with non-inheritable mergeinfo
-   -> E155023: can't set properties: invalid status for updating properties
-   Justification:
-     Annoying failure, encountered in real use.
-   Branch: ^/subversion/branches/1.10.x-issue4859
-   Votes:
-     +1: julianfoad, jcorvel, stsp

Modified: subversion/branches/1.10.x/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/libsvn_client/merge.c?rev=1885826&r1=1885825&r2=1885826&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/libsvn_client/merge.c (original)
+++ subversion/branches/1.10.x/subversion/libsvn_client/merge.c Sat Jan 23 04:00:16 2021
@@ -328,7 +328,7 @@ typedef struct merge_cmd_baton_t {
 
     /* Reference to the one-and-only CHILDREN_WITH_MERGEINFO (see global
        comment) or a similar list for single-file-merges */
-    const apr_array_header_t *nodes_with_mergeinfo;
+    apr_array_header_t *nodes_with_mergeinfo;
   } notify_begin;
 
 } merge_cmd_baton_t;
@@ -1542,6 +1542,25 @@ record_update_delete(merge_cmd_baton_t *
                     svn_node_kind_to_word(kind));
     }
 
+  /* Note in children_with_mergeinfo that all paths in this subtree are
+   * being deleted, to avoid trying to set mergeinfo on them later. */
+  if (merge_b->notify_begin.nodes_with_mergeinfo)
+    {
+      int i;
+
+      for (i = 0; i < merge_b->notify_begin.nodes_with_mergeinfo->nelts; i++)
+        {
+          svn_client__merge_path_t *child
+            = APR_ARRAY_IDX(merge_b->notify_begin.nodes_with_mergeinfo, i,
+                            svn_client__merge_path_t *);
+
+          if (svn_dirent_is_ancestor(local_abspath, child->abspath))
+            {
+              svn_sort__array_delete(merge_b->notify_begin.nodes_with_mergeinfo, i--, 1);
+            }
+        }
+    }
+
   return SVN_NO_ERROR;
 }
 
@@ -5552,7 +5571,7 @@ svn_client__make_merge_conflict_error(sv
    with paths (svn_client__merge_path_t *) arranged in depth first order,
    which have mergeinfo set on them or meet one of the other criteria
    defined in get_mergeinfo_paths().  Remove any paths absent from disk
-   or scheduled for deletion from CHILDREN_WITH_MERGEINFO which are equal to
+   from CHILDREN_WITH_MERGEINFO which are equal to
    or are descendants of TARGET_WCPATH by setting those children to NULL. */
 static void
 remove_absent_children(const char *target_wcpath,
@@ -5566,7 +5585,7 @@ remove_absent_children(const char *targe
     {
       svn_client__merge_path_t *child =
         APR_ARRAY_IDX(children_with_mergeinfo, i, svn_client__merge_path_t *);
-      if ((child->absent || child->scheduled_for_deletion)
+      if (child->absent
           && svn_dirent_is_ancestor(target_wcpath, child->abspath))
         {
           svn_sort__array_delete(children_with_mergeinfo, i--, 1);

Modified: subversion/branches/1.10.x/subversion/libsvn_client/mergeinfo.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/libsvn_client/mergeinfo.h?rev=1885826&r1=1885825&r2=1885826&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/libsvn_client/mergeinfo.h (original)
+++ subversion/branches/1.10.x/subversion/libsvn_client/mergeinfo.h Sat Jan 23 04:00:16 2021
@@ -74,8 +74,6 @@ typedef struct svn_client__merge_path_t
                                            prior to a merge.  May be NULL. */
   svn_boolean_t inherited_mergeinfo;    /* Whether PRE_MERGE_MERGEINFO was
                                            explicit or inherited. */
-  svn_boolean_t scheduled_for_deletion; /* ABSPATH is scheduled for
-                                           deletion. */
   svn_boolean_t immediate_child_dir;    /* ABSPATH is an immediate child
                                            directory of the merge target,
                                            has no explicit mergeinfo prior

Modified: subversion/branches/1.10.x/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/tests/cmdline/merge_tests.py?rev=1885826&r1=1885825&r2=1885826&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/branches/1.10.x/subversion/tests/cmdline/merge_tests.py Sat Jan 23 04:00:16 2021
@@ -18530,6 +18530,146 @@ def merge_dir_delete_force(sbox):
                                      'merge', '-c2', '^/', sbox.wc_dir,
                                      '--ignore-ancestry', '--force')
 
+# Issue #4859: Merge removing a folder with non-inheritable mergeinfo ->
+# E155023: can't set properties: invalid status for updating properties
+@Issue(4859)
+def merge_deleted_folder_with_mergeinfo(sbox):
+  "merge deleted folder with mergeinfo"
+
+  sbox.build()
+
+  was_cwd = os.getcwd()
+  os.chdir(sbox.wc_dir)
+  sbox.wc_dir = ''
+
+  # Some non-inheritable mergeinfo
+  sbox.simple_propset('svn:mergeinfo', '/A/C:1*', 'A/D')
+  sbox.simple_commit() # r2
+
+  # Branching
+  sbox.simple_repo_copy('A', 'branch_A')  # r3
+  sbox.simple_update()
+
+  # On branch, remove a folder that has non-inheritable mergeinfo
+  sbox.simple_rm('branch_A/D')
+  sbox.simple_commit() # r4
+
+  sbox.simple_update()
+
+  # A merge that removes that folder
+  # (merge revision 4 only from 'branch_A' to 'A')
+  expected_output = wc.State(sbox.ospath(''), {
+    'A/D'    : Item(status='D '),
+    })
+  expected_mergeinfo_output = wc.State(sbox.ospath(''), {
+    'A'      : Item(status=' U'),
+    })
+  expected_status = svntest.actions.get_virginal_state(sbox.ospath('A'), 4).subtree('A')
+  expected_status.add({ '': Item(status=' M', wc_rev=4) })
+  expected_status.tweak_some(
+    lambda path, item: [True] if path.split('/')[0] == 'D' else [],
+    status='D ')
+  svntest.actions.run_and_verify_merge(sbox.ospath('A'), 3, 4,
+                                       '^/branch_A', None,
+                                       expected_output,
+                                       expected_mergeinfo_output,
+                                       None,
+                                       None,
+                                       expected_status,
+                                       wc.State('', {}),
+                                       [],
+                                       check_props=False,
+                                       dry_run=False  # as dry run is broken
+                                       )
+
+  os.chdir(was_cwd)
+
+# Issue #4859: Merge removing a folder with non-inheritable mergeinfo ->
+# E155023: can't set properties: invalid status for updating properties
+#
+# In this test we split the merge into two separate operable parts, a
+# delete followed later by an add, to check it will set the mergeinfo on the
+# subtree paths if the deleted folder is later replaced within the same
+# overall merge.
+@Issue(4859)
+def merge_deleted_folder_with_mergeinfo_2(sbox):
+  "merge deleted folder with mergeinfo 2"
+
+  sbox.build()
+
+  was_cwd = os.getcwd()
+  os.chdir(sbox.wc_dir)
+  sbox.wc_dir = ''
+
+  # Some non-inheritable mergeinfo
+  sbox.simple_propset('svn:mergeinfo', '/A/C:1*', 'A/D')
+  sbox.simple_commit() # r2
+
+  # Branching
+  sbox.simple_repo_copy('A', 'branch_A')  # r3
+  sbox.simple_update()
+
+  # On branch, remove a folder that has non-inheritable mergeinfo
+  sbox.simple_rm('branch_A/D')
+  sbox.simple_commit() # r4
+
+  # A commit that we don't want to merge from the branch, to split the merge
+  # into two separate operable parts.
+  sbox.simple_mkdir('branch_A/IgnoreThis')
+  sbox.simple_commit() # r5
+
+  # On branch, replace the deleted folder with a new one, with mergeinfo,
+  # to check we don't omit setting mergeinfo on this.
+  sbox.simple_mkdir('branch_A/D')
+  sbox.simple_propset('svn:mergeinfo', '/branch_B/C:1*', 'branch_A/D')
+  sbox.simple_mkdir('branch_A/D/G', 'branch_A/D/G2')
+  sbox.simple_propset('svn:mergeinfo', '/branch_B/C/G:1*', 'branch_A/D/G')
+  sbox.simple_propset('svn:mergeinfo', '/branch_B/C/G2:1*', 'branch_A/D/G2')
+  sbox.simple_commit() # r6
+
+  sbox.simple_propset('svn:mergeinfo', '/branch_A:5', 'A')
+  sbox.simple_commit() # r7
+
+  sbox.simple_update()
+
+  # A merge that removes that folder
+  expected_output = wc.State(sbox.ospath(''), {
+    'A/D'    : Item(status='A ', prev_status='D '),
+    'A/D/G'  : Item(status='A '),
+    'A/D/G2' : Item(status='A '),
+    })
+  # verify that mergeinfo is set/changed on A/D, A/D/G, A/D/G2.
+  #expected_mergeinfo_output = wc.State(sbox.ospath(''), {
+  #  'A'      : Item(status=' U'),
+  #  'A/D'    : Item(status=' G'),
+  #  'A/D/G'  : Item(status=' G'),  # varies, G or U: see issue #4862
+  #  'A/D/G2' : Item(status=' G'),  # varies, G or U: see issue #4862
+  #  })
+  expected_status = svntest.actions.get_virginal_state(sbox.ospath('A'), 7).subtree('A')
+  expected_status.tweak_some(
+    lambda path, item: [True] if path.split('/')[0] == 'D' else [],
+    status='D ')
+  expected_status.add({
+    ''     : Item(status=' M', wc_rev=7),
+    'D'    : Item(status='RM', copied='+', wc_rev='-'),
+    'D/G'  : Item(status=' M', copied='+', wc_rev='-'),
+    'D/G2' : Item(status=' M', copied='+', wc_rev='-'),
+    })
+  svntest.actions.run_and_verify_merge(sbox.ospath('A'), None, None,
+                                       '^/branch_A', None,
+                                       expected_output,
+                                       None, #expected_mergeinfo_output
+                                       None,
+                                       None,
+                                       expected_status,
+                                       wc.State('', {}),
+                                       [],
+                                       check_props=False,
+                                       dry_run=False  # as dry run is broken
+                                       )
+
+  os.chdir(was_cwd)
+
 ########################################################################
 # Run the tests
 
@@ -18677,6 +18817,8 @@ test_list = [ None,
               merge_to_empty_target_merge_to_infinite_target,
               conflict_naming,
               merge_dir_delete_force,
+              merge_deleted_folder_with_mergeinfo,
+              merge_deleted_folder_with_mergeinfo_2,
              ]
 
 if __name__ == '__main__':