Re: svn commit: r25049 - in trunk/subversion: include libsvn_client svn tests/cmdline

On 5/16/07, hwright@tigris.org <hwright@tigris.org> wrote:

Very nice feature, Hyrum! A couple questions:

> +/* Go up the directory tree, looking for a versioned directory. If found,
> + add all the intermediate directories. Otherwise, return SVN_ERR_XXX */
> +static svn_error_t *
> +add_parent_dirs(const char *path,
> + svn_wc_adm_access_t **parent_access,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + svn_wc_adm_access_t *adm_access;
> + svn_error_t *err;
> +
> + err = svn_wc_adm_open3(&adm_access, NULL, path, TRUE, 0,
> + ctx->cancel_func, ctx->cancel_baton, pool);
> +
> + if (err && err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> + {
> + if (svn_dirent_is_root(path, strlen(path)))
> + {
> + svn_error_clear(err);
> +
> + return svn_error_create
> + (SVN_ERR_CLIENT_NO_VERSIONED_PARENT, NULL, NULL);
> + }
> + else
> + {
> + const char *parent_path = svn_path_dirname(path, pool);
> +
> + svn_error_clear(err);
> + SVN_ERR(add_parent_dirs(parent_path, &adm_access, ctx, pool));
> + SVN_ERR(svn_wc_adm_retrieve(&adm_access, adm_access, parent_path,
> + pool));
> + SVN_ERR(svn_wc_add2(path, adm_access, NULL, SVN_INVALID_REVNUM,
> + ctx->cancel_func, ctx->cancel_baton,
> + ctx->notify_func2, ctx->notify_baton2, pool));
> + }
> + }
> + else if (err)
> + {
> + return err;
> + }
> +
> + if (parent_access)
> + *parent_access = adm_access;
> +
> + return SVN_NO_ERROR;
> +}

So it's recursive, not iterative, but it's still essentially an
unbounded iteration... should there be some pool clearing? I guess
it's a bit tricky since each call is passing a structure to the next
one. And it's bounded by the depth of the filesystem, which hopefully
shouldn't be enormous. And you're calling from a subpool anyway. So
maybe it doesn't matter.

> --- trunk/subversion/svn/main.c (original)
> +++ trunk/subversion/svn/main.c Wed May 16 15:56:38 2007
> @@ -203,7 +203,7 @@
> " "
> "using the name=value format")},
> {"parents", svn_cl__parents_opt, 0,
> - N_("make intermediate directories")},
> + N_("make and/or add intermediate directories")},
> {0, 0, 0, 0}
> };

I don't really like the and/or in this: if I understand correctly, for
some commands --parents makes the intermediate directories, and for
some commands it adds them, right?

Our infrastructure does support per-option help descriptions: see -F
in propset, for example. Maybe have the default say "make" and
override add's to say "add"?

--dave

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org