You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2010/01/04 17:16:32 UTC
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
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;
}
Re: 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
Posted by Paul Burba <pt...@gmail.com>.
On Mon, Jan 4, 2010 at 12:34 PM, Julian Foad <ju...@btopenworld.com> wrote:
> Hyrum K. Wright wrote:
>> On Jan 4, 2010, at 11:18 AM, Bert Huijben wrote:
>> >> -----Original Message-----
>> >> From: hwright@apache.org [mailto:hwright@apache.org]
>> >> 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
>> >
>> > What happens here ^^^^
>> >
>> > I think you triggered an old bug here that should be resolved by a previous merge?
>>
>> Dunno. I was using the latest 1.6.x client to do this merge, so it shouldn't be a already-fixed bug.
>
> If it's the "1.6.x-r39109" part that's worrying, that could be genuine
> mergeinfo. A branch named "1.6.x-r39109" did exist for a while; its name
> was a typo and so it was deleted and recreated.
>
> - Julian
>
Julian beat me to the explanation.
And yes, I should have simply deleted the 1.6.x-r39109 branch and
created a new 1.6.x-r39019 branch rather than doing a rename. It's
not as if I had actually backported anything yet. So user error on my
part, but no harm and no bug.
Paul
Re: 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
Posted by Julian Foad <ju...@btopenworld.com>.
Hyrum K. Wright wrote:
> On Jan 4, 2010, at 11:18 AM, Bert Huijben wrote:
> >> -----Original Message-----
> >> From: hwright@apache.org [mailto:hwright@apache.org]
> >> 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
> >
> > What happens here ^^^^
> >
> > I think you triggered an old bug here that should be resolved by a previous merge?
>
> Dunno. I was using the latest 1.6.x client to do this merge, so it shouldn't be a already-fixed bug.
If it's the "1.6.x-r39109" part that's worrying, that could be genuine
mergeinfo. A branch named "1.6.x-r39109" did exist for a while; its name
was a typo and so it was deleted and recreated.
- Julian
Re: 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
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Jan 4, 2010, at 11:18 AM, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: hwright@apache.org [mailto:hwright@apache.org]
>> Sent: maandag 4 januari 2010 17:17
>> To: commits@subversion.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
>>
>> 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
>
> What happens here ^^^^
>
> I think you triggered an old bug here that should be resolved by a previous merge?
Dunno. I was using the latest 1.6.x client to do this merge, so it shouldn't be a already-fixed bug.
-Hyrum
RE: 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
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: hwright@apache.org [mailto:hwright@apache.org]
> Sent: maandag 4 januari 2010 17:17
> To: commits@subversion.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
>
> 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
What happens here ^^^^
I think you triggered an old bug here that should be resolved by a previous merge?
Bert