On Fri, Mar 28, 2003 at 05:36:09PM +0000, Philip Martin wrote:
>
> > but what about other
> > platforms? i.e., is it okay for path functions to handle URLs?
>
> I don't understand the question.
'svn_path_condense_targets' assumes that each target is a path, and
converts each path to an absolute path. my patch, as it stands, changes
this function so that if a target appears to be a URL, it's left as
is, i.e., the function doesn't try to convert it to an absolute path.
what i'm wondering is whether this would cause problems if there were a
file path that looked like a URL, or whether it is OK to assume that if
a target looks like a URL, then it is.
> I've looked over the patch but I can't determine which problem you are
> trying to solve, perhaps I need to go and look up that issue...
sorry. this patch addresses the issue of 'svn log' giving the wrong
output for a file which has been switched relative to its parent
directory.
the original reproduction recipe from the issue tracker is:
$ svnadmin create repo
$ svn mkdir file://`pwd`/repo/trunk
$ svn co file://`pwd`/repo/trunk wc
$ echo foo > wc/foo
$ svn add wc/foo
$ svn ci -m "trunk create" wc
$ svn cp -m "branch cp" file://`pwd`/repo/trunk file://`pwd`/repo/branch
$ svn ps x x wc/foo
$ svn ci -m "trunk ps" wc
$ svn sw file://`pwd`/repo/branch/foo wc/foo
$ svn ps y y wc/foo
$ svn ci -m "branch ps" wc
$ svn log wc/foo # shows the log for the trunk.
the last substantive comment about this issue in IZ was (cmpilato):
"So, the solution is to call svn_path_condense_targets on the *URLs* of
the targets on the path?"
after thinking about it for a while, this seems like a good approach to
me. but it requires that svn_path_condense_targets (or some equivalent)
handle URLs.
same patch, with a bit of a log message, follows.
-brian
Log:
Resolves Issue #1108
('svn log' for switched file reports for unswitched file)
* target.c
(get_absolute_path_or_url): New function.
(svn_path_condense_targets): Accept URL targets; don't try to convert
them to absolute paths.
* log.c
(svn_client_log): Find the URL for each target, and send the list of
URLs, not file targets, to svn_path_condense_targets.
Index: subversion/libsvn_subr/target.c
===================================================================
--- subversion/libsvn_subr/target.c (revision 5485)
+++ subversion/libsvn_subr/target.c (working copy)
@@ -32,6 +32,28 @@
/*** Code. ***/
+/* Get the absolute path corresponding to RELATIVE, treating URLs
+ * as absolute paths.
+ *
+ * If RELATIVE is a URL, return the same URL in *PABSOLUTE.
+ * Otherwise return an absolute path for RELATIVE.
+ *
+ * ALLOCATE everything in POOL.
+ */
+static svn_error_t *
+get_absolute_path_or_url (const char **pabsolute,
+ const char *relative,
+ apr_pool_t *pool)
+{
+ if (svn_path_is_url (relative))
+ *pabsolute = apr_pstrdup (pool, relative);
+ else
+ SVN_ERR (svn_path_get_absolute (pabsolute, relative, pool));
+
+ return SVN_NO_ERROR;
+}
+
+
svn_error_t *
svn_path_condense_targets (const char **pbasedir,
apr_array_header_t **pcondensed_targets,
@@ -63,9 +85,9 @@
apr_array_header_t *abs_targets
= apr_array_make (pool, targets->nelts, sizeof (const char *));
- SVN_ERR (svn_path_get_absolute (pbasedir,
- ((const char **) targets->elts)[0],
- pool));
+ SVN_ERR (get_absolute_path_or_url (pbasedir,
+ ((const char **) targets->elts)[0],
+ pool));
(*((const char **)apr_array_push (abs_targets))) = *pbasedir;
@@ -73,7 +95,7 @@
{
const char *rel = ((const char **)targets->elts)[i];
const char *absolute;
- SVN_ERR (svn_path_get_absolute (&absolute, rel, pool));
+ SVN_ERR (get_absolute_path_or_url (&absolute, rel, pool));
(*((const char **)apr_array_push (abs_targets))) = absolute;
*pbasedir = svn_path_get_longest_ancestor (*pbasedir,
absolute,
@@ -163,13 +185,16 @@
}
}
- /* Finally check if pbasedir is a dir or a file. */
- SVN_ERR (svn_path_split_if_file (*pbasedir, pbasedir, &file, pool));
- if ((pcondensed_targets != NULL) && (! svn_path_is_empty (file)))
+ /* Finally check if pbasedir is a dir or a file (or a URL). */
+ if (! svn_path_is_url (*pbasedir))
{
- /* If there was just one target, and it was a file, then
- return it as the sole condensed target. */
- (*((const char **)apr_array_push (*pcondensed_targets))) = file;
+ SVN_ERR (svn_path_split_if_file (*pbasedir, pbasedir, &file, pool));
+ if ((pcondensed_targets != NULL) && (! svn_path_is_empty (file)))
+ {
+ /* If there was just one target, and it was a file, then
+ return it as the sole condensed target. */
+ (*((const char **)apr_array_push (*pcondensed_targets))) = file;
+ }
}
}
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 5485)
+++ subversion/libsvn_client/log.c (working copy)
@@ -59,7 +59,7 @@
svn_ra_plugin_t *ra_lib;
void *ra_baton, *session;
const char *path;
- const char *URL;
+ const char *base_url;
const char *base_name = NULL;
const char *auth_dir;
apr_array_header_t *condensed_targets;
@@ -81,7 +81,7 @@
/* Use the passed URL, if there is one. */
if (svn_path_is_url (path))
{
- URL = path;
+ base_url = path;
/* Initialize this array, since we'll be building it below */
condensed_targets = apr_array_make (pool, 1, sizeof (const char *));
@@ -110,49 +110,59 @@
else
{
svn_wc_adm_access_t *adm_access;
- const svn_wc_entry_t *entry;
+ apr_array_header_t *target_urls;
+ int i;
+
+ /* Get URLs for each target */
+ target_urls = apr_array_make (pool, 1, sizeof (const char *));
+ for (i = 0; i < targets->nelts; i++)
+ {
+ const svn_wc_entry_t *entry;
+ const char *URL;
+ const char *target = APR_ARRAY_IDX(targets, i, const char *);
+ SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target,
+ FALSE, FALSE, pool));
+ SVN_ERR (svn_wc_entry (&entry, target, adm_access, FALSE, pool));
+ if (! entry)
+ return svn_error_createf
+ (SVN_ERR_UNVERSIONED_RESOURCE, NULL,
+ "svn_client_log: '%s' is not under revision control", target);
+ if (! entry->url)
+ return svn_error_createf
+ (SVN_ERR_ENTRY_MISSING_URL, NULL,
+ "svn_client_log: entry '%s' has no URL", target);
+ URL = apr_pstrdup (pool, entry->url);
+ SVN_ERR (svn_wc_adm_close (adm_access));
+ (*((const char **)apr_array_push (target_urls))) = URL;
+ }
- /* Use local working copy. */
+ /* Find the base URL and condensed targets relative to it. */
+ SVN_ERR (svn_path_condense_targets (&base_url, &condensed_targets,
+ target_urls, pool));
- SVN_ERR (svn_path_condense_targets (&base_name, &condensed_targets,
- targets, pool));
-
if (condensed_targets->nelts == 0)
(*((const char **)apr_array_push (condensed_targets))) = "";
-
- SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, base_name,
- FALSE, FALSE, pool));
- SVN_ERR (svn_wc_entry (&entry, base_name, adm_access, FALSE, pool));
- if (! entry)
- return svn_error_createf
- (SVN_ERR_UNVERSIONED_RESOURCE, NULL,
- "svn_client_log: '%s' is not under revision control", base_name);
- if (! entry->url)
- return svn_error_createf
- (SVN_ERR_ENTRY_MISSING_URL, NULL,
- "svn_client_log: entry '%s' has no URL", base_name);
- URL = apr_pstrdup (pool, entry->url);
- SVN_ERR (svn_wc_adm_close (adm_access));
}
- /* Get the RA library that handles URL. */
+ /* Get the RA library that handles BASE_URL */
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_ra_get_ra_library (&ra_lib, ra_baton, base_url, pool));
- /* Open a repository session to the URL. If we got here from a full URL
- passed to the command line, then if the current directory is a
+ /* Open a repository session to the BASE_URL If we got here from a full
+ URL passed to the command line, then if the current directory is a
working copy, we pass it as base_name for authentication
purposes. But we make sure to treat it as read-only, since when
one operates on URLs, one doesn't expect it to change anything in
the working copy. */
+ SVN_ERR (svn_path_condense_targets (&base_name, NULL, targets, pool));
if (NULL != base_name)
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL, base_name,
- NULL, NULL, TRUE, TRUE,
+ SVN_ERR (svn_client__open_ra_session (&session, ra_lib, base_url,
+ base_name, NULL, NULL, TRUE, TRUE,
ctx, pool));
else
{
SVN_ERR (svn_client__dir_if_wc (&auth_dir, "", pool));
- SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL,
+ SVN_ERR (svn_client__open_ra_session (&session, ra_lib, base_url,
auth_dir,
NULL, NULL, FALSE, TRUE,
ctx, pool));
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 28 18:59:58 2003