[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn mergeinfo bug with script to reproduce it (finally)! (was Re: empty mergeinfo produced by svn_mergeinfo_inheritable)

From: Mark Eichin <eichin_at_gmail.com>
Date: Fri, 31 Oct 2008 13:47:38 -0400

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 <eichin_at_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 <eichin_at_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 <eichin_at_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_ <eichin_at_metacarta.com> <eichin_at_gmail.com>
>>>
>>
>>
>>
>> --
>> _Mark_ <eichin_at_thok.org> <eichin_at_gmail.com>
>>
>
>
>
> --
> _Mark_ <eichin_at_thok.org> <eichin_at_gmail.com>
>

-- 
_Mark_ <eichin_at_thok.org> <eichin_at_gmail.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-31 18:47:52 CET

This is an archived mail posted to the Subversion Dev mailing list.