Attached is a patch I am working on for issue #1093 with sussman. I
have two things that I am not sure of, one that is technical, the
other which is more of a bikeshed quality.
First, the technical question, I found what I think is a URI escaping
bug in libsvn_client/ra.c, but am not that familiar with how URI
escaping is handled in Subversion to feel confident in my fix. Could
someone check this over?
Second, I factorized some code from several of the client commands,
but there didn't seem an appropriate file in libsvn_client to put it
in and I wasn't sure if my function naming convention gelled with the
rest of the code. My questions: Is the new file 'util.c' appropriate?
Is the name svn_client__get_ra_from_path_or_url too verbose or
otherwise not good?
Thanks,
Josh
================================================
Issue #1093: Make list, propget, proplist, and blame follow copy
history in the same way cat does currently. Have all of these
subcommands use a new helper function that does the initial work for
any subcommand that takes a single working copy path or a single URL.
Additionally, fix a URI escaping problem in
svn_client__repos_locations that causes it to fail on filenames with
spaces.
* subversion/libsvn_client/client.h
* subversion/libsvn_client/util.c: New
(svn_client__get_ra_for_path_or_url): New utility function that sets
up a RA plugin for the subcommands that take a single working copy
path or URL.
* subversion/libsvn_client/cat.c
* subversion/libsvn_client/ls.c
* subversion/libsvn_client/blame.c
* subversion/libsvn_client/prop_commands.c
(svn_client_cat, svn_client_ls, svn_client_blame,
svn_client_propget, svn_client_proplist): Use the new
svn_client__get_ra_for_path_or_url to get an RA connection and URL
for the given command line arguments.
* subversion/libsvn_client/ra.c
(svn_client__repos_locations): The log handler expects to have a
non-escaped path in lrb.last_path, so decode the discovered URL.
Index: subversion/libsvn_client/util.c
===================================================================
--- subversion/libsvn_client/util.c (revision 0)
+++ subversion/libsvn_client/util.c (revision 0)
@@ -0,0 +1,122 @@
+/*
+ * util.c: utility functions for client commands
+ *
+ * ====================================================================
+ * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+
+
+#include <apr_pools.h>
+
+#include "svn_error.h"
+#include "svn_string.h"
+#include "svn_ra.h"
+#include "svn_wc.h"
+#include "svn_client.h"
+#include "svn_path.h"
+#include "svn_pools.h"
+#include "client.h"
+
+
+
+
+svn_error_t *
+svn_client__get_ra_for_path_or_url (svn_ra_plugin_t **ra_lib_p,
+ void **session_p,
+ svn_revnum_t *rev_p,
+ const char **url_p,
+ const char *path_or_url,
+ const svn_opt_revision_t *revision,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ const char *initial_url, *url;
+ void *ra_baton, *session;
+ svn_ra_plugin_t *ra_lib;
+ apr_pool_t *ra_subpool;
+ const svn_opt_revision_t *good_rev;
+ svn_revnum_t rev;
+
+ /* Open an RA session to the incoming URL. */
+ SVN_ERR (svn_client_url_from_path (&initial_url, path_or_url, pool));
+ if (! initial_url)
+ return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
+ "'%s' has no URL", path_or_url);
+
+ SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
+ SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, initial_url, pool));
+
+ ra_subpool = svn_pool_create (pool);
+ SVN_ERR (svn_client__open_ra_session (&session, ra_lib, initial_url,
+ NULL, NULL, NULL, FALSE, FALSE,
+ ctx, ra_subpool));
+
+ if (svn_path_is_url (path_or_url))
+ {
+ /* If an explicit URL was passed in, just use it. */
+ good_rev = revision;
+ url = initial_url;
+ }
+ else
+ {
+ /* For a working copy path, don't blindly use its initial_url
+ from the entries file. Run the history function to get the
+ object's (possibly different) url in REVISION. */
+ svn_opt_revision_t base_rev, dead_end_rev, start_rev;
+ svn_opt_revision_t *ignored_rev, *new_rev;
+ const char *ignored_url;
+
+ dead_end_rev.kind = svn_opt_revision_unspecified;
+ base_rev.kind = svn_opt_revision_base;
+
+ if (revision->kind == svn_opt_revision_unspecified)
+ start_rev.kind = svn_opt_revision_base;
+ else
+ start_rev = *revision;
+
+ SVN_ERR (svn_client__repos_locations (&url, &new_rev,
+ &ignored_url, &ignored_rev,
+ /* peg coords are path@BASE: */
+ path_or_url, &base_rev,
+ /* search range: */
+ &start_rev, &dead_end_rev,
+ ra_lib, session,
+ ctx, pool));
+ good_rev = new_rev;
+
+ /* If 'url' turns out to be different than 'initial_url', then we
+ need to open a new RA session to it. */
+ if (strcmp (url, initial_url) != 0)
+ {
+ svn_pool_clear (ra_subpool);
+ SVN_ERR (svn_client__open_ra_session (&session, ra_lib, url,
+ NULL, NULL, NULL, FALSE, FALSE,
+ ctx, ra_subpool));
+ }
+ }
+
+ /* Resolve good_rev into a real revnum. */
+ SVN_ERR (svn_client__get_revision_number (&rev, ra_lib, session,
+ good_rev, url, pool));
+ if (! SVN_IS_VALID_REVNUM (rev))
+ SVN_ERR (ra_lib->get_latest_revnum (session, &rev, pool));
+
+ *ra_lib_p = ra_lib;
+ *session_p = session;
+ *rev_p = rev;
+ *url_p = url;
+
+ return SVN_NO_ERROR;
+}
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 9612)
+++ subversion/libsvn_client/client.h (working copy)
@@ -88,6 +88,31 @@
svn_client__revision_is_local (const svn_opt_revision_t *revision);
+/* Given PATH_OR_URL, which contains either a working copy path or an
+ absolute url and a revision REVISION, create an RA connection to
+ that object as it exists in that revision, following copy history
+ if necessary.
+
+ The resulting ra_plugin is stored in *RA_LIB_P along with its
+ session baton in *SESSION_P. The actual revision number of the
+ object is stored in *REV_P and the final resulting url is stored in
+ *URL_P.
+
+ Use authentication baton cached in CTX to authenticate against the
+ repository.
+
+ Use POOL for all allocations. */
+svn_error_t *
+svn_client__get_ra_for_path_or_url (svn_ra_plugin_t **ra_lib_p,
+ void **session_p,
+ svn_revnum_t *rev_p,
+ const char **url_p,
+ const char *path_or_url,
+ const svn_opt_revision_t *revision,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+
/* Given the CHANGED_PATHS and REVISION from an instance of a
svn_log_message_receiver_t function, determine at which location
PATH may be expected in the next log message, and set *PREV_PATH_P
Index: subversion/libsvn_client/prop_commands.c
===================================================================
--- subversion/libsvn_client/prop_commands.c (revision 9609)
+++ subversion/libsvn_client/prop_commands.c (working copy)
@@ -498,6 +498,7 @@
svn_wc_adm_access_t *adm_access;
const svn_wc_entry_t *node;
const char *utarget; /* target, or the url for target */
+ const char *url;
svn_revnum_t revnum;
*props = apr_hash_make (pool);
@@ -508,55 +509,18 @@
requested property information is not available locally. */
if (svn_path_is_url (utarget))
{
- void *ra_baton, *session;
+ void *session;
svn_ra_plugin_t *ra_lib;
svn_node_kind_t kind;
- svn_opt_revision_t new_revision; /* only used in one case */
- SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
- SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, utarget, pool));
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, utarget,
- NULL, NULL, NULL,
- FALSE, FALSE, ctx, pool));
+ /* Get an RA plugin for this filesystem object. */
+ SVN_ERR (svn_client__get_ra_for_path_or_url (&ra_lib, &session, &revnum,
+ &url, target, revision,
+ ctx, pool));
- /* Default to HEAD. */
- if (revision->kind == svn_opt_revision_unspecified)
- {
- new_revision.kind = svn_opt_revision_head;
- revision = &new_revision;
- }
-
- /* Handle the various different kinds of revisions. */
- if ((revision->kind == svn_opt_revision_head)
- || (revision->kind == svn_opt_revision_date)
- || (revision->kind == svn_opt_revision_number))
- {
- SVN_ERR (svn_client__get_revision_number
- (&revnum, ra_lib, session, revision, NULL, pool));
- }
- else if (revision->kind == svn_opt_revision_previous)
- {
- if (svn_path_is_url (target))
- {
- return svn_error_createf
- (SVN_ERR_ILLEGAL_TARGET, NULL,
- _("'%s' is a URL, but revision kind requires a "
- "working copy"), target);
- }
-
- /* target is a working copy path */
- SVN_ERR (svn_client__get_revision_number
- (&revnum, NULL, NULL, revision, target, pool));
- }
- else
- {
- return svn_error_create
- (SVN_ERR_CLIENT_BAD_REVISION, NULL, _("Unknown revision kind"));
- }
-
SVN_ERR (ra_lib->check_path (session, "", revnum, &kind, pool));
- SVN_ERR (remote_propget (*props, propname, utarget, "",
+ SVN_ERR (remote_propget (*props, propname, url, "",
kind, revnum, ra_lib, session,
recurse, pool));
}
@@ -856,6 +820,7 @@
svn_wc_adm_access_t *adm_access;
const svn_wc_entry_t *node;
const char *utarget; /* target, or the url for target */
+ const char *url;
svn_revnum_t revnum;
*props = apr_array_make (pool, 5, sizeof (svn_client_proplist_item_t *));
@@ -866,55 +831,18 @@
requested property information is not available locally. */
if (svn_path_is_url (utarget))
{
- void *ra_baton, *session;
+ void *session;
svn_ra_plugin_t *ra_lib;
svn_node_kind_t kind;
- svn_opt_revision_t new_revision; /* only used in one case */
- SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
- SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, utarget, pool));
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, utarget,
- NULL, NULL, NULL,
- FALSE, FALSE, ctx, pool));
-
- /* Default to HEAD. */
- if (revision->kind == svn_opt_revision_unspecified)
- {
- new_revision.kind = svn_opt_revision_head;
- revision = &new_revision;
- }
-
- /* Handle the various different kinds of revisions. */
- if ((revision->kind == svn_opt_revision_head)
- || (revision->kind == svn_opt_revision_date)
- || (revision->kind == svn_opt_revision_number))
- {
- SVN_ERR (svn_client__get_revision_number
- (&revnum, ra_lib, session, revision, NULL, pool));
- }
- else if (revision->kind == svn_opt_revision_previous)
- {
- if (svn_path_is_url (target))
- {
- return svn_error_createf
- (SVN_ERR_ILLEGAL_TARGET, NULL,
- _("'%s' is a URL, but revision kind requires a "
- "working copy"), target);
- }
-
- /* target is a working copy path */
- SVN_ERR (svn_client__get_revision_number
- (&revnum, NULL, NULL, revision, target, pool));
- }
- else
- {
- return svn_error_create
- (SVN_ERR_CLIENT_BAD_REVISION, NULL, _("Unknown revision kind"));
- }
-
+ /* Get an RA plugin for this filesystem object. */
+ SVN_ERR (svn_client__get_ra_for_path_or_url (&ra_lib, &session, &revnum,
+ &url, target, revision,
+ ctx, pool));
+
SVN_ERR (ra_lib->check_path (session, "", revnum, &kind, pool));
- SVN_ERR (remote_proplist (*props, utarget, "",
+ SVN_ERR (remote_proplist (*props, url, "",
kind, revnum, ra_lib, session,
recurse, pool));
}
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 9609)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -570,7 +570,7 @@
/* Populate most of our log receiver baton structure. */
SVN_ERR (ra_lib->get_repos_root (ra_session, &repos_url, pool));
- lrb.last_path = url + strlen (repos_url);
+ lrb.last_path = svn_path_uri_decode (url + strlen (repos_url), pool);
lrb.start_revision = start_revnum;
lrb.end_revision = end_revnum;
lrb.peg_revision = peg_revnum;
Index: subversion/libsvn_client/cat.c
===================================================================
--- subversion/libsvn_client/cat.c (revision 9609)
+++ subversion/libsvn_client/cat.c (working copy)
@@ -43,83 +43,19 @@
apr_pool_t *pool)
{
svn_ra_plugin_t *ra_lib;
- void *ra_baton, *session;
+ void *session;
svn_revnum_t rev;
svn_node_kind_t url_kind;
svn_string_t *eol_style;
svn_string_t *keywords;
apr_hash_t *props;
- const char *initial_url, *url;
- const svn_opt_revision_t *good_rev;
- apr_pool_t *ra_subpool;
+ const char *url;
- /* Open an RA session to the incoming URL. */
- SVN_ERR (svn_client_url_from_path (&initial_url, path_or_url, pool));
- if (! initial_url)
- return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
- "'%s' has no URL", path_or_url);
+ /* Get an RA plugin for this filesystem object. */
+ SVN_ERR (svn_client__get_ra_for_path_or_url (&ra_lib, &session, &rev,
+ &url, path_or_url, revision,
+ ctx, pool));
- SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
- SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, initial_url, pool));
-
- ra_subpool = svn_pool_create (pool);
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, initial_url,
- NULL, NULL, NULL, FALSE, FALSE,
- ctx, ra_subpool));
-
- if (svn_path_is_url (path_or_url))
- {
- /* If an explicit URL was passed in, just use it. */
- good_rev = revision;
- url = initial_url;
- }
- else
- {
- /* For a working copy path, don't blindly use its initial_url
- from the entries file. Run the history function to get the
- object's (possibly different) url in REVISION. */
- svn_opt_revision_t base_rev, dead_end_rev, start_rev;
- svn_opt_revision_t *ignored_rev, *new_rev;
- const char *ignored_url;
-
- dead_end_rev.kind = svn_opt_revision_unspecified;
- base_rev.kind = svn_opt_revision_base;
-
- if (revision->kind == svn_opt_revision_unspecified)
- start_rev.kind = svn_opt_revision_base;
- else
- start_rev = *revision;
-
- SVN_ERR (svn_client__repos_locations (&url, &new_rev,
- &ignored_url, &ignored_rev,
- /* peg coords are path@BASE: */
- path_or_url, &base_rev,
- /* search range: */
- &start_rev, &dead_end_rev,
- ra_lib, session,
- ctx, pool));
- good_rev = new_rev;
-
- /* If 'url' turns out to be different than 'initial_url', then we
- need to open a new RA session to it. */
- if (strcmp (url, initial_url) != 0)
- {
- svn_pool_clear (ra_subpool);
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, url,
- NULL, NULL, NULL, FALSE, FALSE,
- ctx, ra_subpool));
- }
- }
-
- /* From here out, 'url' and 'good_rev' are the true repos
- coordinates we need to fetch. */
-
- /* Resolve good_rev into a real revnum. */
- SVN_ERR (svn_client__get_revision_number (&rev, ra_lib, session,
- good_rev, url, pool));
- if (! SVN_IS_VALID_REVNUM (rev))
- SVN_ERR (ra_lib->get_latest_revnum (session, &rev, pool));
-
/* Make sure the object isn't a directory. */
SVN_ERR (ra_lib->check_path (session, "", rev, &url_kind, pool));
if (url_kind == svn_node_dir)
Index: subversion/libsvn_client/ls.c
===================================================================
--- subversion/libsvn_client/ls.c (revision 9609)
+++ subversion/libsvn_client/ls.c (working copy)
@@ -78,34 +78,16 @@
apr_pool_t *pool)
{
svn_ra_plugin_t *ra_lib;
- void *ra_baton, *session;
+ void *session;
svn_revnum_t rev;
svn_node_kind_t url_kind;
const char *url;
- SVN_ERR (svn_client_url_from_path (&url, path_or_url, pool));
- if (! url)
- return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
- "'%s' has no URL", path_or_url);
+ /* Get an RA plugin for this filesystem object. */
+ SVN_ERR (svn_client__get_ra_for_path_or_url (&ra_lib, &session, &rev,
+ &url, path_or_url, revision,
+ ctx, pool));
- /* Get the RA library that handles URL. */
- SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
- SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, url, pool));
-
- /* Open a repository session to the URL. */
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, url,
- NULL,
- NULL, NULL, FALSE, TRUE,
- ctx, pool));
-
- /* Resolve REVISION into a real revnum. */
- SVN_ERR (svn_client__get_revision_number
- (&rev, ra_lib, session, revision,
- (path_or_url == url) ? NULL : path_or_url, pool));
-
- if (! SVN_IS_VALID_REVNUM (rev))
- SVN_ERR (ra_lib->get_latest_revnum (session, &rev, pool));
-
/* Decide if the URL is a file or directory. */
SVN_ERR (ra_lib->check_path (session, "", rev, &url_kind, pool));
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 9609)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -286,7 +286,7 @@
struct log_message_baton lmb;
apr_array_header_t *condensed_targets;
svn_ra_plugin_t *ra_lib;
- void *ra_baton, *session;
+ void *session;
const char *url;
svn_revnum_t start_revnum, end_revnum;
struct blame *walk;
@@ -307,23 +307,13 @@
iterpool = svn_pool_create (pool);
lastpool = svn_pool_create (pool);
- SVN_ERR (svn_client_url_from_path (&url, target, pool));
- if (! url)
- return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
- "'%s' has no URL", target);
+ /* Get an RA plugin for this filesystem object. */
+ SVN_ERR (svn_client__get_ra_for_path_or_url (&ra_lib, &session, &end_revnum,
+ &url, target, end,
+ ctx, pool));
-
- SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
- SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, url, pool));
-
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, url, NULL,
- NULL, NULL, FALSE, FALSE,
- ctx, pool));
-
SVN_ERR (svn_client__get_revision_number (&start_revnum, ra_lib, session,
start, target, pool));
- SVN_ERR (svn_client__get_revision_number (&end_revnum, ra_lib, session,
- end, target, pool));
if (end_revnum < start_revnum)
return svn_error_create
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 4 13:47:28 2004