Here it is again, with significant changes. I went ahead and folded all
the URL-handling into the path library functions, and tied up some loose
ends discovered along the way.
Comments?
-brian
Log:
Resolves Issue #1108 ('svn log' for switched file reports for unswitched
file.)
Along the way, makes the following bugfixes/improvements:
- svn_path_condense_targets no longer trims the first character off of the
returned targets if they have no common basepath.
- svn_path_get_longest_ancestor now really does return NULL if there is no
common ancestor, like its documentation says; and callers of this
function check for a NULL return value.
- svn_path_get_longest_ancestor, svn_path_get_absolute, and
svn_path_condense_targets now handle URLs more intelligently.
* svn_path.h
(svn_path_get_longest_ancestor): Update comment.
(svn_path_get_absolute): Update comment.
(svn_path_condense_targets): Update comment.
* svn_client.h
(svn_client_log): Update comment.
* target.c
(svn_path_condense_targets): Accept URL targets; don't try to convert
them to absolute paths. Make sure to accept NULL return values from
svn_path_get_longest_ancestor.
* path.c
(get_longest_path_ancestor): New helper function, mostly identical with
the old svn_path_get_longest_ancestor, but fixed to return NULL if there
is no common ancestor, as per the svn_path_get_longest_ancestor docstring.
(svn_path_get_longest_ancestor): Accept URL targets; don't count
'protocol://' as a common ancestor.
(svn_path_get_absolute): Accept URL targets; treat them as absolute paths.
* copy.c
(repos_to_repos_copy): Handle a NULL return value from
svn_path_get_longest_ancestor.
* 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.
* commit_util.c
(svn_client__condense_commit_items): Handle a NULL return value from
svn_path_get_longest_ancestor.
* target-test.py
(tests): Add URL tests for svn_path_condense_targets.
* switch_tests.py
(log_switched_file): New test function (featuring Python-fu from
Master Ling and Master Stein).
Index: subversion/include/svn_path.h
===================================================================
--- subversion/include/svn_path.h (revision 5495)
+++ subversion/include/svn_path.h (working copy)
@@ -192,6 +192,10 @@
*
* Return the longest common path shared by both @a path1 and @a path2. If
* there's no common ancestor, return @c NULL.
+ *
+ * @a path1 and @a path2 may be URLs. It is not enough for two URLs to
+ * share the prefix 'protocol://'; they must actually share some of the
+ * subsequent path in order to be deemed to have a common ancestor.
*/
char *svn_path_get_longest_ancestor (const char *path1,
const char *path2,
@@ -199,6 +203,9 @@
/** Convert @a relative path to an absolute path and return the results in
* @a *pabsolute, allocated in @a pool.
+ *
+ * @a relative may be a URL, in which case no attempt is made to convert it,
+ * and a copy of the URL is returned.
*/
svn_error_t *
svn_path_get_absolute (const char **pabsolute,
@@ -223,14 +230,15 @@
*
* Find the common prefix of the paths in @a targets, and remove redundancies.
*
- * The elements in @a targets must be existing files or directories (as
- * const char *).
+ * Each of the elements in @a targets must be a URL, or an existing file or
+ * directory (as const char *).
*
* If there are multiple targets, or exactly one target and it's not a
* directory, then
*
* - @a *pbasename is set to the absolute path of the common parent
- * directory of all of those targets, and
+ * directory of those targets (if the targets are files/directories),
+ * or the common URL prefix of the targets (if they are URLs).
*
* - If @a pcondensed_targets is non-null, @a *pcondensed_targets is set
* to an array of targets relative to @a *pbasename, with
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 5495)
+++ subversion/include/svn_client.h (working copy)
@@ -695,10 +695,11 @@
* given log message more than once).
*
* @a targets contains all the working copy paths (as <tt>const char
- * *</tt>'s) for which log messages are desired; the common prefix of @a
- * targets determines the repository and auth info. @a receiver is invoked
- * only on messages whose revisions involved a change to some path in
- * @a targets.
+ * *</tt>'s) for which log messages are desired. The repository info is
+ * determinied by taking the common prefix of the target entries' URLs.
+ * The common prefix of @a targets, if it is a valid working copy,
+ * determines the auth info. @a receiver is invoked only on messages
+ * whose revisions involved a change to some path in @a targets.
*
* ### todo: the above paragraph is not fully implemented yet.
*
Index: subversion/libsvn_subr/target.c
===================================================================
--- subversion/libsvn_subr/target.c (revision 5495)
+++ subversion/libsvn_subr/target.c (working copy)
@@ -78,6 +78,8 @@
*pbasedir = svn_path_get_longest_ancestor (*pbasedir,
absolute,
pool);
+ if (! *pbasedir)
+ *pbasedir = "";
}
/* If we need to find the targets, find the common part of each pair
@@ -155,7 +157,11 @@
continue;
rel_item = ((const char **)abs_targets->elts)[i];
- rel_item += basedir_len + 1;
+
+ /* If a common prefix was found, condensed_targets
+ are given relative to that prefix. */
+ if (basedir_len > 0)
+ rel_item += basedir_len + 1;
(*((const char **)apr_array_push (*pcondensed_targets)))
= apr_pstrdup (pool, rel_item);
@@ -163,13 +169,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_subr/path.c
===================================================================
--- subversion/libsvn_subr/path.c (revision 5495)
+++ subversion/libsvn_subr/path.c (working copy)
@@ -468,10 +468,20 @@
}
-char *
-svn_path_get_longest_ancestor (const char *path1,
- const char *path2,
- apr_pool_t *pool)
+/* Return the longest common ancestor of PATH1 and PATH2.
+ *
+ * This function handles everything except the URL-handling logic
+ * of svn_path_get_longest_ancestor, and assumes that PATH1 and
+ * PATH2 are *not* URLs.
+ *
+ * If the two paths do not share a common ancestor, NULL is returned.
+ *
+ * New strings are allocated in POOL.
+ */
+static char *
+get_longest_path_ancestor (const char *path1,
+ const char *path2,
+ apr_pool_t *pool)
{
char *common_path;
apr_size_t path1_len, path2_len;
@@ -500,10 +510,14 @@
/* last_dirsep is now the offset of the last directory separator we
crossed before reaching a non-matching byte. i is the offset of
that non-matching byte. */
- if (((i == path1_len) && (path2[i] == '/'))
- || ((i == path2_len) && (path1[i] == '/'))
- || ((i == path1_len) && (i == path2_len)))
+ if (i == 0)
+ return NULL;
+ else if (((i == path1_len) && (path2[i] == '/'))
+ || ((i == path2_len) && (path1[i] == '/'))
+ || ((i == path1_len) && (i == path2_len)))
common_path = apr_pstrmemdup (pool, path1, i);
+ else if (last_dirsep == 0)
+ return NULL;
else
common_path = apr_pstrmemdup (pool, path1, last_dirsep);
@@ -511,6 +525,55 @@
}
+char *
+svn_path_get_longest_ancestor (const char *path1,
+ const char *path2,
+ apr_pool_t *pool)
+{
+ if (svn_path_is_url (path1))
+ {
+ if (svn_path_is_url (path2))
+ {
+ char *ancestor, *path_ancestor, *prefix;
+ int i = 0;
+
+ /* Find ':' */
+ for (i = 0; path1[i] != ':'; i++)
+ {
+ /* They're both URLs, so EOS can't come before ':' */
+ assert (path1[i] != '\0' && path2[i] != '\0');
+
+ /* No shared protocol => no common prefix */
+ if (path1[i] != path2[i])
+ return NULL;
+ }
+
+ i += 3; /* Advance past '://' */
+
+ path_ancestor = get_longest_path_ancestor (path1 + i, path2 + i,
+ pool);
+
+ /* No shared path => no common prefix */
+ if ((! path_ancestor) || (*path_ancestor == '\0'))
+ return NULL;
+
+ else
+ return apr_pstrcat (pool, apr_pstrndup (pool, path1, i),
+ path_ancestor, NULL);
+ }
+ else
+ {
+ /* A URL and a non-URL => no common prefix */
+ return NULL;
+ }
+ }
+ else
+ {
+ return get_longest_path_ancestor (path1, path2, pool);
+ }
+}
+
+
const char *
svn_path_is_child (const char *path1,
const char *path2,
@@ -870,16 +933,23 @@
SVN_ERR (svn_path_cstring_from_utf8
(&path_apr, svn_path_canonicalize (relative, pool), pool));
- apr_err = apr_filepath_merge(&buffer, NULL,
- path_apr,
- (APR_FILEPATH_NOTRELATIVE
- | APR_FILEPATH_TRUENAME),
- pool);
+ if (svn_path_is_url (path_apr))
+ {
+ buffer = apr_pstrdup (pool, path_apr);
+ }
+ else
+ {
+ apr_err = apr_filepath_merge(&buffer, NULL,
+ path_apr,
+ (APR_FILEPATH_NOTRELATIVE
+ | APR_FILEPATH_TRUENAME),
+ pool);
- if (apr_err)
- return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
- "Couldn't determine absolute path of '%s'.",
- relative);
+ if (apr_err)
+ return svn_error_createf(SVN_ERR_BAD_FILENAME, NULL,
+ "Couldn't determine absolute path of '%s'.",
+ relative);
+ }
SVN_ERR (svn_path_cstring_to_utf8 (pabsolute, buffer, pool));
*pabsolute = svn_path_canonicalize (*pabsolute, pool);
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 5495)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -224,6 +224,8 @@
checks on both paths, and so we can operate on both paths in the
case of a move. */
top_url = svn_path_get_longest_ancestor (src_url, dst_url, pool);
+ if (! top_url)
+ top_url = "";
/* Special edge-case! (issue #683) If you're resurrecting a
deleted item like this: 'svn cp -rN src_URL dst_URL', then it's
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 5495)
+++ 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));
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 5495)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -689,6 +689,9 @@
else
*base_url = svn_path_get_longest_ancestor (*base_url, url, pool);
+ if (! *base_url)
+ *base_url = "";
+
/* If our BASE_URL is itself a to-be-committed item, and it is
anything other than an already-versioned directory with
property mods, we'll call its parent directory URL the
Index: subversion/tests/libsvn_subr/target-test.py
===================================================================
--- subversion/tests/libsvn_subr/target-test.py (revision 5495)
+++ subversion/tests/libsvn_subr/target-test.py (working copy)
@@ -34,7 +34,22 @@
cwd + '/z/A: \n'),
('single file',
'z/A/file',
- cwd + '/z/A: file, \n')]
+ cwd + '/z/A: file, \n'),
+ ('URLs',
+ 'http://host/A/C http://host/A/C/D http://host/A/B/D',
+ 'http://host/A: C, B/D, \n'),
+ ('URLs with no common prefix',
+ 'http://host1/A/C http://host2/A/C/D http://host3/A/B/D',
+ ': http://host1/A/C, http://host2/A/C/D, http://host3/A/B/D, \n'),
+ ('file URLs with no common prefix',
+ 'file:///A/C file:///B/D',
+ ': file:///A/C, file:///B/D, \n'),
+ ('URLs with mixed protocols',
+ 'http://host/A/C file:///B/D',
+ ': http://host/A/C, file:///B/D, \n'),
+ ('mixed paths and URLs',
+ 'z/A/B http://host/A/C/D',
+ ': ' + cwd + '/z/A/B, http://host/A/C/D, \n')]
# (re)Create the test directory
if os.path.exists('z'):
Index: subversion/tests/clients/cmdline/switch_tests.py
===================================================================
--- subversion/tests/clients/cmdline/switch_tests.py (revision 5495)
+++ subversion/tests/clients/cmdline/switch_tests.py (working copy)
@@ -518,6 +518,34 @@
#----------------------------------------------------------------------
+def log_switched_file(sbox):
+ "show logs for a switched file"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ # Setup some switched things (don't bother verifying)
+ if do_routine_switching(wc_dir, 0):
+ raise svntest.Failure # really, do_routine_switching should raise this
+
+ # edit and commit switched file 'iota'
+ iota_path = os.path.join(wc_dir, 'iota')
+ svntest.main.run_svn (None, 'ps', 'x', 'x', iota_path)
+ svntest.main.run_svn (None, 'ci', '-m',
+ 'set prop on switched iota',
+ iota_path)
+
+ # log switched file 'iota'
+ output, error = svntest.main.run_svn (None, 'log', iota_path)
+ for line in output:
+ if line.find("set prop on switched iota") != -1:
+ break
+ else:
+ raise svntest.Failure
+
+
+#----------------------------------------------------------------------
+
### ...and a slew of others...
########################################################################
@@ -532,6 +560,7 @@
full_rev_update,
update_switched_things,
rev_update_switched_things,
+ log_switched_file,
]
if __name__ == '__main__':
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Mar 30 09:18:47 2003