subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hwri...@apache.org
Subject svn commit: r895677 - in /subversion/branches/1.6.x: ./ CHANGES STATUS subversion/libsvn_client/merge.c subversion/libsvn_subr/mergeinfo.c subversion/tests/libsvn_subr/mergeinfo-test.c
Date Mon, 04 Jan 2010 16:16:32 GMT
Author: hwright
Date: Mon Jan  4 16:16:23 2010
New Revision: 895677

URL: http://svn.apache.org/viewvc?rev=895677&view=rev
Log:
Reintegrate the 1.6.x-r39019 branch:

 * r879093
   Fix bug where svn_[rangelist|mergeinfo]_[merge|intersect|remove|diff]
   APIs can modify their *non*-output arguments.
   Justification:
     No reports of this causing any problems that I know of, which is
     probably due to the fact that users of an API like svn_mergeinfo_merge
     typically only care about the output arguments.  The new C tests added
     to mergeinfo-test.c clearly demonstrate the bug.
   Branch:
     Resolves a minor conflict in libsvn_client/merge.c where the code
     changed was refactored on trunk.
     ^/subversion/branches/1.6.x-r39019
     The relevant commit on the branch is r879175.
   Votes:
     +1: pburba, julianfoad, rhuijben

Modified:
    subversion/branches/1.6.x/   (props changed)
    subversion/branches/1.6.x/CHANGES   (props changed)
    subversion/branches/1.6.x/STATUS
    subversion/branches/1.6.x/subversion/libsvn_client/merge.c
    subversion/branches/1.6.x/subversion/libsvn_subr/mergeinfo.c
    subversion/branches/1.6.x/subversion/tests/libsvn_subr/mergeinfo-test.c

Propchange: subversion/branches/1.6.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Jan  4 16:16:23 2010
@@ -17,6 +17,8 @@
 /subversion/branches/1.6.x-r38572:875006-875011
 /subversion/branches/1.6.x-r38799:875225-875262
 /subversion/branches/1.6.x-r38927:875347-875521
+/subversion/branches/1.6.x-r39019:879132-895676
+/subversion/branches/1.6.x-r39109:879131
 /subversion/branches/1.6.x-r39557:876013-876252
 /subversion/branches/1.6.x-r39887:876369-876411
 /subversion/branches/1.6.x-r40452:880530-890996
@@ -50,4 +52,4 @@
 /subversion/branches/tc_url_rev:870696-870828
 /subversion/branches/tree-conflicts:864636-869499
 /subversion/branches/tree-conflicts-notify:870271-870353
-/subversion/trunk:875976,875980-875981,876054-876056,876092,876175,876299,876306,876427,876440,876450,876507,876571,879688,880274-880275,880370,880474,880525-880526,881905,884842,886164,886197,888979,889081,889840,895653
+/subversion/trunk:875976,875980-875981,876054-876056,876092,876175,876299,876306,876427,876440,876450,876507,876571,879093,879688,880274-880275,880370,880474,880525-880526,881905,884842,886164,886197,888979,889081,889840,895653

Propchange: subversion/branches/1.6.x/CHANGES
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Jan  4 16:16:23 2010
@@ -17,6 +17,8 @@
 /subversion/branches/1.6.x-r38572/CHANGES:875006-875011
 /subversion/branches/1.6.x-r38799/CHANGES:875225-875262
 /subversion/branches/1.6.x-r38927/CHANGES:875347-875521
+/subversion/branches/1.6.x-r39019/CHANGES:879132-895676
+/subversion/branches/1.6.x-r39109/CHANGES:879131
 /subversion/branches/1.6.x-r39557/CHANGES:876013-876252
 /subversion/branches/1.6.x-r39887/CHANGES:876369-876411
 /subversion/branches/1.6.x-r40452/CHANGES:880530-890996

Modified: subversion/branches/1.6.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=895677&r1=895676&r2=895677&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Mon Jan  4 16:16:23 2010
@@ -170,19 +170,3 @@
 
 Approved changes:
 =================
-
- * r879093
-   Fix bug where svn_[rangelist|mergeinfo]_[merge|intersect|remove|diff]
-   APIs can modify their *non*-output arguments.
-   Justification:
-     No reports of this causing any problems that I know of, which is
-     probably due to the fact that users of an API like svn_mergeinfo_merge
-     typically only care about the output arguments.  The new C tests added
-     to mergeinfo-test.c clearly demonstrate the bug.
-   Branch:
-     Resolves a minor conflict in libsvn_client/merge.c where the code
-     changed was refactored on trunk.
-     ^/subversion/branches/1.6.x-r39019
-     The relevant commit on the branch is r879175.
-   Votes:
-     +1: pburba, julianfoad, rhuijben

Modified: subversion/branches/1.6.x/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_client/merge.c?rev=895677&r1=895676&r2=895677&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_client/merge.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_client/merge.c Mon Jan  4 16:16:23 2010
@@ -3381,12 +3381,27 @@
   if (is_subtree)
     {
       apr_array_header_t *deleted_rangelist, *added_rangelist;
+      svn_boolean_t is_rollback = revision2 < revision1;
+
+      /* If this is a reverse merge reorder CHILD->REMAINING_RANGES
+         so it will work with the svn_rangelist_diff API. */
+      if (is_rollback)
+        {
+          SVN_ERR(svn_rangelist_reverse(child->remaining_ranges, pool));
+          SVN_ERR(svn_rangelist_reverse(parent->remaining_ranges, pool));
+        }
 
       SVN_ERR(svn_rangelist_diff(&deleted_rangelist, &added_rangelist,
                                  child->remaining_ranges,
                                  parent->remaining_ranges,
                                  TRUE, pool));
 
+      if (is_rollback)
+        {
+          SVN_ERR(svn_rangelist_reverse(child->remaining_ranges, pool));
+          SVN_ERR(svn_rangelist_reverse(parent->remaining_ranges, pool));
+        }
+
       /* If CHILD is the merge target we then know that primary_url,
          REVISION1, and REVISION2 are provided by normalize_merge_sources()
          -- see 'MERGEINFO MERGE SOURCE NORMALIZATION'.  Due to this

Modified: subversion/branches/1.6.x/subversion/libsvn_subr/mergeinfo.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_subr/mergeinfo.c?rev=895677&r1=895676&r2=895677&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_subr/mergeinfo.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_subr/mergeinfo.c Mon Jan  4 16:16:23 2010
@@ -45,7 +45,8 @@
    http://c2.com/cgi-bin/wiki/fullSearch?TestIfDateRangesOverlap
 */
 static svn_boolean_t
-combine_ranges(svn_merge_range_t **output, svn_merge_range_t *in1,
+combine_ranges(svn_merge_range_t *output,
+               svn_merge_range_t *in1,
                svn_merge_range_t *in2,
                svn_boolean_t consider_inheritance)
 {
@@ -55,9 +56,9 @@
           || (consider_inheritance
               && (in1->inheritable == in2->inheritable)))
         {
-          (*output)->start = MIN(in1->start, in2->start);
-          (*output)->end = MAX(in1->end, in2->end);
-          (*output)->inheritable = (in1->inheritable || in2->inheritable);
+          output->start = MIN(in1->start, in2->start);
+          output->end = MAX(in1->end, in2->end);
+          output->inheritable = (in1->inheritable || in2->inheritable);
           return TRUE;
         }
     }
@@ -108,251 +109,324 @@
   return SVN_NO_ERROR;
 }
 
+/* Ways in which two svn_merge_range_t can intersect, if at all. */
+typedef enum
+{
+  /* Ranges don't intersect. */
+  svn__no_intersection,
+
+  /* Ranges are equal. */
+  svn__equal_intersection,
+
+  /* Ranges adjoin but don't overlap. */
+  svn__adjoining_intersection,
+
+  /* Ranges overalp but neither is a subset of the other. */
+  svn__overlapping_intersection,
+
+  /* One range is a proper subset of the other. */
+  svn__proper_subset_intersection
+} intersection_type_t;
+
+/* Given ranges R1 and R2, both of which must be forward merge ranges,
+   set *INTERSECTION_TYPE to describe how the ranges intersect, if they
+   do at all.  The inheritance type of the ranges is not considered. */
+static svn_error_t *
+get_type_of_intersection(svn_merge_range_t *r1,
+                         svn_merge_range_t *r2,
+                         intersection_type_t *intersection_type)
+{
+  SVN_ERR_ASSERT(r1);
+  SVN_ERR_ASSERT(r2);
+  
+  /* Why not use SVN_IS_VALID_REVNUM here?  Because revision 0
+     is described START = -1, END = 0.  See svn_merge_range_t. */
+  SVN_ERR_ASSERT(r1->start >= -1);
+  SVN_ERR_ASSERT(r2->start >= -1);
+  
+  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(r1->end));
+  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(r2->end));
+  SVN_ERR_ASSERT(r1->start < r1->end);
+  SVN_ERR_ASSERT(r2->start < r2->end);
+
+  if (!(r1->start <= r2->end && r2->start <= r1->end))
+    *intersection_type = svn__no_intersection;
+  else if (r1->start == r2->start && r1->end == r2->end)
+    *intersection_type = svn__equal_intersection;
+  else if (r1->end == r2->start || r2->end == r1->start)
+    *intersection_type = svn__adjoining_intersection;
+  else if (r1->start <= r2->start && r1->end >= r2->end)
+    *intersection_type = svn__proper_subset_intersection;
+  else if (r2->start <= r1->start && r2->end >= r1->end)
+    *intersection_type = svn__proper_subset_intersection;
+  else
+    *intersection_type = svn__overlapping_intersection;
+
+  return SVN_NO_ERROR;
+}
+
 /* Helper for svn_rangelist_merge() and rangelist_intersect_or_remove().
 
    If *LASTRANGE is not NULL it should point to the last element in REVLIST.
    REVLIST must be sorted from lowest to highest revision and contain no
-   overlapping revision ranges.  Any changes made to REVLIST will maintain
-   this guarantee.
-
-   If *LASTRANGE is NULL then push MRANGE to REVLIST.
+   overlapping revision ranges.  All ranges in REVLIST must describe forward
+   merges. Any changes made to REVLIST will maintain these guarantees.
 
-   If *LASTRANGE and MRANGE don't intersect then push MRANGE to REVLIST.
-   If they do intersect and have the same inheritability then combine the
-   ranges, updating *LASTRANGE to reflect the new combined range.  If the
-   ranges intersect but differ in inheritability, then merge the ranges - see
-   the doc string for svn_mergeinfo_merge.  This may result in a change to
-   *LASTRANGE's end field and the pushing of up to two new ranges on REVLIST.
-
-     e.g.  *LASTRANGE: '4-10*' merged with MRANGE: '6'________
-                  |                           |               |
-             Update end field               Push       Account for trimmed
-                  |                           |        range from *LASTRANGE.
-                  |                           |        Push it last to
-                  |                           |        maintain sort order.
-                  |                           |               |
-                  V                           V               V
-           *LASTRANGE: '4-5*'              MRANGE: '6'   NEWRANGE: '6-10*'
-
-   Upon return, if any new ranges were pushed onto REVLIST, then set
-   *LASTRANGE to the last range pushed.
+   Make a copy of NEW_RANGE allocated in RESULT_POOL.  In some cases
+   *LASTRANGE may be popped from REVLIST, a copy made (allocated in
+   RESULT_POOL), the copy modified and then pushed back onto REVLIST.
+
+   If *LASTRANGE is NULL then push the copy of NEW_RANGE onto REVLIST.
+
+   If *LASTRANGE and NEW_RANGE don't intersect then push the copy of
+   NEW_RANGE onto REVLIST.
+
+   If the ranges do intersect and have the same inheritability then combine
+   the ranges.
+   
+   If the ranges intersect but differ in inheritability, then merge the
+   ranges as dictated below by CONSIDER_INHERITANCE.
 
    CONSIDER_INHERITANCE determines how to account for the inheritability of
-   MRANGE and *LASTRANGE when determining if they intersect.  If
-   CONSIDER_INHERITANCE is TRUE, then only ranges with the same
-   inheritability can intersect and therefore be combined.
+   NEW_RANGE and *LASTRANGE when determining if they intersect.
+   
+   If CONSIDER_INHERITANCE is false then any intersection between *LASTRANGE
+   and NEW_RANGE is determined strictly on the ranges start and end revisions.
+   If the ranges intersect then they are joined.  The inheritability of the
+   resulting range is non-inheritable *only* if both ranges were
+   non-inheritable, otherwise the combined range is inheritable, e.g.:
+     
+     *LASTRANGE        NEW_RANGE        RESULTING RANGES
+     ----------        ---------        ----------------
+     4-10*             6-13             4-13
+     4-10              6-13*            4-13
+     4-10*             6-13*            4-13*
+
+   If CONSIDER_INHERITANCE is true, then only the intersection between two
+   ranges with differing inheritance can be combined.  If one range has
+   non-inheritable ranges unique to it and the other range is inheritable,
+   then the unique non-inheritable ranges are pushed onto REVLIST as separate
+   ranges.  Adjoining ranges of the same inheritance are joined to make a
+   single range, e.g.:
+
+     *LASTRANGE        NEW_RANGE        RESULTING RANGES
+     ----------        ---------        ----------------
+     4-10*             6                4-5*, 6, 7-10*
+     4-10              6*               4-10
+     4-10*             6-12             4-5*, 6-12
 
-   If DUP_MRANGE is TRUE then allocate a copy of MRANGE before pushing it
-   onto REVLIST.
+   SCRATCH_POOL is used for any temporary allocations.  RESULT_POOL is used
+   to allocate any svn_merge_range_t added to REVLIST.
 */
-static APR_INLINE void
+static svn_error_t *
 combine_with_lastrange(svn_merge_range_t** lastrange,
-                       svn_merge_range_t *mrange, svn_boolean_t dup_mrange,
+                       svn_merge_range_t *new_range,
                        apr_array_header_t *revlist,
                        svn_boolean_t consider_inheritance,
-                       apr_pool_t *pool)
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool)
 {
-  svn_merge_range_t *pushed_mrange_1 = NULL;
-  svn_merge_range_t *pushed_mrange_2 = NULL;
-  svn_boolean_t ranges_intersect = FALSE;
-  svn_boolean_t ranges_have_same_inheritance = FALSE;
+  svn_merge_range_t combined_range;
 
+  /* We don't accept a NULL REVLIST. */
+  SVN_ERR_ASSERT(revlist);
+
+  /* Our contract requires that *LASTRANGE is the "last" range
+     if it isn't NULL. */
   if (*lastrange)
-    {
-      if ((*lastrange)->start <= mrange->end
-          && mrange->start <= (*lastrange)->end)
-        ranges_intersect = TRUE;
-      if ((*lastrange)->inheritable == mrange->inheritable)
-        ranges_have_same_inheritance = TRUE;
-    }
-
-  if (!(*lastrange)
-      || (!ranges_intersect || (!ranges_have_same_inheritance
-                                && consider_inheritance)))
-
-    {
-      /* No *LASTRANGE
-           or
-         LASTRANGE and MRANGE don't intersect
-           or
-         LASTRANGE and MRANGE "intersect" but have different
-         inheritability and we are considering inheritance so
-         can't combined them...
-
-         ...In all these cases just push MRANGE onto *LASTRANGE. */
-      if (dup_mrange)
-        pushed_mrange_1 = svn_merge_range_dup(mrange, pool);
+    SVN_ERR_ASSERT(*lastrange == APR_ARRAY_IDX(revlist,
+                                               revlist->nelts - 1,
+                                               svn_merge_range_t *));
+
+  if (!*lastrange)
+    {
+      /* No *LASTRANGE so push NEW_RANGE onto REVLIST and we are done. */
+      APR_ARRAY_PUSH(revlist, svn_merge_range_t *) =
+        svn_merge_range_dup(new_range, result_pool);
+    }
+  else if (!consider_inheritance)
+    {
+      /* We are not considering inheritance so we can merge intersecting
+         ranges of different inheritability.  Of course if the ranges
+         don't intersect at all we simply push NEW_RANGE only REVLIST. */
+      if (combine_ranges(&combined_range, *lastrange, new_range, FALSE))
+        {
+          (*lastrange)->start = combined_range.start;
+          (*lastrange)->end = combined_range.end;
+          (*lastrange)->inheritable = combined_range.inheritable;
+        }
       else
-        pushed_mrange_1 = mrange;
+        {
+          APR_ARRAY_PUSH(revlist, svn_merge_range_t *) =
+            svn_merge_range_dup(new_range, result_pool);
+        }
     }
-  else /* MRANGE and *LASTRANGE intersect */
+  else /* Considering inheritance */
     {
-      if (ranges_have_same_inheritance)
+      if (combine_ranges(&combined_range, *lastrange, new_range, TRUE))
         {
-          /* Intersecting ranges have the same inheritability
-             so just combine them. */
-          (*lastrange)->start = MIN((*lastrange)->start, mrange->start);
-          (*lastrange)->end = MAX((*lastrange)->end, mrange->end);
-          (*lastrange)->inheritable =
-            ((*lastrange)->inheritable || mrange->inheritable);
+          /* Even when considering inheritance two intersection ranges
+             of the same inheritability can simply be combined. */
+          (*lastrange)->start = combined_range.start;
+          (*lastrange)->end = combined_range.end;
+          (*lastrange)->inheritable = combined_range.inheritable;
         }
-      else /* Ranges intersect but have different
-              inheritability so merge the ranges. */
+      else
         {
-          svn_revnum_t tmp_revnum;
-
-          /* Ranges have same starting revision. */
-          if ((*lastrange)->start == mrange->start)
-            {
-              if ((*lastrange)->end == mrange->end)
-                {
-                  (*lastrange)->inheritable = TRUE;
-                }
-              else if ((*lastrange)->end > mrange->end)
-                {
-                  if (!(*lastrange)->inheritable)
-                    {
-                      tmp_revnum = (*lastrange)->end;
-                      (*lastrange)->end = mrange->end;
-                      (*lastrange)->inheritable = TRUE;
-                      if (dup_mrange)
-                        pushed_mrange_1 = svn_merge_range_dup(mrange, pool);
-                      else
-                        pushed_mrange_1 = mrange;
-                      pushed_mrange_1->start = pushed_mrange_1->start;
-                      pushed_mrange_1->end = tmp_revnum;
-                      *lastrange = pushed_mrange_1;
-                    }
-                }
-              else /* (*lastrange)->end < mrange->end) */
-                {
-                  if (mrange->inheritable)
-                    {
-                      (*lastrange)->inheritable = TRUE;
-                      (*lastrange)->end = mrange->end;
-                    }
-                  else
-                    {
-                      if (dup_mrange)
-                        pushed_mrange_1 = svn_merge_range_dup(mrange, pool);
-                      else
-                        pushed_mrange_1 = mrange;
-                      pushed_mrange_1->start = (*lastrange)->end;
-                    }
-                }
-            }
-          /* Ranges have same ending revision. (Same starting
-             and ending revisions already handled above.) */
-          else if ((*lastrange)->end == mrange->end)
-            {
-              if ((*lastrange)->start < mrange->start)
+          /* If we are here then the ranges either don't intersect or do
+             intersect but have differing inheritability.  Check for the
+             first case as that is easy to handle. */
+          intersection_type_t intersection_type;
+          
+          SVN_ERR(get_type_of_intersection(new_range, *lastrange,
+                                           &intersection_type));
+              
+              switch (intersection_type)
                 {
-                  if (!(*lastrange)->inheritable)
+                  case svn__no_intersection:
+                    /* NEW_RANGE and *LASTRANGE *really* don't intersect so
+                       just push NEW_RANGE only REVLIST. */
+                    APR_ARRAY_PUSH(revlist, svn_merge_range_t *) =
+                      svn_merge_range_dup(new_range, result_pool);
+                    break;
+
+                  case svn__equal_intersection:
+                    /* They range are equal so all we do is force the
+                       inheritability of lastrange to true. */
+                    (*lastrange)->inheritable = TRUE;
+                    break;
+
+                  case svn__adjoining_intersection:
+                    /* They adjoin but don't overlap so just push NEW_RANGE
+                       onto REVLIST. */
+                    APR_ARRAY_PUSH(revlist, svn_merge_range_t *) =
+                      svn_merge_range_dup(new_range, result_pool);
+                    break;
+
+                  case svn__overlapping_intersection:
+                    /* They ranges overlap but neither is a proper subset of
+                       the other.  We'll end up pusing two new ranges onto
+                       REVLIST, the intersecting part and the part unique to
+                       NEW_RANGE.*/
                     {
-                      (*lastrange)->end = mrange->start;
-                      if (dup_mrange)
-                        pushed_mrange_1 = svn_merge_range_dup(mrange, pool);
+                      svn_merge_range_t *r1 = svn_merge_range_dup(*lastrange,
+                                                                  result_pool);
+                      svn_merge_range_t *r2 = svn_merge_range_dup(new_range,
+                                                                  result_pool);
+
+                      /* Pop off *LASTRANGE to make our manipulations
+                         easier. */
+                      apr_array_pop(revlist);
+
+                      /* Ensure R1 is the older range. */
+                      if (r2->start < r1->start)
+                        {
+                          /* Swap R1 and R2. */
+                          r2->start = r1->start;
+                          r2->end = r1->end;
+                          r2->inheritable = r1->inheritable;
+                          r1->start = new_range->start;
+                          r1->end = new_range->end;
+                          r1->inheritable = new_range->inheritable;
+                        }
+
+                      /* Absorb the intersecting ranges into the
+                         inheritable range. */
+                      if (r1->inheritable)
+                        r2->start = r1->end;
                       else
-                        pushed_mrange_1 = mrange;
-                      *lastrange = pushed_mrange_1;
+                        r1->end = r2->start;
+                      
+                      /* Push everything back onto REVLIST. */
+                      APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = r1;
+                      APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = r2;
+
+                      break;
                     }
-                }
-              else /* (*lastrange)->start > mrange->start */
-                {
-                  (*lastrange)->start = mrange->start;
-                  (*lastrange)->end = mrange->end;
-                  (*lastrange)->inheritable = mrange->inheritable;
-                  if (dup_mrange)
-                    pushed_mrange_1 = svn_merge_range_dup(mrange, pool);
-                  else
-                    pushed_mrange_1 = mrange;
-                  pushed_mrange_1->start = (*lastrange)->end;
-                  pushed_mrange_1->inheritable = TRUE;
 
-                }
-            }
-          else /* Ranges have different starting and ending revisions. */
-            {
-              if ((*lastrange)->start < mrange->start)
-                {
-                  /* If MRANGE is a proper subset of *LASTRANGE and
-                     *LASTRANGE is inheritable there is nothing more
-                     to do. */
-                  if (!((*lastrange)->end > mrange->end
-                        && (*lastrange)->inheritable))
+                  default: /* svn__proper_subset_intersection */
                     {
-                      tmp_revnum = (*lastrange)->end;
-                      if (!(*lastrange)->inheritable)
-                        (*lastrange)->end = mrange->start;
-                      else
-                        mrange->start = (*lastrange)->end;
-                      if (dup_mrange)
-                        pushed_mrange_1 = svn_merge_range_dup(mrange, pool);
-                      else
-                        pushed_mrange_1 = mrange;
+                      /* One range is a proper subset of the other. */
+                      svn_merge_range_t *r1 = svn_merge_range_dup(*lastrange,
+                                                                  result_pool);
+                      svn_merge_range_t *r2 = svn_merge_range_dup(new_range,
+                                                                  result_pool);
+                      svn_merge_range_t *r3 = NULL;
+                      svn_revnum_t tmp_revnum;
+
+                      /* Pop off *LASTRANGE to make our manipulations
+                         easier. */
+                      apr_array_pop(revlist);
 
-                      if (tmp_revnum > mrange->end)
+                      /* Ensure R1 is the superset. */
+                      if (r2->start < r1->start || r2->end > r1->end)
                         {
-                          pushed_mrange_2 =
-                            apr_palloc(pool, sizeof(*pushed_mrange_2));
-                          pushed_mrange_2->start = mrange->end;
-                          pushed_mrange_2->end = tmp_revnum;
-                          pushed_mrange_2->inheritable =
-                            (*lastrange)->inheritable;
+                          /* Swap R1 and R2. */
+                          r2->start = r1->start;
+                          r2->end = r1->end;
+                          r2->inheritable = r1->inheritable;
+                          r1->start = new_range->start;
+                          r1->end = new_range->end;
+                          r1->inheritable = new_range->inheritable;
                         }
-                      mrange->inheritable = TRUE;
-                    }
-                }
-              else /* ((*lastrange)->start > mrange->start) */
-                {
-                  pushed_mrange_2 =
-                    apr_palloc(pool, sizeof(*pushed_mrange_2));
 
-                  if ((*lastrange)->end < mrange->end)
-                    {
-                      pushed_mrange_2->start = (*lastrange)->end;
-                      pushed_mrange_2->end = mrange->end;
-                      pushed_mrange_2->inheritable = mrange->inheritable;
-
-                      tmp_revnum = (*lastrange)->start;
-                      (*lastrange)->start = mrange->start;
-                      (*lastrange)->end = tmp_revnum;
-                      (*lastrange)->inheritable = mrange->inheritable;
-
-                      mrange->start = tmp_revnum;
-                      mrange->end = pushed_mrange_2->start;
-                      mrange->inheritable = TRUE;
-                    }
-                  else /* (*lastrange)->end > mrange->end */
-                    {
-                      pushed_mrange_2->start = mrange->end;
-                      pushed_mrange_2->end = (*lastrange)->end;
-                      pushed_mrange_2->inheritable =
-                        (*lastrange)->inheritable;
-
-                      tmp_revnum = (*lastrange)->start;
-                      (*lastrange)->start = mrange->start;
-                      (*lastrange)->end = tmp_revnum;
-                      (*lastrange)->inheritable = mrange->inheritable;
-
-                      mrange->start = tmp_revnum;
-                      mrange->end = pushed_mrange_2->start;
-                      mrange->inheritable = TRUE;
+                      if (r1->inheritable)
+                        {
+                          /* The simple case: The superset is inheritable, so
+                             just combine r1 and r2. */
+                          r1->start = MIN(r1->start, r2->start);
+                          r1->end = MAX(r1->end, r2->end);
+                          r2 = NULL;
+                        }
+                      else if (r1->start == r2->start)
+                        {
+                          /* *LASTRANGE and NEW_RANGE share an end point. */
+                          tmp_revnum = r1->end;
+                          r1->end = r2->end;
+                          r2->inheritable = r1->inheritable;
+                          r1->inheritable = TRUE;
+                          r2->start = r1->end;
+                          r2->end = tmp_revnum;
+                        }
+                      else if (r1->end == r2->end)
+                        {
+                          /* *LASTRANGE and NEW_RANGE share an end point. */
+                          r1->end = r2->start;
+                          r2->inheritable = TRUE;
+                        }
+                      else
+                        {
+                          /* NEW_RANGE and *LASTRANGE share neither start
+                             nor end points. */
+                          r3 = apr_pcalloc(result_pool, sizeof(*r3));
+                          r3->start = r2->end;
+                          r3->end = r1->end;
+                          r3->inheritable = r1->inheritable;
+                          r2->inheritable = TRUE;
+                          r1->end = r2->start;
+                        }
+
+                      /* Push everything back onto REVLIST. */
+                      APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = r1;
+                      if (r2)
+                        APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = r2;
+                      if (r3)
+                        APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = r3;
+
+                      break;
                     }
                 }
-            }
+
+              /* Some of the above cases might have put *REVLIST out of
+                 order, so re-sort.*/
+              qsort(revlist->elts, revlist->nelts, revlist->elt_size,
+                    svn_sort_compare_ranges);
         }
     }
-  if (pushed_mrange_1)
-    {
-      APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = pushed_mrange_1;
-      *lastrange = pushed_mrange_1;
-    }
-  if (pushed_mrange_2)
-    {
-      APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = pushed_mrange_2;
-      *lastrange = pushed_mrange_2;
-    }
+
+  /* Make sure *LASTRANGE points at the "last" range. */
+ *lastrange = APR_ARRAY_IDX(revlist, revlist->nelts - 1, svn_merge_range_t *);
+  return SVN_NO_ERROR;
 }
 
 /* Convert a single svn_merge_range_t * back into an svn_string_t *.  */
@@ -646,21 +720,21 @@
              result. */
           if (elt1->inheritable || elt2->inheritable)
             elt1->inheritable = TRUE;
-          combine_with_lastrange(&lastrange, elt1, TRUE, output,
-                                 FALSE, pool);
+          SVN_ERR(combine_with_lastrange(&lastrange, elt1, output,
+                                         TRUE, pool, pool));
           i++;
           j++;
         }
       else if (res < 0)
         {
-          combine_with_lastrange(&lastrange, elt1, TRUE, output,
-                                 FALSE, pool);
+          SVN_ERR(combine_with_lastrange(&lastrange, elt1, output,
+                                         TRUE, pool, pool));
           i++;
         }
       else
         {
-          combine_with_lastrange(&lastrange, elt2, TRUE, output,
-                                 FALSE, pool);
+          SVN_ERR(combine_with_lastrange(&lastrange, elt2, output,
+                                         TRUE, pool, pool));
           j++;
         }
     }
@@ -673,16 +747,16 @@
     {
       svn_merge_range_t *elt = APR_ARRAY_IDX(*rangelist, i,
                                              svn_merge_range_t *);
-      combine_with_lastrange(&lastrange, elt, TRUE, output,
-                             FALSE, pool);
+      SVN_ERR(combine_with_lastrange(&lastrange, elt, output,
+                                     TRUE, pool, pool));
     }
 
 
   for (; j < changes->nelts; j++)
     {
       svn_merge_range_t *elt = APR_ARRAY_IDX(changes, j, svn_merge_range_t *);
-      combine_with_lastrange(&lastrange, elt, TRUE, output,
-                             FALSE, pool);
+      SVN_ERR(combine_with_lastrange(&lastrange, elt, output,
+                                     TRUE, pool, pool));
     }
 
   *rangelist = output;
@@ -794,8 +868,9 @@
       if (range_contains(elt2, elt1, consider_inheritance))
         {
           if (!do_remove)
-              combine_with_lastrange(&lastrange, elt1, TRUE, *output,
-                                     consider_inheritance, pool);
+            SVN_ERR(combine_with_lastrange(&lastrange, elt1, *output,
+                                           consider_inheritance, pool,
+                                           pool));
 
           i++;
 
@@ -823,8 +898,9 @@
                   tmp_range.end = MIN(elt1->end, elt2->end);
                 }
 
-              combine_with_lastrange(&lastrange, &tmp_range, TRUE,
-                                     *output, consider_inheritance, pool);
+              SVN_ERR(combine_with_lastrange(&lastrange, &tmp_range,
+                                             *output, consider_inheritance,
+                                             pool, pool));
             }
 
           /* Set up the rest of the whiteboard range for further
@@ -839,8 +915,10 @@
                   tmp_range.start = MAX(elt1->start, elt2->start);
                   tmp_range.end = elt2->end;
                   tmp_range.inheritable = elt1->inheritable;
-                  combine_with_lastrange(&lastrange, &tmp_range, TRUE,
-                                         *output, consider_inheritance, pool);
+                  SVN_ERR(combine_with_lastrange(&lastrange, &tmp_range,
+                                                 *output,
+                                                 consider_inheritance,
+                                                 pool, pool));
                 }
 
               wboardelt.start = elt2->end;
@@ -862,7 +940,7 @@
           else
             {
               if (do_remove && !(lastrange &&
-                                 combine_ranges(&lastrange, lastrange, elt1,
+                                 combine_ranges(lastrange, lastrange, elt1,
                                                 consider_inheritance)))
                 {
                   lastrange = svn_merge_range_dup(elt1, pool);
@@ -883,8 +961,8 @@
          the whiteboard element. */
       if (i == lasti && i < whiteboard->nelts)
         {
-          combine_with_lastrange(&lastrange, &wboardelt, TRUE, *output,
-                                 consider_inheritance, pool);
+          SVN_ERR(combine_with_lastrange(&lastrange, &wboardelt, *output,
+                                         consider_inheritance, pool, pool));
           i++;
         }
 
@@ -894,8 +972,8 @@
           svn_merge_range_t *elt = APR_ARRAY_IDX(whiteboard, i,
                                                  svn_merge_range_t *);
 
-          combine_with_lastrange(&lastrange, elt, TRUE, *output,
-                                 consider_inheritance, pool);
+          SVN_ERR(combine_with_lastrange(&lastrange, elt, *output,
+                                         consider_inheritance, pool, pool));
         }
     }
 

Modified: subversion/branches/1.6.x/subversion/tests/libsvn_subr/mergeinfo-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/tests/libsvn_subr/mergeinfo-test.c?rev=895677&r1=895676&r2=895677&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/tests/libsvn_subr/mergeinfo-test.c (original)
+++ subversion/branches/1.6.x/subversion/tests/libsvn_subr/mergeinfo-test.c Mon Jan  4 16:16:23
2010
@@ -698,11 +698,33 @@
   for (i = 0; i < NBR_MERGEINFO_MERGES; i++)
     {
       int j;
+      svn_string_t *info2_starting, *info2_ending;
+
       SVN_ERR(svn_mergeinfo_parse(&info1, mergeinfo[i].mergeinfo1, pool));
       SVN_ERR(svn_mergeinfo_parse(&info2, mergeinfo[i].mergeinfo2, pool));
+
+      /* Make a copy of info2.  We will merge it into info1, but info2
+         should remain unchanged.  Store the mergeinfo as a svn_string_t
+         rather than making a copy and using svn_mergeinfo_diff().  Since
+         that API uses some of the underlying code as svn_mergeinfo_merge
+         we might mask potential errors. */
+      SVN_ERR(svn_mergeinfo_to_string(&info2_starting, info2, pool));
+
       SVN_ERR(svn_mergeinfo_merge(info1, info2, pool));
       if (mergeinfo[i].expected_paths != apr_hash_count(info1))
         return fail(pool, "Wrong number of paths in merged mergeinfo");
+
+      /* Check that info2 remained unchanged. */
+      SVN_ERR(svn_mergeinfo_to_string(&info2_ending, info2, pool));
+
+      if (strcmp(info2_ending->data, info2_starting->data))
+        return fail(pool,
+                    apr_psprintf(pool,
+                                 "svn_mergeinfo_merge case %i "
+                                 "modified its CHANGES arg from "
+                                 "%s to %s", i, info2_starting->data,
+                                 info2_ending->data));
+
       for (j = 0; j < mergeinfo[i].expected_paths; j++)
         {
           int k;
@@ -821,6 +843,10 @@
         {
           int expected_nbr_ranges;
           svn_merge_range_t *expected_ranges;
+          svn_string_t *eraser_starting;
+          svn_string_t *eraser_ending;
+          svn_string_t *whiteboard_starting;
+          svn_string_t *whiteboard_ending;
 
           SVN_ERR(svn_mergeinfo_parse(&info1, (test_data[i]).eraser, pool));
           SVN_ERR(svn_mergeinfo_parse(&info2, (test_data[i]).whiteboard, pool));
@@ -847,6 +873,13 @@
               expected_ranges = (test_data[i]).expected_removed_ignore_inheritance;
 
             }
+
+         /* Make a copies of whiteboard and eraser.  They should not be
+            modified by svn_rangelist_remove(). */
+         SVN_ERR(svn_rangelist_to_string(&eraser_starting, eraser, pool));
+         SVN_ERR(svn_rangelist_to_string(&whiteboard_starting, whiteboard,
+                                         pool));
+
           SVN_ERR(svn_rangelist_remove(&output, eraser, whiteboard,
                                        j == 0,
                                        pool));
@@ -865,6 +898,43 @@
               else
                 err = child_err;
             }
+
+          /* Check that eraser and whiteboard were not modified. */
+          SVN_ERR(svn_rangelist_to_string(&eraser_ending, eraser, pool));
+          SVN_ERR(svn_rangelist_to_string(&whiteboard_ending, whiteboard,
+                                          pool));
+          if (strcmp(eraser_starting->data, eraser_ending->data))
+            {
+              child_err = fail(pool,
+                               apr_psprintf(pool,
+                                            "svn_rangelist_remove case %i "
+                                            "modified its ERASER arg from "
+                                            "%s to %s when %sconsidering "
+                                            "inheritance", i,
+                                            eraser_starting->data,
+                                            eraser_ending->data,
+                                            j ? "" : "not "));
+              if (err)
+                svn_error_compose(err, child_err);
+              else
+                err = child_err;
+            }
+          if (strcmp(whiteboard_starting->data, whiteboard_ending->data))
+            {
+              child_err = fail(pool,
+                               apr_psprintf(pool,
+                                            "svn_rangelist_remove case %i "
+                                            "modified its WHITEBOARD arg "
+                                            "from %s to %s when "
+                                            "%sconsidering inheritance", i,
+                                            whiteboard_starting->data,
+                                            whiteboard_ending->data,
+                                            j ? "" : "not "));
+              if (err)
+                svn_error_compose(err, child_err);
+              else
+                err = child_err;
+            }
         }
     }
   return err;
@@ -1265,6 +1335,8 @@
   err = child_err = SVN_NO_ERROR;
   for (i = 0; i < SIZE_OF_RANGE_MERGE_TEST_ARRAY; i++)
     {
+      svn_string_t *rangelist2_starting, *rangelist2_ending;
+
       SVN_ERR(svn_mergeinfo_parse(&info1, (test_data[i]).mergeinfo1, pool));
       SVN_ERR(svn_mergeinfo_parse(&info2, (test_data[i]).mergeinfo2, pool));
       rangelist1 = apr_hash_get(info1, "/A", APR_HASH_KEY_STRING);
@@ -1276,6 +1348,10 @@
       if (rangelist2 == NULL)
         rangelist2 = apr_array_make(pool, 0, sizeof(svn_merge_range_t *));
 
+      /* Make a copy of rangelist2.  We will merge it into rangelist1, but
+         rangelist2 should remain unchanged. */
+      SVN_ERR(svn_rangelist_to_string(&rangelist2_starting, rangelist2,
+                                      pool));
       SVN_ERR(svn_rangelist_merge(&rangelist1, rangelist2, pool));
       child_err = verify_ranges_match(rangelist1,
                                      (test_data[i]).expected_merge,
@@ -1293,6 +1369,23 @@
           else
             err = child_err;
         }
+
+      /* Check that rangelist2 remains unchanged. */
+      SVN_ERR(svn_rangelist_to_string(&rangelist2_ending, rangelist2, pool));
+      if (strcmp(rangelist2_ending->data, rangelist2_starting->data))
+        {
+          child_err = fail(pool,
+                           apr_psprintf(pool,
+                                        "svn_rangelist_merge case %i "
+                                        "modified its CHANGES arg from "
+                                        "%s to %s", i,
+                                        rangelist2_starting->data,
+                                        rangelist2_ending->data));
+          if (err)
+            svn_error_compose(err, child_err);
+          else
+            err = child_err;
+        }
     }
   return err;
 }



Mime
View raw message