Re: Assert IS_VALID_FORWARD_RANGE fails in merge_tests 125

Paul Burba wrote: > Julian Foad wrote: >>>>>>>>   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. > merge_tests.py 100 demonstrates the problem with what you propose [...] [...] [...] > Anyway, I hope that helps explain what is happening here. Thanks for the detailed walk-through, Paul. What I have found so far (I spent a bit more time on it today) is that the error is *much* earlier than here, right at the start of the merge: normalize_merge_sources_internal() comes up with the result (2,9). Adding some debugging as in the attached 'merge-t-125-1.patch': $ svn merge -r9:2 .../A A_COPY --dry-run ... DBG: merge.c:6566: oldest 2, youngest 9 DBG: merge.c:6592: Location segments: DBG: merge.c:6597:   0: 2-2 'A' DBG: merge.c:6597:   1: 3-6 '(null)' DBG: merge.c:6597:   2: 7-9 'A' DBG: merge.c:6701: Resulting merge_source_ts: DBG: merge.c:6706:   0: 9 '.../repositories/merge_tests-100/A' DBG: merge.c:6707:      2 '.../repositories/merge_tests-100/A' So that's where I'm at. - Julian