[merge tracking] Merge test #17

On Fri, 22 Sep 2006, Daniel Rall wrote:
...
> I traced the problem to the following call stack:
>
> svn_wc_adm_retrieve (libsvn_wc/lock.c:962)
> walker_helper (libsvn_wc/entries.c)
> svn_wc_walk_entries2 (libsvn_wc/entries.c)
> svn_client__get_prop_from_wc (libsvn_client/prop_commands.c)
> parse_merge_info (libsvn_client/diff.c)
> get_wc_target_merge_info (libsvn_client/diff.c)
>
> The problem is that a WC directory *underneath* the directory which is
> the target of the merge is missing from the WC. As
> svn_wc_walk_entries2() is strolling through the WC to retrieve the
> value for the "svn:mergeinfo" property from each sub-path, it attempts
> to access the .svn area of a directory which doesn't exist:
>
> From walker_helper():
>
> if (current_entry->kind == svn_node_dir)
> {
> svn_wc_adm_access_t *entry_access;
> SVN_ERR(svn_wc_adm_retrieve(&entry_access, adm_access, entrypath,
> subpool));
> From svn_wc_adm_retrieve():
>
> if (kind == svn_node_none)
> return svn_error_createf(SVN_ERR_WC_NOT_LOCKED, NULL,
> _("Directory '%s' is missing"),
> svn_path_local_style(path, pool));

Incidentally, this produces the test failure:

subversion/libsvn_wc/lock.c:962: (apr_err=155005)
svn: Directory 'svn-test-work/working_copies/merge_tests-17/A/B/F/Q' is missing

> walker_helper()'s attempt to retrieve the svn_wc_adm_access_t for the
> sub-directory without a svn_io_check_path() call might be a reasonable
> optimization. However, because of the generic SVN_ERR_WC_NOT_LOCKED
> error thrown by svn_wc_adm_retrieve(), there doesn't seem to be way
> for walker_helper() to determine that the directory is missing (at
> least, not without subsequently resorting to the path check). David
> James and I were thinking that this scenario should result in an error
> with a different code (e.g. SVN_ERR_WC_PATH_MISSING or something),
> which could be detected further up the call stack.
>
> While this seems like a fairly reasonable approach, I'm not clear what
> other implications it might have for walker_helper(). Any advice
> welcome...

The following patch is an example of how to get merge test #17
working. However, as it changes the core of the WC walker, so likely
isn't an appropriate generic behavior.

On IRC, we discussed some other possible solutions, including:

o Change svn_wc_walk_entries2() to take a flag indicating whether it
should ignore entries for directories which are missing on disk.

o Add an additional callback hook to the svn_wc_entry_callbacks_t
structure for handling errors. This hook could either be generic, and
used for pretty much every error encountered under the call stack of
svn_wc_walk_entries2(), or be specific to missing paths.

Either of these approaches would probably require a new error code for
the code snippet pasted above (from lock.c). Thoughts?

Index: subversion/libsvn_wc/entries.c
===================================================================
--- subversion/libsvn_wc/entries.c (revision 21605)
+++ subversion/libsvn_wc/entries.c (working copy)
@@ -2764,12 +2764,23 @@
       if (current_entry->kind == svn_node_dir)
         {
           svn_wc_adm_access_t *entry_access;
- SVN_ERR(svn_wc_adm_retrieve(&entry_access, adm_access, entrypath,
- subpool));
- SVN_ERR(walker_helper(entrypath, entry_access,
- walk_callbacks, walk_baton,
- show_hidden, cancel_func, cancel_baton,
- subpool));
+ svn_error_t *err = svn_wc_adm_retrieve(&entry_access, adm_access,
+ entrypath, subpool);
+ if (err)
+ {
+ /* Supress errors from missing sub-diretories. */
+ svn_node_kind_t entry_kind;
+ SVN_ERR(svn_io_check_path(entrypath, &entry_kind, pool));
+ if (entry_kind != svn_node_none)
+ return err;
+ }
+ else
+ {
+ SVN_ERR(walker_helper(entrypath, entry_access,
+ walk_callbacks, walk_baton,
+ show_hidden, cancel_func, cancel_baton,
+ subpool));
+ }
         }
 
       svn_pool_clear(subpool);