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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2

I posted this already to the TortoiseSVN users list, but I think this issue may be on the client layer and not the tortoise interface.

 

I'm trying to catch up a branch with the changes on trunk. It seems to update the correct files, though at the end it fails with

"Unable to parse reversed revision range '19634-19631'"

I'm using TortoiseSVN 1.9.5 on Windows 8.1:

  X:\Subversion >s​vn --version
  svn, version 1.9.5 (r1770682)

I looked both on trunk and the branch for odd mergeinfo. Trunk has no mergeinfo newer than the branch (the last merge to it was ages ago). I did a search for those revisions on the branch, but they don't appear reversed:

  X:\Subversion>svn propget svn:mergeinfo --depth=infinity | find "19631"
  /trunk:15014-19472,1​9473-19612*,19613-19​614,19615-19630*,196​31-19634,19635-20055​*

What exactly is that error message telling me, and how do I resolve it?

I did a google search, but the responses I found were for svn versions from years ago, or for svndumps.

I'm tempted to just copy everything over and to hack the mergeinfo manually, but I'm sure that'll cause more problems.

 

I just tried a “Test” merge in TortoiseSVN and that completed without that error.

 

Cheers,

Jens

 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Johan Corveleyn-3
On Thu, Jun 29, 2017 at 3:47 PM, Jens Christian Restemeier
<[hidden email]> wrote:

> I posted this already to the TortoiseSVN users list, but I think this issue
> may be on the client layer and not the tortoise interface.
>
>
>
> I'm trying to catch up a branch with the changes on trunk. It seems to
> update the correct files, though at the end it fails with
>
> "Unable to parse reversed revision range '19634-19631'"
>
> I'm using TortoiseSVN 1.9.5 on Windows 8.1:
>
>   X:\Subversion >svn --version
>   svn, version 1.9.5 (r1770682)
>
> I looked both on trunk and the branch for odd mergeinfo. Trunk has no
> mergeinfo newer than the branch (the last merge to it was ages ago). I did a
> search for those revisions on the branch, but they don't appear reversed:
>
>   X:\Subversion>svn propget svn:mergeinfo --depth=infinity | find "19631"
>
> /trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,19635-20055*
>
> What exactly is that error message telling me, and how do I resolve it?
>
> I did a google search, but the responses I found were for svn versions from
> years ago, or for svndumps.
>
> I'm tempted to just copy everything over and to hack the mergeinfo manually,
> but I'm sure that'll cause more problems.
>
>
>
> I just tried a “Test” merge in TortoiseSVN and that completed without that
> error.
>

That's quite strange. The error message comes from this line in the source code:

http://svn.apache.org/viewvc/subversion/tags/1.9.5/subversion/libsvn_subr/mergeinfo.c?view=markup#l536

I'm not familiar with that part of the code, but I can only imagine
that somehow this reversed range gets processed during the merge
operation.

Can you reproduce this with commandline svn, and get the same error
message that way? If so, can you give the exact command?

I still think this reversed range is somewhere present in the
svn:mergeinfo property, where it shouldn't be.

--
Johan
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Daniel Shahaf-2
Johan Corveleyn wrote on Thu, 29 Jun 2017 22:55 +0200:

> On Thu, Jun 29, 2017 at 3:47 PM, Jens Christian Restemeier
> <[hidden email]> wrote:
> >   X:\Subversion>svn propget svn:mergeinfo --depth=infinity | find "19631"
> >
> > /trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,19635-20055*
>
> http://svn.apache.org/viewvc/subversion/tags/1.9.5/subversion/libsvn_subr/mergeinfo.c?view=markup#l536
>
> I'm not familiar with that part of the code, but I can only imagine
> that somehow this reversed range gets processed during the merge
> operation.

> I still think this reversed range is somewhere present in the
> svn:mergeinfo property, where it shouldn't be.

Unless 'svn merge' has learnt to look for svn:mergeinfo above the cwd
(which appears to be a wc root in this case) or below cropped subtrees,
the natural place to look for the reversed ranged would be in
svn:mergeinfo of the tree being merged.  (The one passed to 'svn merge'
as a positional argument.)
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
In reply to this post by Johan Corveleyn-3
I made a clean checkout of the same branch onto my Linux  machine, to make sure the workspace doesn't contain any odd leftover data. The workspace is sparse, it excludes a few folders at the root level contain only data that is irrelevant for code work. It's the same checkout as on my Windows machine, though.

svn --version
svn, version 1.9.5 (r1770682)
   compiled Dec  1 2016, 14:48:33 on x86_64-unknown-linux-gnu

(This is the Wandisco build. I installed the sourcecode and the dependencies as well in case extra logging or breakpoints are required.)

svn merge ^/trunk --dry-run

This completes with a few conflicts but without errors.

svn merge ^/trunk

This fails with:

--- Recording mergeinfo for merge of r15014 through r20508 into '.':
Summary of conflicts:
  Text conflicts: 1
  Tree conflicts: 2
  Skipped paths: 1
svn: E200020: Unable to parse reversed revision range '19634-19631'

I looked on trunk, and the highest mergeinfo I can find is for 19432:
/branches/xxxxxxx:19329,19406,19431-19432

The majority are ancient merges with mergeinfo <10000

I can add some diagnostics to the mergeinfo parser, though I've got some work to finish first.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Stefan Sperling
On Fri, Jun 30, 2017 at 01:59:02PM +0100, Jens Christian Restemeier wrote:
> I can add some diagnostics to the mergeinfo parser, though I've got some work to finish first.

Please do. Don't forget to pack a pickaxe, a torch, and some
elixirs to restore HP.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
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...

-----Original Message-----
From: Stefan Sperling [mailto:[hidden email]]
Sent: 30 June 2017 14:10
To: Jens Christian Restemeier <[hidden email]>
Cc: 'Johan Corveleyn' <[hidden email]>; [hidden email]
Subject: Re: "Unable to parse reversed revision range" when merging from
trunk to branch

On Fri, Jun 30, 2017 at 01:59:02PM +0100, Jens Christian Restemeier wrote:
> I can add some diagnostics to the mergeinfo parser, though I've got some
work to finish first.

Please do. Don't forget to pack a pickaxe, a torch, and some elixirs to
restore HP.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Johan Corveleyn-3
On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
<[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
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 <[hidden email]>:
>
> On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
> <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
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:[hidden email]]
Sent: 02 July 2017 23:44
To: Johan Corveleyn <[hidden email]>
Cc: Stefan Sperling <[hidden email]>; [hidden email]
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 <[hidden email]>:
>
> On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
> <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Stefan Sperling
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2

Am 03.07.2017 um 19:17 schrieb Stefan Sperling <[hidden email]>:

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Daniel Shahaf-2
Jens Restemeier wrote on Mon, 03 Jul 2017 20:01 +0100:
> > Am 03.07.2017 um 19:17 schrieb Stefan Sperling <[hidden email]>:
> >
> > 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 .

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Bert Huijben-5
In reply to this post by Jens Christian Restemeier-2


> -----Original Message-----
> From: Jens Christian Restemeier [mailto:[hidden email]]
> Sent: maandag 3 juli 2017 16:31
> To: 'Johan Corveleyn' <[hidden email]>
> Cc: 'Stefan Sperling' <[hidden email]>; [hidden email]
> 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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
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:[hidden email]]
Sent: 04 July 2017 10:41
To: 'Jens Christian Restemeier' <[hidden email]>; 'Johan Corveleyn' <[hidden email]>
Cc: 'Stefan Sperling' <[hidden email]>; [hidden email]
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch



> -----Original Message-----
> From: Jens Christian Restemeier [mailto:[hidden email]]
> Sent: maandag 3 juli 2017 16:31
> To: 'Johan Corveleyn' <[hidden email]>
> Cc: 'Stefan Sperling' <[hidden email]>; [hidden email]
> 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



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
In reply to this post by Bert Huijben-5
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:[hidden email]]
Sent: 04 July 2017 11:01
To: 'Bert Huijben' <[hidden email]>; 'Johan Corveleyn' <[hidden email]>
Cc: 'Stefan Sperling' <[hidden email]>; '[hidden email]' <[hidden email]>
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:[hidden email]]
Sent: 04 July 2017 10:41
To: 'Jens Christian Restemeier' <[hidden email]>; 'Johan Corveleyn' <[hidden email]>
Cc: 'Stefan Sperling' <[hidden email]>; [hidden email]
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch



> -----Original Message-----
> From: Jens Christian Restemeier [mailto:[hidden email]]
> Sent: maandag 3 juli 2017 16:31
> To: 'Johan Corveleyn' <[hidden email]>
> Cc: 'Stefan Sperling' <[hidden email]>; [hidden email]
> 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



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
In reply to this post by Daniel Shahaf-2
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:[hidden email]]
Sent: 04 July 2017 06:09
To: Jens Restemeier <[hidden email]>
Cc: Johan Corveleyn <[hidden email]>; [hidden email]; Stefan Sperling <[hidden email]>
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 <[hidden email]>:
> >
> > 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 .


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Daniel Shahaf-2
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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jens Christian Restemeier-2
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:[hidden email]]
Sent: 04 July 2017 15:01
To: Jens Christian Restemeier <[hidden email]>
Cc: Johan Corveleyn <[hidden email]>; [hidden email]; Stefan Sperling <[hidden email]>
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.


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Daniel Shahaf-2
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.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Branko Čibej
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

12
Loading...