[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: Paul Burba <ptburba_at_gmail.com>
Date: Tue, 25 Nov 2008 14:32:52 -0500

On Fri, Oct 31, 2008 at 12:47 PM, Mark Eichin <eichin_at_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 <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...

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_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-25 20:33:02 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.