corrupted mergeinfo when do_directory_merge adds a file

libsvn_client/merge.c, do_directory_merge, in the "if
(notify_b->added_paths)" block that handles propagating inheritable
mergeinfo down to newly added file, you have this block of code:

                  const char *common_ancestor_path =
                    svn_path_get_longest_ancestor(added_path,
                                                  target_merge_path->path,
                                                  iterpool);
                  const char *relative_added_path =
                    added_path + strlen(common_ancestor_path) + 1;
                  SVN_ERR(svn_wc__entry_versioned(&entry, added_path,
                                                  adm_access, FALSE,
                                                  iterpool));

(lines 6337-6345, on trunk, as of r34547, but the code appears
unchanged from 1.5 which I'm actually working with.)

The problem I'm seeing is that
   added_path == "TestFoo.py"
   target_merge_path->path == ""
thus
   common_ancestor_path == ""
thus
   strlen(common_ancestor_path) == 0
thus
   relative_added_path == "estFoo.py"

The setup for this is a merge -c 88093 of a single revision which
creates TestFoo.py on trunk, into a directory which is a checkout of
an earlier branch; the directory has both inheritable and
non-inheritable mergeinfo on it already. relative_added_path gets
joined with mergeinfo_path and applied to the newly created
TestFoo.py, producing an entirely garbage mergeinfo record of
"/trunk/tests/estFoo.py:88093"

My first thought is that it simply needs to check if
common_ancestor_path is empty and not add the +1 in that case. That
would at least make the path not meaningless. Some variant on this:

Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 34552)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -6339,7 +6339,7 @@
                                                   target_merge_path->path,
                                                   iterpool);
                   const char *relative_added_path =
- added_path + strlen(common_ancestor_path) + 1;
+ added_path + strlen(common_ancestor_path) +
(common_ancestor_path[0] ? 1 : 0);
                   SVN_ERR(svn_wc__entry_versioned(&entry, added_path,
                                                   adm_access, FALSE,
                                                   iterpool));

would cover that (or whatever your house style suggests for testing
that common_ancestor_path is non-empty.)

However, I'm not sure that the mergeinfo applied is right either -
since with the correct path, since it's using the mergeinfo from the
current directory *after* applying this merge, it just describes the
"natural history" of the change anyway, right? (Or does that count as
"redundant but not wrong" and will get optimized out in the future?)

(Should have a reduced demonstration script soon, but since the code
is wrong by inspection, I figured I'd give people (especially Paul
Burba :-) a chance to look at the problem...)

-- 
_Mark_ <eichin_at_thok.org> <eichin_at_gmail.com>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=979211