Discussion:
"Unable to parse reversed revision range" when merging from trunk to branch
Jens Christian Restemeier
2017-06-29 13:47:03 UTC
Permalink
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
Johan Corveleyn
2017-06-29 20:55:34 UTC
Permalink
On Thu, Jun 29, 2017 at 3:47 PM, Jens Christian Restemeier
Post by Jens Christian Restemeier
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'"
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
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
Daniel Shahaf
2017-06-30 02:17:44 UTC
Permalink
Post by Johan Corveleyn
On Thu, Jun 29, 2017 at 3:47 PM, Jens Christian Restemeier
Post by Jens Christian Restemeier
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.

Post by Johan Corveleyn
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.)
Jens Christian Restemeier
2017-06-30 12:59:02 UTC
Permalink
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.
Stefan Sperling
2017-06-30 13:09:36 UTC
Permalink
Post by Jens Christian Restemeier
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.
Jens Christian Restemeier
2017-06-30 16:10:55 UTC
Permalink
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:***@elego.de]
Sent: 30 June 2017 14:10
To: Jens Christian Restemeier <***@playtonicgames.com>
Cc: 'Johan Corveleyn' <***@gmail.com>; ***@subversion.apache.org
Subject: Re: "Unable to parse reversed revision range" when merging from
trunk to branch
Post by Jens Christian Restemeier
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.
Johan Corveleyn
2017-06-30 19:50:07 UTC
Permalink
On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
Post by Jens Christian Restemeier
I narrowed it down to somewhere in update_wc_mergeinfo. At the start of the
/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
/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
Jens Restemeier
2017-07-02 22:44:00 UTC
Permalink
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. ;)
Post by Johan Corveleyn
On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
Post by Jens Christian Restemeier
I narrowed it down to somewhere in update_wc_mergeinfo. At the start of the
/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
/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
Jens Christian Restemeier
2017-07-03 14:31:00 UTC
Permalink
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:***@playtonicgames.com]
Sent: 02 July 2017 23:44
To: Johan Corveleyn <***@gmail.com>
Cc: Stefan Sperling <***@elego.de>; ***@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. ;)
On Fri, Jun 30, 2017 at 6:10 PM, Jens Christian Restemeier
Post by Jens Christian Restemeier
I narrowed it down to somewhere in update_wc_mergeinfo. At the start
/trunk:15014-19472,19473-19612*,19613-19614,19615-19630*,19631-19634,
19635-2
0055*
A bit later where this gets parsed again from
/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
Stefan Sperling
2017-07-03 18:17:41 UTC
Permalink
Post by Jens Christian Restemeier
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
Jens Restemeier
2017-07-03 19:01:19 UTC
Permalink
Done: https://issues.apache.org/jira/browse/SVN-4686 <https://issues.apache.org/jira/browse/SVN-4686>
Post by Stefan Sperling
Post by Jens Christian Restemeier
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
Daniel Shahaf
2017-07-04 05:08:57 UTC
Permalink
Post by Jens Restemeier
Post by Stefan Sperling
Post by Jens Christian Restemeier
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!
Post by Jens Restemeier
Post by Stefan Sperling
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 .
Jens Christian Restemeier
2017-07-04 13:43:49 UTC
Permalink
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:***@daniel.shahaf.name]
Sent: 04 July 2017 06:09
To: Jens Restemeier <***@playtonicgames.com>
Cc: Johan Corveleyn <***@gmail.com>; ***@subversion.apache.org; Stefan Sperling <***@elego.de>
Subject: Re: "Unable to parse reversed revision range" when merging from trunk to branch
Post by Jens Restemeier
Post by Stefan Sperling
Post by Jens Christian Restemeier
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!
Post by Jens Restemeier
Post by Stefan Sperling
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 .
Daniel Shahaf
2017-07-04 14:01:18 UTC
Permalink
Post by Jens Christian Restemeier
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
Post by Jens Christian Restemeier
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.
Jens Christian Restemeier
2017-07-04 14:10:28 UTC
Permalink
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:***@daniel.shahaf.name]
Sent: 04 July 2017 15:01
To: Jens Christian Restemeier <***@playtonicgames.com>
Cc: Johan Corveleyn <***@gmail.com>; ***@subversion.apache.org; Stefan Sperling <***@elego.de>
Subject: Re: "Unable to parse reversed revision range" when merging from trunk to branch
Post by Jens Christian Restemeier
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
Post by Jens Christian Restemeier
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.
Daniel Shahaf
2017-07-04 14:33:56 UTC
Permalink
Post by Jens Christian Restemeier
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.
Branko Čibej
2017-07-05 07:31:20 UTC
Permalink
Post by Daniel Shahaf
Post by Jens Christian Restemeier
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
Daniel Shahaf
2017-07-05 08:35:30 UTC
Permalink
Post by Branko Čibej
Post by Daniel Shahaf
Post by Jens Christian Restemeier
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.
Paul Hammant
2017-07-07 11:25:11 UTC
Permalink
Post by Jens Restemeier
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
Bert Huijben
2017-07-07 15:26:11 UTC
Permalink
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:***@hammant.org]
Sent: vrijdag 7 juli 2017 13:25
To: Jens Restemeier <***@playtonicgames.com>
Cc: Stefan Sperling <***@elego.de>; Johan Corveleyn <***@gmail.com>; ***@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
Paul Hammant
2017-07-07 15:38:52 UTC
Permalink
"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.

Bert Huijben
2017-07-04 09:40:35 UTC
Permalink
Post by Jens Christian Restemeier
-----Original Message-----
Sent: maandag 3 juli 2017 16:31
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
Jens Christian Restemeier
2017-07-04 10:01:10 UTC
Permalink
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:***@qqmail.nl]
Sent: 04 July 2017 10:41
To: 'Jens Christian Restemeier' <***@playtonicgames.com>; 'Johan Corveleyn' <***@gmail.com>
Cc: 'Stefan Sperling' <***@elego.de>; ***@subversion.apache.org
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch
Post by Jens Christian Restemeier
-----Original Message-----
Sent: maandag 3 juli 2017 16:31
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
Jens Christian Restemeier
2017-07-04 10:18:26 UTC
Permalink
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:***@playtonicgames.com]
Sent: 04 July 2017 11:01
To: 'Bert Huijben' <***@qqmail.nl>; 'Johan Corveleyn' <***@gmail.com>
Cc: 'Stefan Sperling' <***@elego.de>; '***@subversion.apache.org' <***@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:***@qqmail.nl]
Sent: 04 July 2017 10:41
To: 'Jens Christian Restemeier' <***@playtonicgames.com>; 'Johan Corveleyn' <***@gmail.com>
Cc: 'Stefan Sperling' <***@elego.de>; ***@subversion.apache.org
Subject: RE: "Unable to parse reversed revision range" when merging from trunk to branch
Post by Jens Christian Restemeier
-----Original Message-----
Sent: maandag 3 juli 2017 16:31
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
Loading...