On Fri, 22 Sep 2006, Daniel Rall wrote:
> 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.
DannyB and EH confirmed this type of optimization has made a
significant performance difference, especially for WCs of the gcc
class.
> > 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.
I made use of SVN_ERR_WC_PATH_NOT_FOUND in r21615 (on trunk) to
indicate the root cause of the errors coming out of this WC locking
API, and ported this change to the merge-tracking branch.
...
> 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.
I went with this approach, which I broke into two patches. The first
patch I'd like to commit to trunk:
[[[
Add a generic error handler to routine to the WC walker API, adding
the ability to modify or squelch errors which occur while walking the
WC tree.
* subversion/include/svn_wc.h
(svn_wc_entry_callbacks2_t): Rev the WC walker callbacks, which
adding a handle_error() callback hook.
(svn_wc_walk_entries3): Rev the WC walker API, switching to
svn_wc_entry_callbacks2_t.
(svn_wc_walk_entries2): Deprecate the previous API.
* subversion/libsvn_wc/entries.c
(walker_helper): Accept a svn_wc_entry_callbacks2_t for the
WALK_CALLBACKS parameter, and use its handle_error() API
throughout. When attempting recursion into entry sub-directories,
skip them if a lock cannot be procured.
(svn_wc_walk_entries3): Rev the svn_wc_walk_entries2() API, and use
the handle_error() API.
(svn_wc_walk_entries2): Replace with compat code.
(walker_default_error_handler): A new compat error handler callback.
]]]
An alternate approach which is somewhat similar to this would be to
define a much more specific callback API to use when error likes
SVN_ERR_WC_PATH_NOT_FOUND are encountered inside
svn_wc_walk_entries2()/walker_helper(). The consensus on IRC was that
a more generic API would be preferrable.
The second patch depends upon the first. If no one has problems with
the first patch, I'll port the first patch to the merge-tracking
branch, and commit the second patch there as well.
[[[
On the merge-tracking branch: Ignore missing paths when collecting the
values for a property.
* subversion/libsvn_client/prop_commands.c
(wc_walker_error_handler): Add error handler to squelch ERR by
returning SVN_NO_ERROR if ERR is casued by a missing path.
(svn_client__get_prop_from_wc): Use a svn_wc_entry_callbacks2_t data
structure carrying a wc_walker_error_handler() callback to invoke
svn_wc_walk_entries3().
]]]
I'd follow these two patches up by adding some type of error chain
interrogation API to trunk to cleanup the FIXME in
wc_walker_error_handler(). Alternately, we could just call
svn_io_check_path() (along the lines of the patch I posted in the
previous message in this thread).
Thoughts?
Thanks, Dan
- application/pgp-signature attachment: stored
Received on Sat Sep 23 02:42:30 2006