You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by pb...@apache.org on 2011/12/07 22:38:47 UTC

svn commit: r1211647 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tests.py

Author: pburba
Date: Wed Dec  7 21:38:46 2011
New Revision: 1211647

URL: http://svn.apache.org/viewvc?rev=1211647&view=rev
Log:
Fix new issue #4057 bug where a --depth=immediate merge, which touches only
immediate children, was still recording spurious non-inheritable subtree
mergeinfo.

* subversion/libsvn_client/merge.c

  (calculate_merge_inheritance): Remove.  This function had become so simple
   in r1211199 (and could be made simpler still) that it made more sense to
   move the code inline to the sole caller.

  (log_find_operative_subtree_revs): During --depth=immediates merges, don't
   consider a change to an immediate directory child as an operative change
   *unless* that also touches proper subtrees of the child.

  (get_operative_immediate_children): Clarify doc string: This function 
   only gets *immediate* children of the merge target.  Also update the doc
   string to reflect the changes made to the log_find_operative_subtree_revs
   callback: It's *additional* differences between a shallow and infinite
   depth merge we get. Lastly, fix a typo.

  (flag_subtrees_needing_mergeinfo): Move what was left of
   calculate_merge_inheritance's logic into this function and better
   document the logic.

* subversion/tests/cmdline/merge_tests.py

  (unnecessary_noninheritable_mergeinfo_shallow_merge): Remove XFail
   decorator and tweak comments re failure status.

Modified:
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/tests/cmdline/merge_tests.py

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1211647&r1=1211646&r2=1211647&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Wed Dec  7 21:38:46 2011
@@ -4507,61 +4507,6 @@ populate_remaining_ranges(apr_array_head
 
 /*** Other Helper Functions ***/
 
-/* Helper for record_mergeinfo_for_dir_merge().
-
-   Set *NON_INHERITABLE to TRUE if non-inheritable mergeinfo is needed to
-   describe a merge into LOCAL_ABSPATH at DEPTH.  Set *NON_INHERTIABLE to
-   FALSE otherwise.
-
-   WC_PATH_IS_MERGE_TARGET is true if WC_PATH is the target of the merge,
-   otherwise WC_PATH is a subtree.
-
-   WC_PATH_HAS_MISSING_CHILD is true if WC_PATH is missing an immediate child
-   because the child is switched or absent from the WC, or due to a sparse
-   checkout (see get_mergeinfo_paths) or because DEPTH is shallow
-   (i.e. < svn_depth_infinity) and the merge would affect a child if
-   performed at a deeper depth.
-
-   Perform any temporary allocations in SCRATCH_POOL. */
-static svn_error_t *
-calculate_merge_inheritance(svn_boolean_t *non_inheritable,
-                            const char *local_abspath,
-                            svn_boolean_t wc_path_is_merge_target,
-                            svn_boolean_t wc_path_has_missing_child,
-                            svn_depth_t depth,
-                            svn_wc_context_t *wc_ctx,
-                            apr_pool_t * scratch_pool)
-{
-  svn_node_kind_t path_kind;
-
-  SVN_ERR(svn_wc_read_kind(&path_kind, wc_ctx, local_abspath, FALSE,
-                           scratch_pool));
-
-  /* Starting assumption. */
-  *non_inheritable = FALSE;
-
-  /* Only directories can have non-inheritable mergeinfo. */
-  if (path_kind == svn_node_dir)
-    {
-      if (wc_path_is_merge_target)
-        {
-          if (wc_path_has_missing_child)
-            {
-              *non_inheritable = TRUE;
-            }
-        }
-      else /* WC_PATH is a directory subtree of the target. */
-        {
-          if (wc_path_has_missing_child
-              || depth == svn_depth_immediates)
-            {
-              *non_inheritable = TRUE;
-            }
-        }
-    }
-  return SVN_NO_ERROR;
-}
-
 /* Calculate the new mergeinfo for the target tree rooted at TARGET_ABSPATH
    based on MERGES (a mapping of absolute WC paths to rangelists representing
    a merge from the source SOURCE_FSPATH).
@@ -7331,6 +7276,14 @@ log_find_operative_subtree_revs(void *ba
               if (log_baton->depth == svn_depth_files
                   && change->node_kind == svn_node_file)
                 continue;
+
+              /* If depth is svn_depth_immediates, then we only care
+                 about changes to proper subtrees of PATH.  If the change
+                 is to PATH itself then PATH is within the operational
+                 depth of the merge. */
+              if (log_baton->depth == svn_depth_immediates)
+                continue;
+
               child = rel_path;
             }
 
@@ -7353,7 +7306,8 @@ log_find_operative_subtree_revs(void *ba
   return SVN_NO_ERROR;
 }
 
-/* Find operative subtrees if record_mergeinfo_for_dir_merge() were
+/* Find immediate subtrees of MERGE_TARGET_ABSPATH which would have
+   additional differences applied if record_mergeinfo_for_dir_merge() were
    recording mergeinfo describing a merge at svn_depth_infinity, rather
    than at DEPTH (which is assumed to be shallow; if
    DEPTH == svn_depth_infinity then this function does nothing beyond
@@ -7371,9 +7325,10 @@ log_find_operative_subtree_revs(void *ba
 
    Set *OPERATIVE_CHILDREN to a hash (mapping const char * absolute
    working copy paths to those path's const char * repos absolute paths)
-   containing all the subtrees which would be inoperative if
-   MERGE_SOURCE_REPOS_PATH -r(OLDEST_REV - 1):YOUNGEST_REV were merged to
-   MERGE_TARGET_ABSPATH at svn_depth_infinity.
+   containing all the immediate subtrees of MERGE_TARGET_ABSPATH which would
+   have a different diff applied if MERGE_SOURCE_REPOS_PATH
+   -r(OLDEST_REV - 1):YOUNGEST_REV were merged to MERGE_TARGET_ABSPATH at
+   svn_depth_infinity rather than DEPTH.
 
    RESULT_POOL is used to allocate the contents of *OPERATIVE_CHILDREN.
    SCRATCH_POOL is used for temporary allocations. */
@@ -7581,26 +7536,57 @@ flag_subtrees_needing_mergeinfo(svn_bool
 
       if (child->record_mergeinfo)
         {
-          svn_boolean_t missing_child =
-            child->missing_child || child->switched_child;
+          /* We need to record mergeinfo, but should that mergeinfo be
+             non-inheritable? */
+          svn_node_kind_t path_kind;
+          SVN_ERR(svn_wc_read_kind(&path_kind, merge_b->ctx->wc_ctx,
+                                   child->abspath, FALSE, iterpool));
+
+          /* Only directories can have non-inheritable mergeinfo. */
+          if (path_kind == svn_node_dir)
+            {
+              /* There are two general cases where non-inheritable mergeinfo
+                 is required:
+
+                 1) There merge target has missing subtrees (due to authz
+                    restrictions, switched subtrees, or a shallow working
+                    copy).
+
+                 2) The operational depth of the merge itself is shallow. */
+
+              /* We've already determined the first case. */
+              child->record_noninheritable =
+                child->missing_child || child->switched_child;
 
-          /* If, during a shallow merge, the merge target has immediate
-             children that would be affected by a deeper merge, then
-             consider the merge target as missing a child for the purposes
-             of recording non-inheritable mergeinfo. */
-          if (i == 0
-              && depth < svn_depth_immediates
-              && operative_immediate_children
-              && apr_hash_count(operative_immediate_children))
-            missing_child = TRUE;
-
-          SVN_ERR(calculate_merge_inheritance(&(child->record_noninheritable),
-                                              child->abspath,
-                                              i == 0,
-                                              missing_child,
-                                              depth,
-                                              merge_b->ctx->wc_ctx,
-                                              iterpool));
+              /* The second case requires a bit more work. */
+              if (i == 0)
+                {
+                  /* If CHILD is the root of the merge target and the
+                     operational depth is empty or files, then the mere
+                     existence of operative immediate children means we
+                     must record non-inheritable mergeinfo.
+                     
+                     ### What about svn_depth_immediates?  In that case
+                     ### the merge target needs only normal inheritable
+                     ### mergeinfo and the target's immediate children will
+                     ### get non-inheritable mergeinfo, assuming they
+                     ### need even that. */
+                  if (depth < svn_depth_immediates
+                      && operative_immediate_children
+                      && apr_hash_count(operative_immediate_children))
+                    child->record_noninheritable = TRUE;
+                }
+              else if (depth == svn_depth_immediates)
+                {
+                  /* An immediate directory child of the merge target, which
+                      was affected by a --depth=immediates merge, needs
+                      non-inheritable mergeinfo. */
+                  if (apr_hash_get(operative_immediate_children,
+                                   child->abspath,
+                                   APR_HASH_KEY_STRING))
+                    child->record_noninheritable = TRUE;
+                }
+            }
         }
       else
         {

Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1211647&r1=1211646&r2=1211647&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Wed Dec  7 21:38:46 2011
@@ -17241,7 +17241,6 @@ def unnecessary_noninheritable_mergeinfo
 # Test for issue #4057 "don't record non-inheritable mergeinfo in shallow
 # merge if entire diff is within requested depth".
 @Issue(4057)
-@XFail()
 @SkipUnless(server_has_mergeinfo)
 def unnecessary_noninheritable_mergeinfo_shallow_merge(sbox):
   "shallow merge reaches all neccessary subtrees"
@@ -17296,11 +17295,10 @@ def unnecessary_noninheritable_mergeinfo
 
   # Merge r4 from ^/A/B to branch/B at operational depth=immediates
   #
-  # Despite the recent fixes to issue #4057, this still fails because
-  # the mergetracking logic doesn't realize that despite being a shallow
-  # merge, the diff can only affect branch/B/E, which is within the specified
-  # depth, so there is no need to record non-inheritable mergeinfo
-  # or subtree mergeinfo:
+  # Previously this failed because the mergetracking logic didn't realize
+  # that despite being a shallow merge, the diff only affected branch/B/E,
+  # which was within the specified depth, so there was no need to record
+  # non-inheritable mergeinfo or subtree mergeinfo:
   #
   #   >svn pg svn:mergeinfo -vR
   #   Properties on 'branch\B':