You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2010/03/16 14:07:41 UTC
svn commit: r923720 - in /subversion/trunk/subversion/libsvn_client:
client.h diff.c repos_diff.c
Author: philip
Date: Tue Mar 16 13:07:40 2010
New Revision: 923720
URL: http://svn.apache.org/viewvc?rev=923720&view=rev
Log:
Remove access batons from repository diff.
* subversion/libsvn_client/repos_diff.c
(struct edit_baton): Replace access baton member with wc context.
(get_dir_abspath): Replace access baton parameter with wc context,
return a different, documented, error.
(get_parent_dir_abspath): Replace access baton parameter with wc context.
(delete_entry): Use wc context not access baton.
(close_file, close_directory): Use wc context not access baton, check for
a different error.
(svn_client__get_diff_editor): Store wc context instead of access baton.
* subversion/libsvn_client/client.h
(svn_client__get_diff_editor): Tweak doc string.
* subversion/libsvn_client/diff.c
(diff_repos_repso): Pass NULL wc context.
Modified:
subversion/trunk/subversion/libsvn_client/client.h
subversion/trunk/subversion/libsvn_client/diff.c
subversion/trunk/subversion/libsvn_client/repos_diff.c
Modified: subversion/trunk/subversion/libsvn_client/client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=923720&r1=923719&r2=923720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/client.h (original)
+++ subversion/trunk/subversion/libsvn_client/client.h Tue Mar 16 13:07:40 2010
@@ -655,7 +655,8 @@ svn_client__switch_internal(svn_revnum_t
TARGET is a working-copy path, the base of the hierarchy to be
compared. It corresponds to the URL opened in RA_SESSION below.
- WC_CTX is a context for the working copy.
+ WC_CTX is a context for the working copy and should be NULL for
+ operations that do not involve a working copy.
DIFF_CMD/DIFF_CMD_BATON represent the callback and callback argument that
implement the file comparison function
Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=923720&r1=923719&r2=923720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Tue Mar 16 13:07:40 2010
@@ -1281,7 +1281,7 @@ diff_repos_repos(const struct diff_param
Otherwise, we just use "". */
SVN_ERR(svn_client__get_diff_editor
(drr.base_path ? drr.base_path : "",
- ctx->wc_ctx, callbacks, callback_baton, diff_param->depth,
+ NULL, callbacks, callback_baton, diff_param->depth,
FALSE /* doesn't matter for diff */, extra_ra_session, drr.rev1,
NULL /* no notify_func */, NULL /* no notify_baton */,
ctx->cancel_func, ctx->cancel_baton,
Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=923720&r1=923719&r2=923720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/repos_diff.c Tue Mar 16 13:07:40 2010
@@ -51,7 +51,7 @@ struct edit_baton {
const char *target;
/* ADM_ACCESS is an access baton that includes the TARGET directory */
- svn_wc_adm_access_t *adm_access;
+ svn_wc_context_t *wc_ctx;
/* The callback and calback argument that implement the file comparison
function */
@@ -321,56 +321,65 @@ get_dirprops_from_ra(struct dir_baton *b
}
-/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory PATH by
- searching the access baton set of ADM_ACCESS. If ADM_ACCESS is NULL then
- *LOCAL_DIR_ABSPATH will be NULL. If LENIENT is TRUE then failure to find
- an access baton will not return an error but will set *LOCAL_DIR_ABSPATH to
- NULL instead. */
+/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory
+ PATH if PATH is a versioned directory. If PATH is not a versioned
+ directory and LENIENT is FALSE then return an error
+ SVN_ERR_WC_NOT_WORKING_COPY. If LENIENT is TRUE then failure to
+ find an access baton will not return an error but will set
+ *LOCAL_DIR_ABSPATH to NULL instead.
+
+ This rather odd interface was originally designed around searching
+ an access baton set. */
static svn_error_t *
get_dir_abspath(const char **local_dir_abspath,
- svn_wc_adm_access_t *adm_access,
+ svn_wc_context_t *wc_ctx,
const char *path,
svn_boolean_t lenient,
apr_pool_t *pool)
{
*local_dir_abspath = NULL;
- if (adm_access)
+ if (wc_ctx)
{
- svn_wc_adm_access_t *path_access;
- svn_error_t *err = svn_wc_adm_retrieve(&path_access, adm_access, path,
- pool);
+ svn_node_kind_t kind;
+ svn_error_t *err;
+ const char *local_abspath;
+ SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
+ err = svn_wc__node_get_kind(&kind, wc_ctx, local_abspath, FALSE, pool);
if (err)
{
- if (! lenient)
+ if (lenient)
+ kind = svn_node_none;
+ else
return svn_error_return(err);
- svn_error_clear(err);
}
- else if (path_access != NULL)
- SVN_ERR(svn_dirent_get_absolute(local_dir_abspath,
- svn_wc_adm_access_path(path_access),
- pool));
-
+ svn_error_clear(err);
+ if (kind == svn_node_dir)
+ *local_dir_abspath = local_abspath;
+ else if (!lenient)
+ return svn_error_createf(SVN_ERR_WC_NOT_WORKING_COPY, NULL,
+ "'%s' is not a versioned directory",
+ svn_dirent_local_style(local_abspath, pool));
}
return SVN_NO_ERROR;
}
-/* Like get_path_access except the returned access baton, in
- *PARENT_ACCESS, is for the parent of PATH rather than for PATH
- itself. */
+/* Like get_path_access except the returned path, in
+ *LOCAL_PARENT_DIR_ABSPATH, is for the parent of PATH rather than
+ for PATH itself. */
static svn_error_t *
get_parent_dir_abspath(const char **local_parent_dir_abspath,
- svn_wc_adm_access_t *adm_access,
+ svn_wc_context_t *wc_ctx,
const char *path,
svn_boolean_t lenient,
- apr_pool_t *pool)
+ apr_pool_t *pool)
{
- if (! adm_access)
+ if (!wc_ctx)
*local_parent_dir_abspath = NULL; /* Avoid messing around with paths */
else
{
- SVN_ERR(get_dir_abspath(local_parent_dir_abspath, adm_access,
+ SVN_ERR(get_dir_abspath(local_parent_dir_abspath, wc_ctx,
svn_dirent_dirname(path, pool),
lenient, pool));
}
@@ -450,9 +459,9 @@ delete_entry(const char *path,
/* We need to know if this is a directory or a file */
SVN_ERR(svn_ra_check_path(eb->ra_session, path, eb->revision, &kind, pool));
- SVN_ERR(get_dir_abspath(&local_dir_abspath, eb->adm_access, pb->wcpath,
+ SVN_ERR(get_dir_abspath(&local_dir_abspath, eb->wc_ctx, pb->wcpath,
TRUE, pool));
- if ((! eb->adm_access) || local_dir_abspath)
+ if ((! eb->wc_ctx) || local_dir_abspath)
{
switch (kind)
{
@@ -554,7 +563,7 @@ add_directory(const char *path,
}
- SVN_ERR(get_dir_abspath(&local_dir_abspath, eb->adm_access, pb->wcpath, TRUE,
+ SVN_ERR(get_dir_abspath(&local_dir_abspath, eb->wc_ctx, pb->wcpath, TRUE,
pool));
SVN_ERR(eb->diff_callbacks->dir_added
@@ -642,7 +651,7 @@ open_directory(const char *path,
SVN_ERR(get_dirprops_from_ra(b, base_revision));
- SVN_ERR(get_dir_abspath(&local_dir_abspath, eb->adm_access, pb->wcpath, TRUE,
+ SVN_ERR(get_dir_abspath(&local_dir_abspath, eb->wc_ctx, pb->wcpath, TRUE,
pool));
SVN_ERR(eb->diff_callbacks->dir_opened
@@ -799,10 +808,10 @@ close_file(void *file_baton,
if (b->skip)
return SVN_NO_ERROR;
- err = get_parent_dir_abspath(&local_dir_abspath, eb->adm_access,
+ err = get_parent_dir_abspath(&local_dir_abspath, eb->wc_ctx,
b->wcpath, eb->dry_run, b->pool);
- if (err && err->apr_err == SVN_ERR_WC_NOT_LOCKED)
+ if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
{
/* ### maybe try to stat the local b->wcpath? */
/* If the file path doesn't exist, then send a 'skipped' notification. */
@@ -934,10 +943,10 @@ close_directory(void *dir_baton,
if (eb->dry_run)
svn_hash__clear(svn_client__dry_run_deletions(eb->diff_cmd_baton), pool);
- err = get_dir_abspath(&local_dir_abspath, eb->adm_access, b->wcpath,
+ err = get_dir_abspath(&local_dir_abspath, eb->wc_ctx, b->wcpath,
eb->dry_run, b->pool);
- if (err && err->apr_err == SVN_ERR_WC_NOT_LOCKED)
+ if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
{
/* ### maybe try to stat the local b->wcpath? */
/* If the path doesn't exist, then send a 'skipped' notification.
@@ -1161,8 +1170,7 @@ svn_client__get_diff_editor(const char *
SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, pool));
eb->target = target;
- SVN_ERR(svn_wc__adm_retrieve_from_context(&(eb->adm_access), wc_ctx,
- target_abspath, pool));
+ eb->wc_ctx = wc_ctx;
eb->diff_callbacks = diff_callbacks;
eb->diff_cmd_baton = diff_cmd_baton;
eb->dry_run = dry_run;
Re: svn commit: r923720 - in /subversion/trunk/subversion/libsvn_client: client.h diff.c repos_diff.c
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
>> +/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory
>> + PATH if PATH is a versioned directory. If PATH is not a versioned
>> + directory and LENIENT is FALSE then return an error
>> + SVN_ERR_WC_NOT_WORKING_COPY. If LENIENT is TRUE then failure to
>> + find an access baton will not return an error but will set
>> + *LOCAL_DIR_ABSPATH to NULL instead.
>> +
>> + This rather odd interface was originally designed around searching
>> + an access baton set. */
>
> "failure to find an access baton" ?? That certainly doesn't make any
> sense.
Oops. I wrote the doc string and reverted the patch several times,
obviously I got a bit sloppy with the final version.
> The docstring should also mention that if WC_CTX is NULL, then
> nothing will ever be returned (*LOCAL_DIR_ABSPATH is set to NULL).
> IMO, this function shouldn't even be called in that situation. Callers
> should not even be trying functionality which doesn't make sense (ie.
> push the WC_CTX check out).
It's a very weird function now because in 1.6 it was a wrapper around
svn_wc_adm_retrieve and the interface kind of matches that function.
I'll look at whether checking WC_CTX in the caller is better.
--
Philip
Re: svn commit: r923720 - in /subversion/trunk/subversion/libsvn_client:
client.h diff.c repos_diff.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 16, 2010 at 09:07, <ph...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_client/client.h Tue Mar 16 13:07:40 2010
> @@ -655,7 +655,8 @@ svn_client__switch_internal(svn_revnum_t
> TARGET is a working-copy path, the base of the hierarchy to be
> compared. It corresponds to the URL opened in RA_SESSION below.
>
> - WC_CTX is a context for the working copy.
> + WC_CTX is a context for the working copy and should be NULL for
> + operations that do not involve a working copy.
This kind of comment should be added to all functions where this is
allowed. Normally wc_ctx!=NULL throughout libsvn_client, so it is a
very different departure to allow this.
>...
> +++ subversion/trunk/subversion/libsvn_client/repos_diff.c Tue Mar 16 13:07:40 2010
> @@ -51,7 +51,7 @@ struct edit_baton {
> const char *target;
>
> /* ADM_ACCESS is an access baton that includes the TARGET directory */
> - svn_wc_adm_access_t *adm_access;
> + svn_wc_context_t *wc_ctx;
The comment should be adjusted to reference WC_CTX, and to note the
NULL possibility.
> @@ -321,56 +321,65 @@ get_dirprops_from_ra(struct dir_baton *b
> }
>
>
> -/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory PATH by
> - searching the access baton set of ADM_ACCESS. If ADM_ACCESS is NULL then
> - *LOCAL_DIR_ABSPATH will be NULL. If LENIENT is TRUE then failure to find
> - an access baton will not return an error but will set *LOCAL_DIR_ABSPATH to
> - NULL instead. */
> +/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory
> + PATH if PATH is a versioned directory. If PATH is not a versioned
> + directory and LENIENT is FALSE then return an error
> + SVN_ERR_WC_NOT_WORKING_COPY. If LENIENT is TRUE then failure to
> + find an access baton will not return an error but will set
> + *LOCAL_DIR_ABSPATH to NULL instead.
> +
> + This rather odd interface was originally designed around searching
> + an access baton set. */
"failure to find an access baton" ?? That certainly doesn't make any
sense. The docstring should also mention that if WC_CTX is NULL, then
nothing will ever be returned (*LOCAL_DIR_ABSPATH is set to NULL).
IMO, this function shouldn't even be called in that situation. Callers
should not even be trying functionality which doesn't make sense (ie.
push the WC_CTX check out).
>...
> -/* Like get_path_access except the returned access baton, in
> - *PARENT_ACCESS, is for the parent of PATH rather than for PATH
> - itself. */
> +/* Like get_path_access except the returned path, in
> + *LOCAL_PARENT_DIR_ABSPATH, is for the parent of PATH rather than
> + for PATH itself. */
Add comment about WC_CTX.
>...
Cheers,
-g