You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Eichin <ei...@gmail.com> on 2008/10/31 00:09:53 UTC
svn mergeinfo bug with script to reproduce it (finally)! (was Re: empty mergeinfo produced by svn_mergeinfo_inheritable)
After a bit more examination of the repository where I'm having this
problem, the cross-directory aspects finally became clear, and I was
able to produce a testcase script (attached) that when run with 1.5.x
produces this error on the final merge command:
--- Merging r11 into '.':
U other.py
svn: Mergeinfo for '/trunk/product/frob' maps to an empty revision range
I'm fairly sure that all of the steps are reasonable end-user svn
operations - the use of --depth is new, but not unusual, and if you
examine the repository (and particularly the svn:mergeinfo properties)
at the point of failure, I don't think any of it is wrong or
inconsistent.
This narrows the problem down to two possibilities:
* the mergeinfo being constructed *is* inconsistent, and the
mergeinfo construction side needs to change
(note that this also leads to a "so how do we repair existing
repositories" question...)
* the mergeinfo is fine, but the svn_mergeinfo_inheritable is wrong
as I theorized in my earlier post (with patch) and it's really enough
to discard the non-relevant mergeinfo entierly, instead of only
collapsing the rangelist.
Looking forward to further commentary...
On Thu, Oct 30, 2008 at 2:49 PM, Mark Eichin <ei...@gmail.com> wrote:
> Narrowed down the failure case to one where get_mergeinfo_for_path
> walks up the parent directories, finds non-inheritable svn:mergeinfo
> on a directory, stops there (correctly) but then
> svn_mergeinfo_inheritable filters it down to an (invalid) path with an
> empty mergelist (instead of no path at all.) Still haven't built a
> constructive example, though. (I have confirmed that neither
> svn_mergeinfo_inheritable nor append_to_merged_froms have the relevant
> filtering needed to avoid it, on trunk...)
>
> On Fri, Oct 24, 2008 at 4:41 PM, Mark Eichin <ei...@gmail.com> wrote:
>> I've been hunting down a "Mergeinfo for '...' maps to an empty
>> revision range" problem, and after cooking up a bunch of gdb macros
>> and tracing through
>> svn_fs_get_mergeinfo I think I have a handle on it. (I haven't
>> produced a stripped down case yet, I've been working with a copy of
>> our 17G 86000rev repository, though I'll attempt that next - I (and my
>> coworkers) wanted a second opinion on the analysis, though.)
>>
>> The actual failure occurs in either an attempted merge or a "log -g";
>> the latter was easier to work with... do_logs calls
>> get_combined_mergeinfo_changes, which then calls
>> svn_fs_get_mergeinfo, eventually calling get_mergeinfo_for_path to
>> actually resolve the inherited mergeinfo. It finds some mergeinfo
>> higher up the tree, calls svn_mergeinfo_inheritable to filter it
>> (which produces an svn_mergeinfo_t with paths that have empty
>> rangelists.) That's where the problem appears; the empty rangelists
>> eventually get unparsed into a string that gets reparsed by
>> svn_parse_mergeinfo, which blows up on the empty values.
>>
>> The workaround I'm looking at is to simply filter the empty mergeinfo
>> in append_to_merged_froms (patch below.) It might be more correct to
>> prevent svn_mergeinfo_inheritable from generating them in the first
>> place - *is* there a general rule about where producing/discarding
>> empty mergeinfo should happen? For example, svn_rangelist_diff talks
>> about permitting empty rangelists in their arguments, so they're not
>> completely forbidden...
>>
>> Index: subversion/libsvn_fs_fs/tree.c
>> ===================================================================
>> --- subversion/libsvn_fs_fs/tree.c (revision 87267)
>> +++ subversion/libsvn_fs_fs/tree.c (working copy)
>> @@ -3502,6 +3504,10 @@
>> char *newpath;
>>
>> apr_hash_this(hi, &key, NULL, &val);
>> + if (((apr_array_header_t *)val)->nelts == 0) {
>> + /* [eichin:20081024T1511-04] don't propagate an empty one! */
>> + continue;
>> + }
>> newpath = svn_path_join((const char *) key, path_piece, pool);
>> apr_hash_set(*output, newpath, APR_HASH_KEY_STRING,
>> svn_rangelist_dup((apr_array_header_t *) val, pool));
>>
>> (ignore the revision numbers, they're relative to a local repository,
>> though the function in question appears unchanged in 1.5.4, and 1.5.4
>> does exhibit the original failure.)
>>
>> --
>> _Mark_ <ei...@metacarta.com> <ei...@gmail.com>
>>
>
>
>
> --
> _Mark_ <ei...@thok.org> <ei...@gmail.com>
>
--
_Mark_ <ei...@thok.org> <ei...@gmail.com>
Re: svn mergeinfo bug with script to reproduce it (finally)! (was Re: empty mergeinfo produced by svn_mergeinfo_inheritable)
Posted by Arcady Volman <ar...@gmail.com>.
Hello,
Will this patch help me with a repository that already has this problem ?
Thanks,
Arcady
On Wed, Nov 26, 2008 at 12:03 AM, Mark Eichin <ei...@gmail.com> wrote:
> Sweet! Thanks.
>
> On Nov 25, 2008 2:32 PM, "Paul Burba" <pt...@gmail.com> wrote:
>
> On Fri, Oct 31, 2008 at 12:47 PM, Mark Eichin <ei...@gmail.com> wrote: >
> I've confirmed this fails ...
> Hi Mark,
>
> Your patch at
> http://subversion.tigris.org/nonav/issues/showattachment.cgi/949/svn_inheritable_rangelist.patch
> was the right approach -- svn_mergeinfo_inheritable should not be
> allowed to create syntactically incorrect mergeinfo. I committed it
> (with some minor comment additions) in r34426. Nominated it for
> backport to 1.5.x as well.
>
> Thanks for the thorough reproduction script and legwork in finding this
> problem.
>
> Paul
>
>
--
Best Regards,
Arcady Volman.
Re: svn mergeinfo bug with script to reproduce it (finally)! (was Re: empty mergeinfo produced by svn_mergeinfo_inheritable)
Posted by Mark Eichin <ei...@gmail.com>.
Sweet! Thanks.
On Nov 25, 2008 2:32 PM, "Paul Burba" <pt...@gmail.com> wrote:
On Fri, Oct 31, 2008 at 12:47 PM, Mark Eichin <ei...@gmail.com> wrote: >
I've confirmed this fails ...
Hi Mark,
Your patch at
http://subversion.tigris.org/nonav/issues/showattachment.cgi/949/svn_inheritable_rangelist.patch
was the right approach -- svn_mergeinfo_inheritable should not be
allowed to create syntactically incorrect mergeinfo. I committed it
(with some minor comment additions) in r34426. Nominated it for
backport to 1.5.x as well.
Thanks for the thorough reproduction script and legwork in finding this
problem.
Paul
Re: svn mergeinfo bug with script to reproduce it (finally)! (was Re: empty mergeinfo produced by svn_mergeinfo_inheritable)
Posted by Paul Burba <pt...@gmail.com>.
On Fri, Oct 31, 2008 at 12:47 PM, Mark Eichin <ei...@gmail.com> wrote:
> I've confirmed this fails on 1.5.1 and 1.5.4; anyone want to run the
> script against a trunk install? I'd like to get this filed (well,
> what I'd really like is someone to confirm that my fix is credible so
> I can justify using it on our local build and get the entire issue off
> my plate, but having it be a confirmed Issue would help with that :-)
>
> On Thu, Oct 30, 2008 at 8:09 PM, Mark Eichin <ei...@gmail.com> wrote:
>> After a bit more examination of the repository where I'm having this
>> problem, the cross-directory aspects finally became clear, and I was
>> able to produce a testcase script (attached) that when run with 1.5.x
>> produces this error on the final merge command:
>>
>> --- Merging r11 into '.':
>> U other.py
>> svn: Mergeinfo for '/trunk/product/frob' maps to an empty revision range
>>
>> I'm fairly sure that all of the steps are reasonable end-user svn
>> operations - the use of --depth is new, but not unusual, and if you
>> examine the repository (and particularly the svn:mergeinfo properties)
>> at the point of failure, I don't think any of it is wrong or
>> inconsistent.
>>
>> This narrows the problem down to two possibilities:
>> * the mergeinfo being constructed *is* inconsistent, and the
>> mergeinfo construction side needs to change
>> (note that this also leads to a "so how do we repair existing
>> repositories" question...)
>> * the mergeinfo is fine, but the svn_mergeinfo_inheritable is wrong
>> as I theorized in my earlier post (with patch) and it's really enough
>> to discard the non-relevant mergeinfo entierly, instead of only
>> collapsing the rangelist.
>>
>> Looking forward to further commentary...
>>
>>
>> On Thu, Oct 30, 2008 at 2:49 PM, Mark Eichin <ei...@gmail.com> wrote:
>>> Narrowed down the failure case to one where get_mergeinfo_for_path
>>> walks up the parent directories, finds non-inheritable svn:mergeinfo
>>> on a directory, stops there (correctly) but then
>>> svn_mergeinfo_inheritable filters it down to an (invalid) path with an
>>> empty mergelist (instead of no path at all.) Still haven't built a
>>> constructive example, though. (I have confirmed that neither
>>> svn_mergeinfo_inheritable nor append_to_merged_froms have the relevant
>>> filtering needed to avoid it, on trunk...)
>>>
>>> On Fri, Oct 24, 2008 at 4:41 PM, Mark Eichin <ei...@gmail.com> wrote:
>>>> I've been hunting down a "Mergeinfo for '...' maps to an empty
>>>> revision range" problem, and after cooking up a bunch of gdb macros
>>>> and tracing through
>>>> svn_fs_get_mergeinfo I think I have a handle on it. (I haven't
>>>> produced a stripped down case yet, I've been working with a copy of
>>>> our 17G 86000rev repository, though I'll attempt that next - I (and my
>>>> coworkers) wanted a second opinion on the analysis, though.)
>>>>
>>>> The actual failure occurs in either an attempted merge or a "log -g";
>>>> the latter was easier to work with... do_logs calls
>>>> get_combined_mergeinfo_changes, which then calls
>>>> svn_fs_get_mergeinfo, eventually calling get_mergeinfo_for_path to
>>>> actually resolve the inherited mergeinfo. It finds some mergeinfo
>>>> higher up the tree, calls svn_mergeinfo_inheritable to filter it
>>>> (which produces an svn_mergeinfo_t with paths that have empty
>>>> rangelists.) That's where the problem appears; the empty rangelists
>>>> eventually get unparsed into a string that gets reparsed by
>>>> svn_parse_mergeinfo, which blows up on the empty values.
>>>>
>>>> The workaround I'm looking at is to simply filter the empty mergeinfo
>>>> in append_to_merged_froms (patch below.) It might be more correct to
>>>> prevent svn_mergeinfo_inheritable from generating them in the first
>>>> place - *is* there a general rule about where producing/discarding
>>>> empty mergeinfo should happen? For example, svn_rangelist_diff talks
>>>> about permitting empty rangelists in their arguments, so they're not
>>>> completely forbidden...
Hi Mark,
Your patch at http://subversion.tigris.org/nonav/issues/showattachment.cgi/949/svn_inheritable_rangelist.patch
was the right approach -- svn_mergeinfo_inheritable should not be
allowed to create syntactically incorrect mergeinfo. I committed it
(with some minor comment additions) in r34426. Nominated it for
backport to 1.5.x as well.
Thanks for the thorough reproduction script and legwork in finding this problem.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn mergeinfo bug with script to reproduce it (finally)! (was Re: empty mergeinfo produced by svn_mergeinfo_inheritable)
Posted by Mark Eichin <ei...@gmail.com>.
I've confirmed this fails on 1.5.1 and 1.5.4; anyone want to run the
script against a trunk install? I'd like to get this filed (well,
what I'd really like is someone to confirm that my fix is credible so
I can justify using it on our local build and get the entire issue off
my plate, but having it be a confirmed Issue would help with that :-)
On Thu, Oct 30, 2008 at 8:09 PM, Mark Eichin <ei...@gmail.com> wrote:
> After a bit more examination of the repository where I'm having this
> problem, the cross-directory aspects finally became clear, and I was
> able to produce a testcase script (attached) that when run with 1.5.x
> produces this error on the final merge command:
>
> --- Merging r11 into '.':
> U other.py
> svn: Mergeinfo for '/trunk/product/frob' maps to an empty revision range
>
> I'm fairly sure that all of the steps are reasonable end-user svn
> operations - the use of --depth is new, but not unusual, and if you
> examine the repository (and particularly the svn:mergeinfo properties)
> at the point of failure, I don't think any of it is wrong or
> inconsistent.
>
> This narrows the problem down to two possibilities:
> * the mergeinfo being constructed *is* inconsistent, and the
> mergeinfo construction side needs to change
> (note that this also leads to a "so how do we repair existing
> repositories" question...)
> * the mergeinfo is fine, but the svn_mergeinfo_inheritable is wrong
> as I theorized in my earlier post (with patch) and it's really enough
> to discard the non-relevant mergeinfo entierly, instead of only
> collapsing the rangelist.
>
> Looking forward to further commentary...
>
>
> On Thu, Oct 30, 2008 at 2:49 PM, Mark Eichin <ei...@gmail.com> wrote:
>> Narrowed down the failure case to one where get_mergeinfo_for_path
>> walks up the parent directories, finds non-inheritable svn:mergeinfo
>> on a directory, stops there (correctly) but then
>> svn_mergeinfo_inheritable filters it down to an (invalid) path with an
>> empty mergelist (instead of no path at all.) Still haven't built a
>> constructive example, though. (I have confirmed that neither
>> svn_mergeinfo_inheritable nor append_to_merged_froms have the relevant
>> filtering needed to avoid it, on trunk...)
>>
>> On Fri, Oct 24, 2008 at 4:41 PM, Mark Eichin <ei...@gmail.com> wrote:
>>> I've been hunting down a "Mergeinfo for '...' maps to an empty
>>> revision range" problem, and after cooking up a bunch of gdb macros
>>> and tracing through
>>> svn_fs_get_mergeinfo I think I have a handle on it. (I haven't
>>> produced a stripped down case yet, I've been working with a copy of
>>> our 17G 86000rev repository, though I'll attempt that next - I (and my
>>> coworkers) wanted a second opinion on the analysis, though.)
>>>
>>> The actual failure occurs in either an attempted merge or a "log -g";
>>> the latter was easier to work with... do_logs calls
>>> get_combined_mergeinfo_changes, which then calls
>>> svn_fs_get_mergeinfo, eventually calling get_mergeinfo_for_path to
>>> actually resolve the inherited mergeinfo. It finds some mergeinfo
>>> higher up the tree, calls svn_mergeinfo_inheritable to filter it
>>> (which produces an svn_mergeinfo_t with paths that have empty
>>> rangelists.) That's where the problem appears; the empty rangelists
>>> eventually get unparsed into a string that gets reparsed by
>>> svn_parse_mergeinfo, which blows up on the empty values.
>>>
>>> The workaround I'm looking at is to simply filter the empty mergeinfo
>>> in append_to_merged_froms (patch below.) It might be more correct to
>>> prevent svn_mergeinfo_inheritable from generating them in the first
>>> place - *is* there a general rule about where producing/discarding
>>> empty mergeinfo should happen? For example, svn_rangelist_diff talks
>>> about permitting empty rangelists in their arguments, so they're not
>>> completely forbidden...
>>>
>>> Index: subversion/libsvn_fs_fs/tree.c
>>> ===================================================================
>>> --- subversion/libsvn_fs_fs/tree.c (revision 87267)
>>> +++ subversion/libsvn_fs_fs/tree.c (working copy)
>>> @@ -3502,6 +3504,10 @@
>>> char *newpath;
>>>
>>> apr_hash_this(hi, &key, NULL, &val);
>>> + if (((apr_array_header_t *)val)->nelts == 0) {
>>> + /* [eichin:20081024T1511-04] don't propagate an empty one! */
>>> + continue;
>>> + }
>>> newpath = svn_path_join((const char *) key, path_piece, pool);
>>> apr_hash_set(*output, newpath, APR_HASH_KEY_STRING,
>>> svn_rangelist_dup((apr_array_header_t *) val, pool));
>>>
>>> (ignore the revision numbers, they're relative to a local repository,
>>> though the function in question appears unchanged in 1.5.4, and 1.5.4
>>> does exhibit the original failure.)
>>>
>>> --
>>> _Mark_ <ei...@metacarta.com> <ei...@gmail.com>
>>>
>>
>>
>>
>> --
>> _Mark_ <ei...@thok.org> <ei...@gmail.com>
>>
>
>
>
> --
> _Mark_ <ei...@thok.org> <ei...@gmail.com>
>
--
_Mark_ <ei...@thok.org> <ei...@gmail.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org