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

Re: [trunk/merge tracking] Merge test #17

From: Daniel Rall <dlr_at_collab.net>
Date: 2006-09-23 02:40:22 CEST

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

This is an archived mail posted to the Subversion Dev mailing list.