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>
---------------------------------------------------------------------
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-30 19:49:30 CET