Hi,
D.J. Heap wrote:
> Change calls from the old svn_wc_adm_* functions to the new
> svn_wc_adm_*_depth functions and mark the old functions as deprecated.
Thanks for doing this. One of the things I've been meaning to do is
audit all uses of these functions to check if we're locking too much
anywhere else. Your patch made this easy.
Your patch looks fine (it's effectively a no-op), my comments below
are suggesting changes that can be done later in different patches.
> Index: subversion/libsvn_client/status.c
> ===================================================================
> --- subversion/libsvn_client/status.c (revision 8957)
> +++ subversion/libsvn_client/status.c (working copy)
> @@ -155,8 +155,8 @@
> /* Using pool cleanup to close it. This needs to be recursive so that
> auth data can be stored. */
> if (strlen (anchor) != strlen (path))
> - SVN_ERR (svn_wc_adm_open (&anchor_access, NULL, anchor, FALSE,
> - TRUE, pool));
> + SVN_ERR (svn_wc_adm_open_depth (&anchor_access, NULL, anchor, FALSE,
> + -1, pool));
Does this *really* need to be recursive? The comment says so; I don't
know enough about the auth mechanism to be sure.
If it does need to be recursive, did my previous patch introduce a
subtle bug when anchor==path and update=TRUE?
> Index: subversion/libsvn_client/prop_commands.c
> ===================================================================
> @@ -156,7 +156,8 @@
> - SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target, TRUE, TRUE, pool));
> + SVN_ERR (svn_wc_adm_probe_open_depth (&adm_access, NULL, target, TRUE,
> + -1, pool));
> @@ -554,8 +556,8 @@
> - SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target, FALSE, TRUE,
> - pool));
> + SVN_ERR (svn_wc_adm_probe_open_depth (&adm_access, NULL, target,
> + FALSE, -1, pool));
> @@ -911,8 +913,8 @@
> - SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target, FALSE, TRUE,
> - pool));
> + SVN_ERR (svn_wc_adm_probe_open_depth (&adm_access, NULL, target,
> + FALSE, -1, pool));
Why do setting, getting and listing properties require a recursive lock?
Surely this can be non-recursive?
> Index: subversion/libsvn_client/update.c
> ===================================================================
> --- subversion/libsvn_client/update.c (revision 8957)
> +++ subversion/libsvn_client/update.c (working copy)
> @@ -74,7 +74,8 @@
> /* Use PATH to get the update's anchor and targets and get a write lock */
> SVN_ERR (svn_wc_get_actual_target (path, &anchor, &target, pool));
> - SVN_ERR (svn_wc_adm_open (&adm_access, NULL, anchor, TRUE, TRUE, pool));
> + SVN_ERR (svn_wc_adm_open_depth (&adm_access, NULL, anchor, TRUE, -1,
> + pool));
Non-recursive updates require a recursive lock? Is this related to the
"auth data can be stored" comment in status.c?
Kind regards,
Jon Foster
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 11 00:05:08 2004