On Thu, Nov 20, 2008 at 04:53:51AM +0100, Neels J. Hofmeyr wrote:
> Julian Foad wrote:
> >
> > We need to document that "svn_wc_is_wc_root()" doesn't do what we were
> > thinking it would do. Done in r34211.
> >
> > We need to abstract out this chunk of code, give it a name and a
> > description. svn_wc_is_really_really_wc_root().
>
> I've got a patch here but would like it reviewed. Should the new function be
> public API?
>
> ~Neels
> Move some code around svn_wc_is_wc_root() in resolved.c out into a separate
> function, for code readability.
>
> Because it is an extension of svn_wc_is_wc_root() and might be useful
> elsewhere, add it next to the definition of svn_wc_is_wc_root() and
> publish it in the private headers, as svn_wc__strictly_is_wc_root().
>
> * subversion/include/private/svn_wc_private.h
> (svn_wc__strictly_is_wc_root): New function declaration.
>
> * subversion/libsvn_client/resolved.c
> (svn_client_resolve):
> Move some code out to svn_wc__strictly_is_wc_root().
>
> * subversion/libsvn_wc/update_editor.c
> (svn_wc__strictly_is_wc_root):
> New function from code that was in svn_client_resolve().
>
> * subversion/include/svn_wc.h
> (svn_wc_is_wc_root): Comment.
>
>
> --This line, and those below, will be ignored--
No they won't be ignored, they'll be reviewed :P
> Index: subversion/include/private/svn_wc_private.h
> ===================================================================
> --- subversion/include/private/svn_wc_private.h (revision 34275)
> +++ subversion/include/private/svn_wc_private.h (working copy)
> @@ -218,6 +218,16 @@ svn_wc__read_tree_conflicts(apr_array_he
> const char *dir_path,
> apr_pool_t *pool);
>
> +/** Like svn_wc_is_wc_root(), but it doesn't consider switched subdirs or
> + * deleted entries as working copy roots.
> + *
> + * @since New in 1.6.*/
> +svn_error_t *
> +svn_wc__strictly_is_wc_root(svn_boolean_t *wc_root,
> + const char *path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 34275)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -3769,7 +3769,8 @@ svn_wc_crawl_revisions(const char *path,
> * @c FALSE otherwise. Here, @a path is a "working copy root" if its parent
> * directory is not a WC or if its parent directory's repository URL is not
> * the parent of its own repository URL. Thus, a switched subtree is
> - * considered to be a working copy root.
> + * considered to be a working copy root. Also, a deleted tree-conflict
> + * victim is considered a "working copy root" because it has no URL.
Oh oh oh oh... doesn't this nicely show that the current concept of
"working copy root" is absolutely ill-defined?
I'd say wc-ng will make this problem disappear, because it will
save meta data in either the working copy root only, or a centralised
meta data store. So for now I think it's OK to simply document the fact
that this function is broken.
Patch looks good.
Stefan
> *
> * If @a path is not found, return the error @c SVN_ERR_ENTRY_NOT_FOUND.
> *
> Index: subversion/libsvn_client/resolved.c
> ===================================================================
> --- subversion/libsvn_client/resolved.c (revision 34276)
> +++ subversion/libsvn_client/resolved.c (working copy)
> @@ -43,7 +43,6 @@ svn_client_resolve(const char *path,
> svn_wc_adm_access_t *adm_access;
> int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> svn_boolean_t wc_root;
> - const svn_wc_entry_t *entry;
>
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL,
> path,
> @@ -53,45 +52,10 @@ svn_client_resolve(const char *path,
> pool));
>
> /* Make sure we do not end up looking for tree conflict info
> - * above the working copy root. */
> - SVN_ERR(svn_wc_is_wc_root(&wc_root, path, adm_access, pool));
> - if (wc_root)
> - {
> - /* Check whether this is a switched subtree or an absent item.
> - * Switched subtrees are considered working copy roots by
> - * svn_wc_is_wc_root(). But it's OK to check for tree conflict
> - * info in the parent of a switched subtree, because the
> - * subtree itself might be a tree conflict victim. */
> - SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, pool));
> -
> - /* If this has no entry, it can't possibly be a switched subdir.
> - * It can't be a WC root either, for that matter.*/
> - if (entry == NULL)
> - wc_root = FALSE;
> - else
> - if (entry->kind == svn_node_dir)
> - {
> - svn_error_t *err;
> - svn_boolean_t switched;
> -
> - err = svn_wc__path_switched(path, &switched, entry, pool);
> -
> - if (err && (err->apr_err == SVN_ERR_ENTRY_MISSING_URL))
> - {
> - /* This is e.g. a locally deleted dir. It has an entry but
> - * no repository URL. It cannot be a WC root. */
> - svn_error_clear(err);
> - wc_root = FALSE;
> - }
> - else
> - {
> - SVN_ERR(err);
> - /* The query for a switched dir succeeded. If switched,
> - * don't consider this a WC root. */
> - wc_root = switched ? FALSE : TRUE;
> - }
> - }
> - }
> + * above the working copy root. It's OK to check for tree conflict
> + * info in the parent of a *switched* subtree, because the
> + * subtree itself might be a tree conflict victim. */
> + SVN_ERR(svn_wc__strictly_is_wc_root(&wc_root, path, adm_access, pool));
>
> if (! wc_root) /* but possibly a switched subdir */
> {
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 34275)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -4603,6 +4603,56 @@ svn_wc_is_wc_root(svn_boolean_t *wc_root
> }
>
>
> +svn_error_t*
> +svn_wc__strictly_is_wc_root(svn_boolean_t *wc_root,
> + const char *path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> +{
> + SVN_ERR(svn_wc_is_wc_root(wc_root, path, adm_access, pool));
> +
> + if (*wc_root)
> + {
> + const svn_wc_entry_t *entry;
> +
> + /* Check whether this is a switched subtree or an absent item.
> + * Switched subtrees are considered working copy roots by
> + * svn_wc_is_wc_root(). */
> + SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, pool));
> +
> + /* If this has no entry, it can't possibly be a switched subdir.
> + * It can't be a WC root either, for that matter.*/
> + if (entry == NULL)
> + *wc_root = FALSE;
> + else
> + if (entry->kind == svn_node_dir)
> + {
> + svn_error_t *err;
> + svn_boolean_t switched;
> +
> + err = svn_wc__path_switched(path, &switched, entry, pool);
> +
> + if (err && (err->apr_err == SVN_ERR_ENTRY_MISSING_URL))
> + {
> + /* This is e.g. a locally deleted dir. It has an entry but
> + * no repository URL. It cannot be a WC root. */
> + svn_error_clear(err);
> + *wc_root = FALSE;
> + }
> + else
> + {
> + SVN_ERR(err);
> + /* The query for a switched dir succeeded. If switched,
> + * don't consider this a WC root. */
> + *wc_root = switched ? FALSE : TRUE;
> + }
> + }
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
> svn_error_t *
> svn_wc_get_actual_target(const char *path,
> const char **anchor,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-20 12:02:27 CET