You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Jens Restemeier <je...@playtonicgames.com> on 2017/07/02 22:44:00 UTC

Re: "Unable to parse reversed revision range" when merging from trunk to branch

The problem seems to come from svn_rangelist_merge2.

This test program recreates the problem. 

#include <svn_pools.h>
#include <svn_mergeinfo.h>

int main(int argc, char * argv[])
{
    apr_pool_t *pool;
    int exit_code = EXIT_SUCCESS;
    svn_error_t *err;
    
    if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
        return EXIT_FAILURE;
    
    pool = apr_allocator_owner_get(svn_pool_create_allocator(FALSE));
    
    // 15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,19635-20055*
    svn_rangelist_t * rangelist = apr_array_make(pool, 1, sizeof(svn_merge_range_t *));
    svn_merge_range_t *mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 15013;
    mrange->end = 19472;
    mrange->inheritable = TRUE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19472;
    mrange->end = 19612;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19612;
    mrange->end = 19614;
    mrange->inheritable = TRUE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19614;
    mrange->end = 19630;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19630;
    mrange->end = 19634;
    mrange->inheritable = TRUE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19634;
    mrange->end = 20055;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    // 15014-20515*
    svn_rangelist_t * changes = apr_array_make(pool, 1, sizeof(svn_merge_range_t *));
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 15013;
    mrange->end = 20515;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(changes, svn_merge_range_t *) = mrange;
    
    {
        svn_string_t * tmpString;
        svn_rangelist_to_string(&tmpString, rangelist, pool);
        printf("rangelist %s\n", tmpString->data);
    }
    {
        svn_string_t * tmpString;
        svn_rangelist_to_string(&tmpString, changes, pool);
        printf("changes %s\n", tmpString->data);
    }
    
    svn_rangelist_merge2(rangelist, changes, pool, pool);
    
    {
        svn_string_t * tmpString;
        svn_rangelist_to_string(&tmpString, rangelist, pool);
        printf("result %s\n", tmpString->data);
    }
    
    // wrong result
    // result 15014-19472,19473-19612*,19613-19614,19615-19630*,19634-19631*,19631-19634,19635-20515*
    
    svn_pool_destroy(pool);
    return exit_code;
}

So while this correctly expands the last range from 20055 to 20515 it inserts a wrong inverse range. It’s a bit late, I’ll have a look tomorrow unless someone beats me to it. ;)

> Am 30.06.2017 um 20:50 schrieb Johan Corveleyn <jc...@gmail.com>:
> 
> On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
> <je...@playtonicgames.com> wrote:
>> I narrowed it down to somewhere in update_wc_mergeinfo. At the start of the
>> function where it gets the mergeinfo for the root directory it is:
>> /trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,19635-2
>> 0055*
>> 
>> A bit later where this gets parsed again from svn_wc_canonicalize_svn_prop
>> it is:
>> /trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19634-19631*,19631-
>> 19634,19635-20511*
>> 
>> So somewhere in the mergeinfo update this gets added.
>> 
>> Debugging this in gdb is not fun, though...
> 
> Maybe it comes from a parent or child of your root directory (either
> in the working copy, or even part of the repository but outside of the
> scope of your working copy). Mergeinfo has some inheritance semantics,
> so it doesn't have to be directly defined on your root directory to be
> applicable.
> 
> -- 
> Johan


Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Wed, 05 Jul 2017 09:31 +0200:
> On 04.07.2017 16:33, Daniel Shahaf wrote:
> > Jens Christian Restemeier wrote on Tue, 04 Jul 2017 15:10 +0100:
> >> I ran "make tests", which fails for undefined references to gmock...
> > The build.conf stanzas for the cxxhl bindings declare them as «install =
> > tests», which causes a «make tests» target to be created for compiling
> > and running the cxxhl bindings tests.
> >
> > I'm not sure whether that was intentional.
> 
> It was not.

Fixed in r1800849.

Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Branko Čibej <br...@apache.org>.
On 04.07.2017 16:33, Daniel Shahaf wrote:
> Jens Christian Restemeier wrote on Tue, 04 Jul 2017 15:10 +0100:
>> I ran "make tests", which fails for undefined references to gmock...
> The build.conf stanzas for the cxxhl bindings declare them as «install =
> tests», which causes a «make tests» target to be created for compiling
> and running the cxxhl bindings tests.
>
> I'm not sure whether that was intentional.

It was not.

The right way to run our tests is 'make check'.

-- Brane


Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jens Christian Restemeier wrote on Tue, 04 Jul 2017 15:10 +0100:
> I ran "make tests", which fails for undefined references to gmock...

The build.conf stanzas for the cxxhl bindings declare them as «install =
tests», which causes a «make tests» target to be created for compiling
and running the cxxhl bindings tests.

I'm not sure whether that was intentional.

RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Jens Christian Restemeier <je...@playtonicgames.com>.
I ran "make tests", which fails for undefined references to gmock... I'll try your instructions.
I didn’t think you would look at this right away... :)

-----Original Message-----
From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name] 
Sent: 04 July 2017 15:01
To: Jens Christian Restemeier <je...@playtonicgames.com>
Cc: Johan Corveleyn <jc...@gmail.com>; users@subversion.apache.org; Stefan Sperling <st...@elego.de>
Subject: Re: "Unable to parse reversed revision range" when merging from trunk to branch

Jens Christian Restemeier wrote on Tue, 04 Jul 2017 14:43 +0100:
> I attached an patch with a test to the bug. I can't get gmock to work at the moment, so I have no idea if this test works...

gmock is not required for running the tests; you only need python 2.7.
To run a single test, the invocation is

    make mergeinfo-test && ./subversion/tests/libsvn_subr/mergeinfo-test
or
    make check TESTS=subversion/tests/libsvn_subr/mergeinfo-test

What made you think gmock was required?

For future reference, when sending patches use either 'svn diff' or 'diff -u' to produce unified diff formatted output.

Note that Bert committed a test based on your test program in r1800754.  (I haven't checked how your new patch relates to that revision)

Feel free to join #svn-dev on freenode IRC as well.

Cheers,

Daniel

> It looks like the URL for gmock has changed since get-deps.sh was written, and it is not distributed as fused files.
> 
> Instead of building the rangelists it uses svn_rangelist__parse, which makes the test a bit more compact.



Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jens Christian Restemeier wrote on Tue, 04 Jul 2017 14:43 +0100:
> I attached an patch with a test to the bug. I can't get gmock to work at the moment, so I have no idea if this test works...

gmock is not required for running the tests; you only need python 2.7.
To run a single test, the invocation is

    make mergeinfo-test && ./subversion/tests/libsvn_subr/mergeinfo-test
or
    make check TESTS=subversion/tests/libsvn_subr/mergeinfo-test

What made you think gmock was required?

For future reference, when sending patches use either 'svn diff' or 'diff -u' to produce unified diff formatted output.

Note that Bert committed a test based on your test program in r1800754.  (I haven't checked how your new patch relates to that revision)

Feel free to join #svn-dev on freenode IRC as well.

Cheers,

Daniel

> It looks like the URL for gmock has changed since get-deps.sh was written, and it is not distributed as fused files.
> 
> Instead of building the rangelists it uses svn_rangelist__parse, which makes the test a bit more compact.


RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Jens Christian Restemeier <je...@playtonicgames.com>.
I attached an patch with a test to the bug. I can't get gmock to work at the moment, so I have no idea if this test works...
It looks like the URL for gmock has changed since get-deps.sh was written, and it is not distributed as fused files.

Instead of building the rangelists it uses svn_rangelist__parse, which makes the test a bit more compact.

-----Original Message-----
From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name] 
Sent: 04 July 2017 06:09
To: Jens Restemeier <je...@playtonicgames.com>
Cc: Johan Corveleyn <jc...@gmail.com>; users@subversion.apache.org; Stefan Sperling <st...@elego.de>
Subject: Re: "Unable to parse reversed revision range" when merging from trunk to branch

Jens Restemeier wrote on Mon, 03 Jul 2017 20:01 +0100:
> > Am 03.07.2017 um 19:17 schrieb Stefan Sperling <st...@elego.de>:
> > 
> > On Mon, Jul 03, 2017 at 03:31:00PM +0100, Jens Christian Restemeier wrote:
> >> Should I open a bug with my findings and the test case?
> > 
> > Yes, please open a bug in the issue tracker.
> > 
> Done: https://issues.apache.org/jira/browse/SVN-4686 
> <https://issues.apache.org/jira/browse/SVN-4686>

Thanks!

> > A regression test for the subversion/tests tree would be essential.
> > Since you already wrote a test program in C, this file would be a 
> > good location for such a test: subversion/libsvn_subr/mergeinfo.c

Stefan meant subversion/tests/libsvn_subr/mergeinfo-test.c .



Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jens Restemeier wrote on Mon, 03 Jul 2017 20:01 +0100:
> > Am 03.07.2017 um 19:17 schrieb Stefan Sperling <st...@elego.de>:
> > 
> > On Mon, Jul 03, 2017 at 03:31:00PM +0100, Jens Christian Restemeier wrote:
> >> Should I open a bug with my findings and the test case?
> > 
> > Yes, please open a bug in the issue tracker.
> > 
> Done: https://issues.apache.org/jira/browse/SVN-4686 <https://issues.apache.org/jira/browse/SVN-4686>

Thanks!

> > A regression test for the subversion/tests tree would be essential.
> > Since you already wrote a test program in C, this file would be
> > a good location for such a test: subversion/libsvn_subr/mergeinfo.c

Stefan meant subversion/tests/libsvn_subr/mergeinfo-test.c .


Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Paul Hammant <pa...@hammant.org>.
"SVN-4635: Cherry-pick merge scenario causes Svn to choke"
June 11th, 2016

Ponymail isn't letting me go back more than a month, so I can't give you a
link to the thread - sorry.

RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Bert Huijben <be...@qqmail.nl>.
Can you point to the discussion on the mailinglist where this issue was discussed before creating the issue in our tracker?

(We only create issues to track confirmed issues on the tracker *after* discussion here, or on the development list)

 

This is the first time I hear about this issue, but perhaps I missed the earlier discussion.

 

                Bert

 

From: Paul Hammant [mailto:paul@hammant.org] 
Sent: vrijdag 7 juli 2017 13:25
To: Jens Restemeier <je...@playtonicgames.com>
Cc: Stefan Sperling <st...@elego.de>; Johan Corveleyn <jc...@gmail.com>; users@subversion.apache.org
Subject: Re: "Unable to parse reversed revision range" when merging from trunk to branch

 

 

Done: https://issues.apache.org/jira/browse/SVN-4686

 

 

Jumping in with something that's in the same area - https://issues.apache.org/jira/browse/SVN-4635 is a merge tracking (or merge info/range props) bug that is still there in v1.9.6, in case anyone wants a scripted reproduction (30 seconds of bash script invocation).

 

-ph 


Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Paul Hammant <pa...@hammant.org>.
> Done: https://issues.apache.org/jira/browse/SVN-4686
>
>
Jumping in with something that's in the same area -
https://issues.apache.org/jira/browse/SVN-4635 is a merge tracking (or
merge info/range props) bug that is still there in v1.9.6, in case anyone
wants a scripted reproduction (30 seconds of bash script invocation).

-ph

Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Jens Restemeier <je...@playtonicgames.com>.
Done: https://issues.apache.org/jira/browse/SVN-4686 <https://issues.apache.org/jira/browse/SVN-4686>
> Am 03.07.2017 um 19:17 schrieb Stefan Sperling <st...@elego.de>:
> 
> On Mon, Jul 03, 2017 at 03:31:00PM +0100, Jens Christian Restemeier wrote:
>> Should I open a bug with my findings and the test case?
> 
> Yes, please open a bug in the issue tracker.
> 
> A regression test for the subversion/tests tree would be essential.
> Since you already wrote a test program in C, this file would be
> a good location for such a test: subversion/libsvn_subr/mergeinfo.c
> 
> We should definitely keep track of this problem and fix it on trunk.
> Perhaps even backport a fix to 1.9 once it exists.
> 
> Thank you very much for the work you have done so far!
> 
> Stefan


Re: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jul 03, 2017 at 03:31:00PM +0100, Jens Christian Restemeier wrote:
> Should I open a bug with my findings and the test case?

Yes, please open a bug in the issue tracker.

A regression test for the subversion/tests tree would be essential.
Since you already wrote a test program in C, this file would be
a good location for such a test: subversion/libsvn_subr/mergeinfo.c

We should definitely keep track of this problem and fix it on trunk.
Perhaps even backport a fix to 1.9 once it exists.

Thank you very much for the work you have done so far!

Stefan

RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Jens Christian Restemeier <je...@playtonicgames.com>.
You are correct, it is a sparse workspace directory, so I assume any merge touching the excluded directories is marked as non-recursive. 
The merge is a plain merge from trunk, so the "changes" range starts at the same revision as the rangelist (15014) and goes up to the then-current revision on trunk (20515). There are no gaps in rangelist and the last range has the same inheritance, so all it should do is extend the last range to 20515.

-----Original Message-----
From: Bert Huijben [mailto:bert@qqmail.nl] 
Sent: 04 July 2017 10:41
To: 'Jens Christian Restemeier' <je...@playtonicgames.com>; 'Johan Corveleyn' <jc...@gmail.com>
Cc: 'Stefan Sperling' <st...@elego.de>; users@subversion.apache.org
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch



> -----Original Message-----
> From: Jens Christian Restemeier [mailto:jens@playtonicgames.com]
> Sent: maandag 3 juli 2017 16:31
> To: 'Johan Corveleyn' <jc...@gmail.com>
> Cc: 'Stefan Sperling' <st...@elego.de>; users@subversion.apache.org
> Subject: RE: "Unable to parse reversed revision range" when merging 
> from trunk to branch
> 
> I narrowed it down to the code in subversion/libsvn_subr/mergeinfo.c 
> line
> 892-898 in adjust_remaining_ranges. At that point next_range actually 
> starts before modified_range, so my guess is svn_sort__array_insert 
> has some unexpected behaviour.
> 
>    x892                   svn_merge_range_t *new_modified_range =
> x
>    x893                     apr_palloc(result_pool, sizeof(*new_modified_range));
> x
>    x894                   new_modified_range->start = next_range->end;
> x
>    x895                   new_modified_range->end = modified_range->end;
> x
>    x896                   new_modified_range->inheritable = FALSE;
> x
>    x897                   modified_range->end = next_range->start;
> x
>    x898                   (*range_index)+=2;
> x
>   >x899                   svn_sort__array_insert(rangelist, &new_modified_range,
> x
>                                           *range_index); x
>    x901                   /* Recurse with the new range. */
> x
>    x902                   adjust_remaining_ranges(rangelist, range_index,
> result_pool);                                                                                                                     x
> 
> Intuitively it seems to be awfully complicated to expand a range to 
> the end of a change, and then cut it down recursively with adjust_remaining_ranges.
> My first thought would be to step through "rangelist" and "changes" 
> side by side in svn_rangelist_merge2, and to modify, insert or skip a 
> range in either list until you're at the end. Though obviously I have 
> no idea about all the edge cases the current code most likely fixes...
> 
> So how should I proceed from here? Should I open a bug with my 
> findings and the test case?

I see you already opened an issue.

To me it looks like the issue is related to mixing recursive and non-recursive mergeinfo inside the same range... (The ranges with and without a '*' in the string). I see that your example case in the issue has quite a bit of overlap with both these kinds of ranges.

It looks like your case triggers a very interesting edge case.

	Bert 




RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Jens Christian Restemeier <je...@playtonicgames.com>.
Just a quick update: I specified the range to merge from trunk manually and the merge completed without error. Looking at the mergeinfo property svn correctly extended the last revision range.

-----Original Message-----
From: Jens Christian Restemeier [mailto:jens@playtonicgames.com] 
Sent: 04 July 2017 11:01
To: 'Bert Huijben' <be...@qqmail.nl>; 'Johan Corveleyn' <jc...@gmail.com>
Cc: 'Stefan Sperling' <st...@elego.de>; 'users@subversion.apache.org' <us...@subversion.apache.org>
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch

You are correct, it is a sparse workspace directory, so I assume any merge touching the excluded directories is marked as non-recursive. 
The merge is a plain merge from trunk, so the "changes" range starts at the same revision as the rangelist (15014) and goes up to the then-current revision on trunk (20515). There are no gaps in rangelist and the last range has the same inheritance, so all it should do is extend the last range to 20515.

-----Original Message-----
From: Bert Huijben [mailto:bert@qqmail.nl] 
Sent: 04 July 2017 10:41
To: 'Jens Christian Restemeier' <je...@playtonicgames.com>; 'Johan Corveleyn' <jc...@gmail.com>
Cc: 'Stefan Sperling' <st...@elego.de>; users@subversion.apache.org
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch



> -----Original Message-----
> From: Jens Christian Restemeier [mailto:jens@playtonicgames.com]
> Sent: maandag 3 juli 2017 16:31
> To: 'Johan Corveleyn' <jc...@gmail.com>
> Cc: 'Stefan Sperling' <st...@elego.de>; users@subversion.apache.org
> Subject: RE: "Unable to parse reversed revision range" when merging 
> from trunk to branch
> 
> I narrowed it down to the code in subversion/libsvn_subr/mergeinfo.c 
> line
> 892-898 in adjust_remaining_ranges. At that point next_range actually 
> starts before modified_range, so my guess is svn_sort__array_insert 
> has some unexpected behaviour.
> 
>    x892                   svn_merge_range_t *new_modified_range =
> x
>    x893                     apr_palloc(result_pool, sizeof(*new_modified_range));
> x
>    x894                   new_modified_range->start = next_range->end;
> x
>    x895                   new_modified_range->end = modified_range->end;
> x
>    x896                   new_modified_range->inheritable = FALSE;
> x
>    x897                   modified_range->end = next_range->start;
> x
>    x898                   (*range_index)+=2;
> x
>   >x899                   svn_sort__array_insert(rangelist, &new_modified_range,
> x
>                                           *range_index); x
>    x901                   /* Recurse with the new range. */
> x
>    x902                   adjust_remaining_ranges(rangelist, range_index,
> result_pool);                                                                                                                     x
> 
> Intuitively it seems to be awfully complicated to expand a range to 
> the end of a change, and then cut it down recursively with adjust_remaining_ranges.
> My first thought would be to step through "rangelist" and "changes" 
> side by side in svn_rangelist_merge2, and to modify, insert or skip a 
> range in either list until you're at the end. Though obviously I have 
> no idea about all the edge cases the current code most likely fixes...
> 
> So how should I proceed from here? Should I open a bug with my 
> findings and the test case?

I see you already opened an issue.

To me it looks like the issue is related to mixing recursive and non-recursive mergeinfo inside the same range... (The ranges with and without a '*' in the string). I see that your example case in the issue has quite a bit of overlap with both these kinds of ranges.

It looks like your case triggers a very interesting edge case.

	Bert 




RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Jens Christian Restemeier [mailto:jens@playtonicgames.com]
> Sent: maandag 3 juli 2017 16:31
> To: 'Johan Corveleyn' <jc...@gmail.com>
> Cc: 'Stefan Sperling' <st...@elego.de>; users@subversion.apache.org
> Subject: RE: "Unable to parse reversed revision range" when merging from
> trunk to branch
> 
> I narrowed it down to the code in subversion/libsvn_subr/mergeinfo.c line
> 892-898 in adjust_remaining_ranges. At that point next_range actually starts
> before modified_range, so my guess is svn_sort__array_insert has some
> unexpected behaviour.
> 
>    x892                   svn_merge_range_t *new_modified_range =
> x
>    x893                     apr_palloc(result_pool, sizeof(*new_modified_range));
> x
>    x894                   new_modified_range->start = next_range->end;
> x
>    x895                   new_modified_range->end = modified_range->end;
> x
>    x896                   new_modified_range->inheritable = FALSE;
> x
>    x897                   modified_range->end = next_range->start;
> x
>    x898                   (*range_index)+=2;
> x
>   >x899                   svn_sort__array_insert(rangelist, &new_modified_range,
> x
>                                           *range_index);
> x
>    x901                   /* Recurse with the new range. */
> x
>    x902                   adjust_remaining_ranges(rangelist, range_index,
> result_pool);                                                                                                                     x
> 
> Intuitively it seems to be awfully complicated to expand a range to the end of
> a change, and then cut it down recursively with adjust_remaining_ranges.
> My first thought would be to step through "rangelist" and "changes" side by
> side in svn_rangelist_merge2, and to modify, insert or skip a range in either
> list until you're at the end. Though obviously I have no idea about all the edge
> cases the current code most likely fixes...
> 
> So how should I proceed from here? Should I open a bug with my findings
> and the test case?

I see you already opened an issue.

To me it looks like the issue is related to mixing recursive and non-recursive mergeinfo inside the same range... (The ranges with and without a '*' in the string). I see that your example case in the issue has quite a bit of overlap with both these kinds of ranges.

It looks like your case triggers a very interesting edge case.

	Bert 



RE: "Unable to parse reversed revision range" when merging from trunk to branch

Posted by Jens Christian Restemeier <je...@playtonicgames.com>.
I narrowed it down to the code in subversion/libsvn_subr/mergeinfo.c line 892-898 in adjust_remaining_ranges. At that point next_range actually starts before modified_range, so my guess is svn_sort__array_insert has some unexpected behaviour.

   x892                   svn_merge_range_t *new_modified_range =                                                                                                                                           x
   x893                     apr_palloc(result_pool, sizeof(*new_modified_range));                                                                                                                           x
   x894                   new_modified_range->start = next_range->end;                                                                                                                                      x
   x895                   new_modified_range->end = modified_range->end;                                                                                                                                    x
   x896                   new_modified_range->inheritable = FALSE;                                                                                                                                          x
   x897                   modified_range->end = next_range->start;                                                                                                                                          x
   x898                   (*range_index)+=2;                                                                                                                                                                x
  >x899                   svn_sort__array_insert(rangelist, &new_modified_range,                                                                                                                            x
                                          *range_index);                                                                                                                                             x
   x901                   /* Recurse with the new range. */                                                                                                                                                 x
   x902                   adjust_remaining_ranges(rangelist, range_index, result_pool);                                                                                                                     x
   
Intuitively it seems to be awfully complicated to expand a range to the end of a change, and then cut it down recursively with adjust_remaining_ranges. My first thought would be to step through "rangelist" and "changes" side by side in svn_rangelist_merge2, and to modify, insert or skip a range in either list until you're at the end. Though obviously I have no idea about all the edge cases the current code most likely fixes...

So how should I proceed from here? Should I open a bug with my findings and the test case?

-----Original Message-----
From: Jens Restemeier [mailto:jens@playtonicgames.com] 
Sent: 02 July 2017 23:44
To: Johan Corveleyn <jc...@gmail.com>
Cc: Stefan Sperling <st...@elego.de>; users@subversion.apache.org
Subject: Re: "Unable to parse reversed revision range" when merging from trunk to branch

The problem seems to come from svn_rangelist_merge2.

This test program recreates the problem. 

#include <svn_pools.h>
#include <svn_mergeinfo.h>

int main(int argc, char * argv[])
{
    apr_pool_t *pool;
    int exit_code = EXIT_SUCCESS;
    svn_error_t *err;
    
    if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
        return EXIT_FAILURE;
    
    pool = apr_allocator_owner_get(svn_pool_create_allocator(FALSE));
    
    // 15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,19635-20055*
    svn_rangelist_t * rangelist = apr_array_make(pool, 1, sizeof(svn_merge_range_t *));
    svn_merge_range_t *mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 15013;
    mrange->end = 19472;
    mrange->inheritable = TRUE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19472;
    mrange->end = 19612;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19612;
    mrange->end = 19614;
    mrange->inheritable = TRUE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19614;
    mrange->end = 19630;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19630;
    mrange->end = 19634;
    mrange->inheritable = TRUE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 19634;
    mrange->end = 20055;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = mrange;
    
    // 15014-20515*
    svn_rangelist_t * changes = apr_array_make(pool, 1, sizeof(svn_merge_range_t *));
    mrange = apr_pcalloc(pool, sizeof(*mrange));
    mrange->start = 15013;
    mrange->end = 20515;
    mrange->inheritable = FALSE;
    APR_ARRAY_PUSH(changes, svn_merge_range_t *) = mrange;
    
    {
        svn_string_t * tmpString;
        svn_rangelist_to_string(&tmpString, rangelist, pool);
        printf("rangelist %s\n", tmpString->data);
    }
    {
        svn_string_t * tmpString;
        svn_rangelist_to_string(&tmpString, changes, pool);
        printf("changes %s\n", tmpString->data);
    }
    
    svn_rangelist_merge2(rangelist, changes, pool, pool);
    
    {
        svn_string_t * tmpString;
        svn_rangelist_to_string(&tmpString, rangelist, pool);
        printf("result %s\n", tmpString->data);
    }
    
    // wrong result
    // result 15014-19472,19473-19612*,19613-19614,19615-19630*,19634-19631*,19631-19634,19635-20515*
    
    svn_pool_destroy(pool);
    return exit_code;
}

So while this correctly expands the last range from 20055 to 20515 it inserts a wrong inverse range. It’s a bit late, I’ll have a look tomorrow unless someone beats me to it. ;)

> Am 30.06.2017 um 20:50 schrieb Johan Corveleyn <jc...@gmail.com>:
> 
> On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier 
> <je...@playtonicgames.com> wrote:
>> I narrowed it down to somewhere in update_wc_mergeinfo. At the start 
>> of the function where it gets the mergeinfo for the root directory it is:
>> /trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,
>> 19635-2
>> 0055*
>> 
>> A bit later where this gets parsed again from 
>> svn_wc_canonicalize_svn_prop it is:
>> /trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19634-19631*
>> ,19631-
>> 19634,19635-20511*
>> 
>> So somewhere in the mergeinfo update this gets added.
>> 
>> Debugging this in gdb is not fun, though...
> 
> Maybe it comes from a parent or child of your root directory (either 
> in the working copy, or even part of the repository but outside of the 
> scope of your working copy). Mergeinfo has some inheritance semantics, 
> so it doesn't have to be directly defined on your root directory to be 
> applicable.
> 
> --
> Johan