Author: pburba
Date: Mon Mar 15 19:00:28 2010
New Revision: 923389
URL: http://svn.apache.org/viewvc?rev=923389&view=rev
Log:
Fix a bug where the ordering of arguments to svn_rangelist_intersect can
produce different results.
* subversion/include/svn_mergeinfo.h
(svn_mergeinfo_intersect2):
(svn_rangelist_intersect):
Correct/improve doc strings to reflect long standing behaviors regarding
rangelist intersection and range inheritance.
* subversion/libsvn_subr/mergeinfo.c
(rangelist_intersect_or_remove): Fix bug where the order of arguments
could effect the result when finding an intersection. Address an old
### FIXME by adjusting argument names and comments to apply to either
an intersection or removal (i.e. make them generic).
* subversion/tests/libsvn_subr/mergeinfo-test.c
(test_rangelist_intersect): Test that order of rangelists passed to
svn_rangelist_intersect() don't effect the result. Test that the
intersection of two ranges is non-inheritable only when both ranges
are non-inheritable. Lastly, test that ranges of differing inheritance
do not intersect when considering inheritance.
Modified:
subversion/trunk/subversion/include/svn_mergeinfo.h
subversion/trunk/subversion/libsvn_subr/mergeinfo.c
subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
Modified: subversion/trunk/subversion/include/svn_mergeinfo.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_mergeinfo.h?rev=923389&r1=923388&r2=923389&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_mergeinfo.h (original)
+++ subversion/trunk/subversion/include/svn_mergeinfo.h Mon Mar 15 19:00:28 2010
@@ -311,8 +311,7 @@ svn_mergeinfo_intersect(svn_mergeinfo_t
*
* @a consider_inheritance determines how to account for the inheritability
* of the two mergeinfo's ranges when calculating the range equivalence,
- * as described for svn_mergeinfo_diff(). If @a consider_inheritance is
- * FALSE then @a *mergeinfo's ranges are always inheritable.
+ * @see svn_rangelist_intersect().
*
* @since New in 1.7.
*/
@@ -331,7 +330,11 @@ svn_mergeinfo_intersect2(svn_mergeinfo_t
* @a consider_inheritance determines how to account for the inheritability
* of the two rangelist's ranges when calculating the intersection,
* @see svn_mergeinfo_diff(). If @a consider_inheritance is FALSE then
- * the ranges in @a *rangelist are always inheritable.
+ * ranges with different inheritance can intersect, but the the resulting
+ * @a *rangelist is non-inheritable only if the corresponding ranges from
+ * both @a rangelist1 and @a rangelist2 are non-inheritable.
+ * If @a consider_inheritance is TRUE, then ranges with different
+ * inheritance can never intersect.
*
* Note: @a rangelist1 and @a rangelist2 must be sorted as said by @c
* svn_sort_compare_ranges(). @a *rangelist is guaranteed to be in sorted
Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mergeinfo.c?rev=923389&r1=923388&r2=923389&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Mon Mar 15 19:00:28 2010
@@ -857,15 +857,42 @@ svn_mergeinfo__set_inheritance(svn_merge
return;
}
-/* Either remove any overlapping ranges described by ERASER from
- WHITEBOARD (when DO_REMOVE is TRUE), or capture the overlap, and
- place the remaining or overlapping ranges in OUTPUT. */
-/* ### FIXME: Some variables names and inline comments for this method
- ### are legacy from when it was solely the remove() impl. */
+/* If DO_REMOVE is true, then remove any overlapping ranges described by
+ RANGELIST1 from RANGELIST2 and place the results in *OUTPUT. When
+ DO_REMOVE is true, RANGELIST1 is effectively the "eraser" and RANGELIST2
+ the "whiteboard".
+
+ If DO_REMOVE is false, then capture the intersection between RANGELIST1
+ and RANGELIST2 and place the results in *OUTPUT. The ordering of
+ RANGELIST1 and RANGELIST2 doesn't matter when DO_REMOVE is false.
+
+ If CONSIDER_INHERITANCE is true, then take the inheritance of the
+ ranges in RANGELIST1 and RANGELIST2 into account when comparing them
+ for intersection, see the doc string for svn_rangelist_intersection().
+
+ If CONSIDER_INHERITANCE is true, then ranges with differing inheritance
+ may intersect, but the resulting intersection is non-inheritable only
+ if both ranges were non-inheritable, e.g.:
+
+ RANGELIST1 RANGELIST2 CONSIDER DO_REMOVE *OUTPUT
+ INHERITANCE
+ ---------- ------ ----------- --------- -------
+
+ 90-420* 1-100 TRUE FALSE Empty Rangelist
+ 90-420 1-100* TRUE FALSE Empty Rangelist
+ 90-420 1-100 TRUE FALSE 90-100
+ 90-420* 1-100* TRUE FALSE 90-100*
+
+ 90-420* 1-100 FALSE FALSE 90-100
+ 90-420 1-100* FALSE FALSE 90-100
+ 90-420 1-100 FALSE FALSE 90-100
+ 90-420* 1-100* FALSE FALSE 90-100*
+
+ Allocate the contents of *OUTPUT in POOL. */
static svn_error_t *
rangelist_intersect_or_remove(apr_array_header_t **output,
- const apr_array_header_t *eraser,
- const apr_array_header_t *whiteboard,
+ const apr_array_header_t *rangelist1,
+ const apr_array_header_t *rangelist2,
svn_boolean_t do_remove,
svn_boolean_t consider_inheritance,
apr_pool_t *pool)
@@ -879,37 +906,46 @@ rangelist_intersect_or_remove(apr_array_
j = 0;
lasti = -1; /* Initialized to a value that "i" will never be. */
- while (i < whiteboard->nelts && j < eraser->nelts)
+ while (i < rangelist2->nelts && j < rangelist1->nelts)
{
svn_merge_range_t *elt1, *elt2;
- elt2 = APR_ARRAY_IDX(eraser, j, svn_merge_range_t *);
+ elt2 = APR_ARRAY_IDX(rangelist1, j, svn_merge_range_t *);
- /* Instead of making a copy of the entire array of whiteboard
- elements, we just keep a copy of the current whiteboard element
+ /* Instead of making a copy of the entire array of rangelist2
+ elements, we just keep a copy of the current rangelist2 element
that needs to be used, and modify our copy if necessary. */
if (i != lasti)
{
- wboardelt = *(APR_ARRAY_IDX(whiteboard, i, svn_merge_range_t *));
+ wboardelt = *(APR_ARRAY_IDX(rangelist2, i, svn_merge_range_t *));
lasti = i;
}
elt1 = &wboardelt;
- /* If the whiteboard range is contained completely in the
- eraser, we increment the whiteboard.
+ /* If the rangelist2 range is contained completely in the
+ rangelist1, we increment the rangelist2.
If the ranges intersect, and match exactly, we increment both
- eraser and whiteboard.
+ rangelist1 and rangelist2.
Otherwise, we have to generate a range for the left part of
- the removal of eraser from whiteboard, and possibly change
- the whiteboard to the remaining portion of the right part of
+ the removal of rangelist1 from rangelist2, and possibly change
+ the rangelist2 to the remaining portion of the right part of
the removal, to test against. */
if (range_contains(elt2, elt1, consider_inheritance))
{
if (!do_remove)
- SVN_ERR(combine_with_lastrange(elt1, *output,
- consider_inheritance, pool,
- pool));
+ {
+ svn_merge_range_t tmp_range;
+ tmp_range.start = elt1->start;
+ tmp_range.end = elt1->end;
+ /* The intersection of two ranges is non-inheritable only
+ if both ranges are non-inheritable. */
+ tmp_range.inheritable =
+ (elt1->inheritable || elt2->inheritable);
+ SVN_ERR(combine_with_lastrange(&tmp_range, *output,
+ consider_inheritance, pool,
+ pool));
+ }
i++;
@@ -920,21 +956,26 @@ rangelist_intersect_or_remove(apr_array_
{
if (elt1->start < elt2->start)
{
- /* The whiteboard range starts before the eraser range. */
+ /* The rangelist2 range starts before the rangelist1 range. */
svn_merge_range_t tmp_range;
- tmp_range.inheritable = elt1->inheritable;
if (do_remove)
{
- /* Retain the range that falls before the eraser start. */
+ /* Retain the range that falls before the rangelist1
+ start. */
tmp_range.start = elt1->start;
tmp_range.end = elt2->start;
+ tmp_range.inheritable = elt1->inheritable;
}
else
{
- /* Retain the range that falls between the eraser
- start and whiteboard end. */
+ /* Retain the range that falls between the rangelist1
+ start and rangelist2 end. */
tmp_range.start = elt2->start;
tmp_range.end = MIN(elt1->end, elt2->end);
+ /* The intersection of two ranges is non-inheritable only
+ if both ranges are non-inheritable. */
+ tmp_range.inheritable =
+ (elt1->inheritable || elt2->inheritable);
}
SVN_ERR(combine_with_lastrange(&tmp_range,
@@ -942,18 +983,21 @@ rangelist_intersect_or_remove(apr_array_
pool, pool));
}
- /* Set up the rest of the whiteboard range for further
+ /* Set up the rest of the rangelist2 range for further
processing. */
if (elt1->end > elt2->end)
{
- /* The whiteboard range ends after the eraser range. */
+ /* The rangelist2 range ends after the rangelist1 range. */
if (!do_remove)
{
/* Partial overlap. */
svn_merge_range_t tmp_range;
- tmp_range.start = MAX(elt1->start, elt2->start);
+ tmp_range.start = MAX(elt1->start, elt2->start;)
tmp_range.end = elt2->end;
- tmp_range.inheritable = elt1->inheritable;
+ /* The intersection of two ranges is non-inheritable only
+ if both ranges are non-inheritable. */
+ tmp_range.inheritable =
+ (elt1->inheritable || elt2->inheritable);
SVN_ERR(combine_with_lastrange(&tmp_range,
*output,
consider_inheritance,
@@ -968,12 +1012,12 @@ rangelist_intersect_or_remove(apr_array_
}
else /* ranges don't intersect */
{
- /* See which side of the whiteboard the eraser is on. If it
- is on the left side, we need to move the eraser.
+ /* See which side of the rangelist2 the rangelist1 is on. If it
+ is on the left side, we need to move the rangelist1.
- If it is on past the whiteboard on the right side, we
- need to output the whiteboard and increment the
- whiteboard. */
+ If it is on past the rangelist2 on the right side, we
+ need to output the rangelist2 and increment the
+ rangelist2. */
if (svn_sort_compare_ranges(&elt2, &elt1) < 0)
j++;
else
@@ -1000,23 +1044,23 @@ rangelist_intersect_or_remove(apr_array_
if (do_remove)
{
- /* Copy the current whiteboard element if we didn't hit the end
- of the whiteboard, and we still had it around. This element
- may have been touched, so we can't just walk the whiteboard
+ /* Copy the current rangelist2 element if we didn't hit the end
+ of the rangelist2, and we still had it around. This element
+ may have been touched, so we can't just walk the rangelist2
array, we have to use our copy. This case only happens when
- we ran out of eraser before whiteboard, *and* we had changed
- the whiteboard element. */
- if (i == lasti && i < whiteboard->nelts)
+ we ran out of rangelist1 before rangelist2, *and* we had changed
+ the rangelist2 element. */
+ if (i == lasti && i < rangelist2->nelts)
{
SVN_ERR(combine_with_lastrange(&wboardelt, *output,
consider_inheritance, pool, pool));
i++;
}
- /* Copy any other remaining untouched whiteboard elements. */
- for (; i < whiteboard->nelts; i++)
+ /* Copy any other remaining untouched rangelist2 elements. */
+ for (; i < rangelist2->nelts; i++)
{
- svn_merge_range_t *elt = APR_ARRAY_IDX(whiteboard, i,
+ svn_merge_range_t *elt = APR_ARRAY_IDX(rangelist2, i,
svn_merge_range_t *);
SVN_ERR(combine_with_lastrange(elt, *output,
Modified: subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c?rev=923389&r1=923388&r2=923389&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c Mon Mar 15
19:00:28 2010
@@ -479,21 +479,68 @@ static svn_error_t *
test_rangelist_intersect(apr_pool_t *pool)
{
apr_array_header_t *rangelist1, *rangelist2, *intersection;
- svn_merge_range_t expected_intersection[] =
- { {0, 1, TRUE}, {2, 4, TRUE}, {11, 12, TRUE}, {30, 32, TRUE},
+
+ /* Expected intersection when considering inheritance. */
+ svn_merge_range_t intersection_consider_inheritance[] =
+ { {0, 1, TRUE}, {11, 12, TRUE}, {30, 32, FALSE}, {39, 42, TRUE} };
+
+ /* Expected intersection when ignoring inheritance. */
+ svn_merge_range_t intersection_ignore_inheritance[] =
+ { {0, 1, TRUE}, {2, 4, TRUE}, {11, 12, TRUE}, {30, 32, FALSE},
{39, 42, TRUE} };
- SVN_ERR(svn_mergeinfo_parse(&info1, "/trunk: 1-6,12-16,30-32,40-42", pool));
- SVN_ERR(svn_mergeinfo_parse(&info2, "/trunk: 1,3-4,7,9,11-12,31-34,38-44",
+ SVN_ERR(svn_mergeinfo_parse(&info1, "/trunk: 1-6,12-16,30-32*,40-42", pool));
+ SVN_ERR(svn_mergeinfo_parse(&info2, "/trunk: 1,3-4*,7,9,11-12,31-34*,38-44",
pool));
rangelist1 = apr_hash_get(info1, "/trunk", APR_HASH_KEY_STRING);
rangelist2 = apr_hash_get(info2, "/trunk", APR_HASH_KEY_STRING);
+ /* Check the intersection while considering inheritance twice, reversing
+ the order of the rangelist arguments on the second call to
+ svn_rangelist_intersection. The order *should* have no effect on
+ the result -- see http://svn.haxx.se/dev/archive-2010-03/0351.shtml.
+
+ '3-4*' has different inheritance than '1-6', so no intersection is
+ expected. '30-32*' and '31-34*' have the same inheritance, so intersect
+ at '31-32*'. Per the svn_rangelist_intersect API, since both ranges
+ are non-inheritable, so is the result. */
SVN_ERR(svn_rangelist_intersect(&intersection, rangelist1, rangelist2,
TRUE, pool));
- return verify_ranges_match(intersection, expected_intersection, 5,
- "svn_rangelist_intersect", "intersect", pool);
+ SVN_ERR(verify_ranges_match(intersection,
+ intersection_consider_inheritance,
+ 4, "svn_rangelist_intersect", "intersect",
+ pool));
+
+ SVN_ERR(svn_rangelist_intersect(&intersection, rangelist2, rangelist1,
+ TRUE, pool));
+
+ SVN_ERR(verify_ranges_match(intersection,
+ intersection_consider_inheritance,
+ 4, "svn_rangelist_intersect", "intersect",
+ pool));
+
+ /* Check the intersection while ignoring inheritance. The one difference
+ from when we consider inheritance is that '3-4*' and '1-6' now intersect,
+ since we don't care about inheritability, just the start and end ranges.
+ Per the svn_rangelist_intersect API, since only one range is
+ non-inheritable the result is inheritable. */
+ SVN_ERR(svn_rangelist_intersect(&intersection, rangelist1, rangelist2,
+ FALSE, pool));
+
+ SVN_ERR(verify_ranges_match(intersection,
+ intersection_ignore_inheritance,
+ 5, "svn_rangelist_intersect", "intersect",
+ pool));
+
+ SVN_ERR(svn_rangelist_intersect(&intersection, rangelist2, rangelist1,
+ FALSE, pool));
+
+ SVN_ERR(verify_ranges_match(intersection,
+ intersection_ignore_inheritance,
+ 5, "svn_rangelist_intersect", "intersect",
+ pool));
+ return SVN_NO_ERROR;
}
static svn_error_t *
