On Mon, 2011-07-11 at 12:23 +0100, Julian Foad wrote:
> On Sat, 2011-07-09, Barry Scott wrote:
> > svn built with ./configure --prefix=/usr/local --enable-debug on
> > Fedora 15 x86_64.
>
> Hi Barry. This stack trace and info is useful, ...
>
> > Let me know if you want to run this yourself. I can provide a tarball
> > with the pysvn sources that trigger this and easy instructions to
> > reproduce.
>
> ... but Yes Please! That would be fantastic. If it's smaller than 100
> KB just attach it here on the list; if bigger, please email to me
> off-list (and I suspect Paul Burba would be interested too).
>
> In the meantime, this is what I can see so far ...
>
>
> > I'm testing 1.7-alpha2 against pysvn and I'm seeing an assert
> > when calling svn_client_merge3. This has all works against 1.6.
> >
> > Is this a bug in svn or pysvn calling svn?
> >
> >
> >
> >
> > Here is what was printed:
> >
> > Info: pysvn command: merge --revision 16:17
> > file:///home/barry/wc/svn/pysvn/Extension/Tests/testroot-01/root/repos/trunk/test
> > /home/barry/wc/svn/pysvn/Extension/Tests/testroot-01/wc3/test-branch
> > subversion/libsvn_subr/mergeinfo.c:801: (apr_err=235000)
> > svn: E235000: In file 'subversion/libsvn_subr/mergeinfo.c' line 801:
> > assertion failed (IS_VALID_FORWARD_RANGE(first))
> > ./test-01-qqq.sh: line 16: 9958 Aborted (core dumped)
> > ${PYSVN} "$@"
> >
> >
> > And gdb output of the stack, the call site and the variables passed
> > into svn_client_merge3.
>
> (I've hand-edited the stack trace quoted here for readability, stripping
> out some memory addresses and other less interesting things.)
>
> > #3 svn_error__malfunction ... "IS_VALID_FORWARD_RANGE(first)"
>
> #define IS_VALID_FORWARD_RANGE(range) \
> (SVN_IS_VALID_REVNUM((range)->start) && ((range)->start < (range)->end))
>
> > #4 range_contains (first=0x19c32b8, ...)
> > at subversion/libsvn_subr/mergeinfo.c:801
>
> This is failing to validate an element of the 'rangelist1' input ...
>
> > #5 rangelist_intersect_or_remove (..., rangelist1=0x19c3290, ...)
> > at subversion/libsvn_subr/mergeinfo.c:959
> > #6 svn_rangelist_intersect (..., rangelist1=0x19c3290, ...)
> > at subversion/libsvn_subr/mergeinfo.c:1107
> > #7 adjust_deleted_subtree_ranges (child=0x19bf4e0, parent=0x194c1d8,
> > mergeinfo_path="/trunk/test/file-merge-1.txt", revision1=16,
> > revision2=17,
> > primary_url="file:///.../trunk/test/file-merge-1.txt", ...)
> > at subversion/libsvn_client/merge.c:2908
>
> ... which is a single-element rangelist created here by:
>
> /* Create a rangelist describing the range PRIMARY_URL@older_rev
> exists and find the intersection of that and
> CHILD->REMAINING_RANGES. */
> svn_rangelist__initialize(older_rev,
> revision_primary_url_deleted - 1,
> TRUE, scratch_pool);
>
> This makes range->start = older_rev which would be 16 (AFAICT from
> reading the code), and range->end = (rev...deleted - 1).
> 'rev...deleted' comes from:
>
> /* PRIMARY_URL@older_rev exists, so it was deleted at some
> revision prior to peg_rev, find that revision. */
> SVN_ERR(svn_ra_get_deleted_rev(ra_session,
> rel_source_path [=.../file-merge-1.txt],
> older_rev [=16], younger_rev [=17],
> &revision_primary_url_deleted,
> scratch_pool));
>
> and this whole code path is entered because file-merge-1.txt doesn't
> exist in r17, so it looks like that "-1" is the problem. Presumably
> 'rev...deleted' is being set to 17 and so an invalid range of (start=16,
> end=16) is being created. The 'end' of a rangelist is supposed be be
> exclusive not inclusive.
Correction: if it's a forward range, then it's the 'start' of a
rangelist that is supposed to be exclusive; the 'end' is inclusive.
In this case what's happening is simply that we're creating an empty
range (where start == end), but an empty range is considered invalid.
The simplest fix is probably just to set the intersection result to the
empty set, so I would expect the following patch to fix the problem.
[[[
@@ @@ adjust_deleted_subtree_ranges(svn_client...
/* Create a rangelist describing the range PRIMARY_URL@older_rev
exists and find the intersection of that and
CHILD->REMAINING_RANGES. */
+ if (revision_primary_url_deleted - 1 > older_rev)
+ {
+ /* It was not deleted immediately after OLDER_REV, so
it has some relevant changes. */
exists_rangelist =
svn_rangelist__initialize(older_rev,
revision_primary_url_deleted - 1,
TRUE, scratch_pool);
SVN_ERR(svn_rangelist_intersect(&(child->remaining_ranges),
exists_rangelist,
child->remaining_ranges,
FALSE, scratch_pool));
+ }
+ else
+ {
+ /* It was deleted immediately after the OLDER rev, so
+ it has no relevant changes. */
+ child->remaining_ranges = apr_array_make(scratch_pool, 0,
+ sizeof(svn_merge_range_t *));
+ }
]]]
> I'll have a go now at reproducing this problem without your test data,
> but it would be good to confirm it with your test data.
There are three existing tests that exercise this code path, which I
found by inserting a debug print statement here and running the test
suite:
merge_authz_tests.py 2
relpath='D/gamma', older_rev=1, younger_rev=6, rev...deleted=5
merge_tests.py 83
relpath='D/H/psi', older_rev=1, younger_rev=12, rev...deleted=7
merge_tests.py 94
relpath='psi', older_rev=1, younger_rev=9, rev...deleted=8
I'm still trying to create a new test specifically for this problem.
- Julian
