-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
At present when invalid revision keywords (BASE|PREV|COMMITTED) are used
with a URL, svn gracefully errors out only for four subcommands (merge,
diff, copy, log AFAIK) as of now. This patch extends to other
subcommands too, by having the error message in a common spot.
[[[
Log:
Make svn error out gracefully when invalid revision
keywords(BASE|PREV|COMMITTED) are used with a URL, hereby extending to
all subcommands. Previously it was handled gracefully only for merge,
diff, copy, log.
* subversion/libsvn_client/revisions.c
(svn_client__get_revision_number): If the incoming path is a URL
demanding a wc revision argument, error out gracefully.
* subversion/libsvn_client/ra.c
(svn_client__repos_locations): Rename `local_abspath' to
`local_abspath_or_url' as it holds either a wc abs-path or a URL and
reflect the other changes accordingly.
* merge.c
(normalize_merge_sources): Remove the redundant code.
* diff.c
(check_paths): Same.
* log.c
(svn_client_log5): Same.
* copy.c
(try_copy): Same.
Patch by: Kannan R <[email protected]>
]]]
P.S: Part of the changes made in ra.c and merge.c, is also submitted in
[1]. This patch extends the same.
[1]-http://mail-archives.apache.org/mod_mbox/subversion-dev/200912.mbox/<[email protected]>
- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQEVAwUBSxj19nlTqcY7ytmIAQJNGQf/WjSCig+DRFOwQrf289NfnSs6o/h4+dwI
mR8S9VamLHSkTcqvAI0Uf3LqSZqAUFY1eN24VL1P0o6TzNy+cq2sV6JczIIb2p6D
uayleu1qwW0YAVVOcKHwEKq28GLufTBl1/18nK+S+nYwHzTUAUnDUGuh5j1qkECJ
M14QmIsvzsq38hpX/2Xp8tT2QWMV7FGLsYHEeZVBCggtJZ0mOY1TYhQQYeipfzp4
SpmV9eW+G61btVyjSC5eyJ0cgy3Ju3AMIs5PVGpYR5iHbUOajSSOgGtZzD8gAIMk
AxgdpE+7ZPkTXJbaNWAwUq2hfDOn7L0BJKmhkOj1zoGyO7PB6LP7LQ==
=VJ9J
-----END PGP SIGNATURE-----
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 887080)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -5952,7 +5952,7 @@
svn_revnum_t trim_revision = SVN_INVALID_REVNUM;
svn_opt_revision_t youngest_opt_rev;
apr_array_header_t *merge_range_ts, *segments;
- const char *source_abspath;
+ const char *source_abspath_or_url;
apr_pool_t *subpool;
int i;
youngest_opt_rev.kind = svn_opt_revision_head;
@@ -5963,15 +5963,18 @@
### operations are just as sloppy/forgiving as we are about
### handling URL inputs where local paths are expected, but
### that's a shaky limb to stand on. */
+ if(!svn_path_is_url(source))
+ SVN_ERR(svn_dirent_get_absolute(&source_abspath_or_url, source, pool));
+ else
+ source_abspath_or_url = source;
- SVN_ERR(svn_dirent_get_absolute(&source_abspath, source, pool));
-
/* Initialize our return variable. */
*merge_sources_p = apr_array_make(pool, 1, sizeof(merge_source_t *));
/* Resolve our PEG_REVISION to a real number. */
SVN_ERR(svn_client__get_revision_number(&peg_revnum, &youngest_rev,
- ctx->wc_ctx, source_abspath,
+ ctx->wc_ctx,
+ source_abspath_or_url,
ra_session, peg_revision, pool));
if (! SVN_IS_VALID_REVNUM(peg_revnum))
return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
@@ -5999,25 +6002,14 @@
return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL,
_("Not all required revisions are specified"));
- /* The BASE, COMMITTED, and PREV revision keywords do not
- apply to URLs. */
- if (svn_path_is_url(source)
- && (range_start->kind == svn_opt_revision_base
- || range_end->kind == svn_opt_revision_base
- || range_start->kind == svn_opt_revision_committed
- || range_end->kind == svn_opt_revision_committed
- || range_start->kind == svn_opt_revision_previous
- || range_end->kind == svn_opt_revision_previous))
- return svn_error_create(
- SVN_ERR_CLIENT_BAD_REVISION, NULL,
- _("PREV, BASE, or COMMITTED revision keywords are invalid for URL"));
-
SVN_ERR(svn_client__get_revision_number(&range_start_rev, &youngest_rev,
- ctx->wc_ctx, source_abspath,
+ ctx->wc_ctx,
+ source_abspath_or_url,
ra_session, range_start,
subpool));
SVN_ERR(svn_client__get_revision_number(&range_end_rev, &youngest_rev,
- ctx->wc_ctx, source_abspath,
+ ctx->wc_ctx,
+ source_abspath_or_url,
ra_session, range_end,
subpool));
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 887080)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -959,14 +959,6 @@
return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL,
_("Not all required revisions are specified"));
- if ((svn_path_is_url(params->path1)
- && SVN_CLIENT__REVKIND_NEEDS_WC(params->revision1->kind))
- || (svn_path_is_url(params->path2)
- && SVN_CLIENT__REVKIND_NEEDS_WC(params->revision2->kind)))
- return svn_error_create(
- SVN_ERR_CLIENT_BAD_REVISION, NULL,
- _("PREV, BASE, or COMMITTED revision keywords are invalid for URL"));
-
/* Revisions can be said to be local or remote. BASE and WORKING,
for example, are local. */
is_local_rev1 =
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 887080)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -1747,19 +1747,6 @@
svn_boolean_t srcs_are_urls, dst_is_url;
int i;
- /* Check to see if the supplied peg revisions make sense. */
- for (i = 0; i < sources->nelts; i++)
- {
- svn_client_copy_source_t *source =
- ((svn_client_copy_source_t **) (sources->elts))[i];
-
- if (svn_path_is_url(source->path)
- && (SVN_CLIENT__REVKIND_NEEDS_WC(source->peg_revision->kind)))
- return svn_error_create
- (SVN_ERR_CLIENT_BAD_REVISION, NULL,
- _("Revision type requires a working copy path, not a URL"));
- }
-
/* Are either of our paths URLs?
* Just check the first src_path. If there are more than one, we'll check
* for homogeneity among them down below. */
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 887080)
+++ subversion/libsvn_client/log.c (working copy)
@@ -334,14 +334,6 @@
/* Use the passed URL, if there is one. */
url_or_path = APR_ARRAY_IDX(targets, 0, const char *);
is_url = svn_path_is_url(url_or_path);
-
- if (is_url && SVN_CLIENT__REVKIND_NEEDS_WC(peg_revision->kind))
- {
- return svn_error_create
- (SVN_ERR_CLIENT_BAD_REVISION, NULL,
- _("Revision type requires a working copy path, not a URL"));
- }
-
session_opt_rev.kind = svn_opt_revision_unspecified;
for (i = 0; i < revision_ranges->nelts; i++)
@@ -393,15 +385,6 @@
_("Missing required revision specification"));
}
- if (is_url
- && (SVN_CLIENT__REVKIND_NEEDS_WC(range->start.kind)
- || SVN_CLIENT__REVKIND_NEEDS_WC(range->end.kind)))
- {
- return svn_error_create
- (SVN_ERR_CLIENT_BAD_REVISION, NULL,
- _("Revision type requires a working copy path, not a URL"));
- }
-
/* Determine the revision to open the RA session to. */
if (session_opt_rev.kind == svn_opt_revision_unspecified)
{
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 887080)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -544,10 +544,10 @@
apr_pool_t *pool)
{
const char *repos_url;
- const char *url;
+ const char *url = NULL;
const char *start_path = NULL;
const char *end_path = NULL;
- const char *local_abspath;
+ const char *local_abspath_or_url;
svn_revnum_t peg_revnum = SVN_INVALID_REVNUM;
svn_revnum_t start_revnum, end_revnum;
svn_revnum_t youngest_rev = SVN_INVALID_REVNUM;
@@ -555,8 +555,6 @@
apr_hash_t *rev_locs;
apr_pool_t *subpool = svn_pool_create(pool);
- SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, subpool));
-
/* Ensure that we are given some real revision data to work with.
(It's okay if the END is unspecified -- in that case, we'll just
set it to the same thing as START.) */
@@ -571,7 +569,9 @@
{
const svn_wc_entry_t *entry;
- SVN_ERR(svn_wc__get_entry_versioned(&entry, ctx->wc_ctx, local_abspath,
+ SVN_ERR(svn_dirent_get_absolute(&local_abspath_or_url, path, subpool));
+ SVN_ERR(svn_wc__get_entry_versioned(&entry, ctx->wc_ctx,
+ local_abspath_or_url,
svn_node_unknown, FALSE, FALSE,
pool, pool));
if (entry->copyfrom_url && revision->kind == svn_opt_revision_working)
@@ -597,7 +597,7 @@
}
else
{
- url = path;
+ local_abspath_or_url = path;
}
/* ### We should be smarter here. If the callers just asks for BASE and
@@ -606,24 +606,26 @@
/* Open a RA session to this URL if we don't have one already. */
if (! ra_session)
- SVN_ERR(svn_client__open_ra_session_internal(&ra_session, url, NULL,
- NULL, FALSE, TRUE,
+ SVN_ERR(svn_client__open_ra_session_internal(&ra_session,
+ url ? url
+ : local_abspath_or_url,
+ NULL, NULL, FALSE, TRUE,
ctx, subpool));
/* Resolve the opt_revision_ts. */
if (peg_revnum == SVN_INVALID_REVNUM)
SVN_ERR(svn_client__get_revision_number(&peg_revnum, &youngest_rev,
- ctx->wc_ctx, local_abspath,
+ ctx->wc_ctx, local_abspath_or_url,
ra_session, revision, pool));
SVN_ERR(svn_client__get_revision_number(&start_revnum, &youngest_rev,
- ctx->wc_ctx, local_abspath,
+ ctx->wc_ctx, local_abspath_or_url,
ra_session, start, pool));
if (end->kind == svn_opt_revision_unspecified)
end_revnum = start_revnum;
else
SVN_ERR(svn_client__get_revision_number(&end_revnum, &youngest_rev,
- ctx->wc_ctx, local_abspath,
+ ctx->wc_ctx, local_abspath_or_url,
ra_session, end, pool));
/* Set the output revision variables. */
@@ -640,9 +642,9 @@
if (start_revnum == peg_revnum && end_revnum == peg_revnum)
{
/* Avoid a network request in the common easy case. */
- *start_url = url;
+ *start_url = url ? url : local_abspath_or_url;
if (end->kind != svn_opt_revision_unspecified)
- *end_url = url;
+ *end_url = url ? url : local_abspath_or_url;
svn_pool_destroy(subpool);
return SVN_NO_ERROR;
}
Index: subversion/libsvn_client/revisions.c
===================================================================
--- subversion/libsvn_client/revisions.c (revision 887080)
+++ subversion/libsvn_client/revisions.c (working copy)
@@ -86,6 +86,11 @@
return svn_error_create(SVN_ERR_CLIENT_VERSIONED_PATH_REQUIRED,
NULL, NULL);
+ /* The BASE, COMMITTED, and PREV revision keywords do not
+ apply to URLs. */
+ if (svn_path_is_url(local_abspath))
+ goto invalid_rev_arg;
+
SVN_ERR(svn_wc__get_entry_versioned(&ent, wc_ctx, local_abspath,
svn_node_unknown, FALSE, FALSE,
scratch_pool, scratch_pool));
@@ -104,6 +109,11 @@
return svn_error_create(SVN_ERR_CLIENT_VERSIONED_PATH_REQUIRED,
NULL, NULL);
+ /* The BASE, COMMITTED, and PREV revision keywords do not
+ apply to URLs. */
+ if (svn_path_is_url(local_abspath))
+ goto invalid_rev_arg;
+
SVN_ERR(svn_wc__get_entry_versioned(&ent, wc_ctx, local_abspath,
svn_node_unknown, FALSE, FALSE,
scratch_pool, scratch_pool));
@@ -155,4 +165,10 @@
*revnum = *youngest_rev;
return SVN_NO_ERROR;
+
+ invalid_rev_arg:
+ return svn_error_create(
+ SVN_ERR_CLIENT_BAD_REVISION, NULL,
+ _("PREV, BASE, or COMMITTED revision keywords are invalid for URL"));
+
}
