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

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: Thu, 30 Oct 2008 20:09:53 -0400

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>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org

  • application/octet-stream attachment: testcase2
Received on 2008-10-31 01:10:08 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.