[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Assert IS_VALID_FORWARD_RANGE fails in merge_tests 125

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 27 Nov 2012 23:53:48 +0000 (GMT)

Johan Corveleyn wrote:
> On Sun, Nov 11, 2012 at 1:59 PM, Stefan Fuhrmann wrote:
>> On Thu, Nov 8, 2012 at 6:04 PM, Julian Foad wrote:
>>> Stefan Fuhrmann wrote:
>>>> But I did run across an assertion in mergeinfo.c, :line 1174
>>>>> during merge_tests.py 125:
>>>>>>
>>>>>>   SVN_ERR_ASSERT_NO_RETURN(IS_VALID_FORWARD_RANGE(first));

I debugged this assertion failure and came up with this fix:

[[[
Fix issue #4132 "merge of replaced source asserts", fixing merge_tests 125.

* subversion/libsvn_client/merge.c
  (find_gaps_in_merge_source_history): Fix an off-by-1 bug. Assert that the
    function's result constitutes a valid non-empty range.

Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c    (revision 1414469)
+++ subversion/libsvn_client/merge.c    (working copy)
@@ -4310,7 +4310,7 @@ find_gaps_in_merge_source_history(svn_re
   /* Get SOURCE as mergeinfo. */
   SVN_ERR(svn_client__get_history_as_mergeinfo(&implicit_src_mergeinfo, NULL,
                                                primary_src,
-                                               primary_src->rev, old_rev,
+                                               primary_src->rev, old_rev + 1,
                                                ra_session,
                                                ctx, scratch_pool));

@@ -4384,6 +4384,9 @@ find_gaps_in_merge_source_history(svn_re
   SVN_ERR_ASSERT(*gap_start == MIN(source->loc1->rev, source->loc2->rev)
                  || (*gap_start == SVN_INVALID_REVNUM
                      && *gap_end == SVN_INVALID_REVNUM));
+  SVN_ERR_ASSERT(*gap_end > *gap_start
+                 || (*gap_start == SVN_INVALID_REVNUM
+                     && *gap_end == SVN_INVALID_REVNUM));
   return SVN_NO_ERROR;
 }
]]]

Trouble is, this fix makes merge_tests.py 100 fail.

I haven't yet investigated further.

In doing that I studied svn_client__get_history_as_mergeinfo() and what it's built on, and came up with a unit test improvement (r1414414) and the following doc improvements:

[[[
Index: subversion/include/svn_ra.h
===================================================================
 /* [...]
  * @a peg_revision may be @c SVN_INVALID_REVNUM to indicate "the HEAD
  * revision", and must evaluate to be at least as young as @a start_rev.
  *
+ * The sequence of segments passed to @a receiver will describe adjacent,
+ * non-overlapping ranges of revisions.  The highest reported revision
+ * (@c range_end of the first segment) will always be @a start_rev, and the
+ * lowest reported revision (@c range_start of the last segment) will be
+ * @a end_rev or the origin of the object, whichever is higher.
+ *
  * [...] */
svn_error_t *
svn_ra_get_location_segments(svn_ra_session_t *session, [...]);

Index: subversion/include/svn_types.h
===================================================================
 /**
- * A representation of a segment of a object's version history with an
+ * A representation of a segment of an object's version history with an
  * emphasis on the object's location in the repository as of various
  * revisions.
  * [...] */
 typedef struct svn_location_segment_t
 {
   /** The beginning (oldest) and ending (youngest) revisions for this
-      segment. */
+      segment, both inclusive. */
   svn_revnum_t range_start;
   svn_revnum_t range_end;

Index: subversion/libsvn_client/mergeinfo.h
===================================================================
 /* Set *MERGEINFO_P to a mergeinfo constructed solely from the
    natural history of PATHREV.

-   If RANGE_YOUNGEST and RANGE_OLDEST are valid, use them to bound the
-   revision ranges of returned mergeinfo.  They are governed by the same
-   rules as the PEG_REVISION, START_REV, and END_REV parameters of
+   If RANGE_YOUNGEST and RANGE_OLDEST are valid, use them as inclusive
+   bounds on the revision ranges of returned mergeinfo.  PATHREV->rev,
+   RANGE_YOUNGEST and RANGE_OLDEST are governed by the same rules as the
+   PEG_REVISION, START_REV, and END_REV parameters (respectively) of
    svn_ra_get_location_segments().

    [...] */
svn_error_t *
svn_client__get_history_as_mergeinfo(svn_mergeinfo_t *mergeinfo_p, [...]);
]]]

- Julian
Received on 2012-11-28 00:54:25 CET

This is an archived mail posted to the Subversion Dev mailing list.