You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Josh Pieper <jj...@pobox.com> on 2004/10/23 10:32:30 UTC
Should all sub-commands accept a peg-revision?
Currently diff and merge accept a "peg revision", it gives the history
tracing algorithm a place to start when looking for the actual node
revisions to operate on. Other sub-commands that perform history
tracing (cat, ls, propget, ...) don't accept peg revisions, and in
fact don't currently perform history tracing for URLs, only for
working copy paths.
This is an inconsistency in our UI that probably should be resolved.
I have a patch (attached) to add peg revisions to commands that used
to trace history but didn't take the peg revision.
I am unsure of several things before I feel comfortable committing
this:
1. Is this desirable in the first place? Adding tracing by default to
sub-commands makes the most intuitive way to specify a specific
revision, 'svn cat -r x URL', perform history tracing from HEAD.
This could possibly take a long time, especially with the new authz
security fixes. You would need to use 'svn cat URL@r' to get the
same quick response as before.
2. There are other sub-commands that reference specific revisions of a
file/directory but don't currently perform any tracing. (copy,
export, checkout). Should they be modified to use the new syntax
and semantics?
3. How do we describe this in help messages? My attached patch punts
on that question, and just shows the extra possible field. In my
attempts to describe peg revisions in the docstrings of the
libsvn_client functions, I was unable to find a simple wording that
would be easily understandable in the limited space of a help message.
-Josh
--------------------------------------
Add peg-revision options to blame, cat, ls, propget, and proplist. As
part of issue #1093 these were modified to trace history from a
working copy path, but did not accept a peg-revision argument in the
same way as diff and merge. Each of the subcommands needs a new API
function in libsvn_client to accept the additional peg revision
argument and new command line code as well.
* subversion/include/svn_client.h
(svn_client_blame2, svn_client_propget2, svn_client_proplist2,
svn_client_ls2, svn_client_cat2): New API functions that accept
peg revision arguments.
(svn_client_blame, svn_client_propget, svn_client_proplist,
svn_client_ls, svn_client_cat): Old deprecated declarations.
* subversion/libsvn_client/client.h
(svn_client__ra_lib_from_path): Since this is the function all the
above sub-commands use to find a revision, have it take a peg
revision argument.
* subversion/libsvn_client/prop_commands.c
* subversion/libsvn_client/cat.c
* subversion/libsvn_client/ls.c
* subversion/libsvn_client/blame.c
(svn_client_propget2, svn_client_proplist2, svn_client_cat2,
svn_client_ls2, svn_client_blame2): New versions of the old API
which accept peg revisions.
(svn_client_propget, svn_client_proplist, svn_client_cat,
svn_client_ls, svn_client_blame): Implemented in terms of their
new successors.
* subversion/libsvn_client/ra.c
(svn_client__ra_lib_from_path): Accept a new peg-revision which is
applied to both URLs and working copy paths. (This is of dubious
value for working copy paths but the fewer the special cases the
better.) If the peg revision is left as unspecified, default it
to HEAD for URLs, or WORKING for WC paths. If the peg revision is
specified, but the desired revision isn't, only get the peg
revision.
* subversion/clients/cmdline/cat-cmd.c
* subversion/clients/cmdline/ls-cmd.c
* subversion/clients/cmdline/blame-cmd.c
* subversion/clients/cmdline/propget-cmd.c
* subversion/clients/cmdline/proplist-cmd.c
(svn_cl__cat, svn_cl__ls, svn_cl__blame, svn_cl__propget,
svn_cl__proplist): Parse the peg-revision off of each target and
pass it into the new libsvn_client functions.
* subversion/clients/cmdline/main.c
(svn_cl__cmd_table): Update the help strings for the affected
sub-commands.
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 11562)
+++ subversion/include/svn_client.h (working copy)
@@ -792,9 +792,18 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool);
-/** Invoke @a receiver with @a receiver_baton on each line-blame item
+/**
+ * @since New in 1.2.
+ *
+ * Invoke @a receiver with @a receiver_baton on each line-blame item
* associated with revision @a end of @a path_or_url, using @a start
- * as the default source of all blame.
+ * as the default source of all blame. @a peg_revision indicates
+ * which in which revision @a path_or_url is valid. The actual file
+ * contents to blame are determined by tracing forward and backward in
+ * history from this point. If @a peg_revision is @c
+ * svn_opt_revision_unspecified then it defaults to @c
+ * svn_opt_revision_head for URL targets or @c
+ * svn_opt_revision_working for working copy targets.
*
* If @a start->kind or @a end->kind is @c svn_opt_revision_unspecified,
* return the error @c SVN_ERR_CLIENT_BAD_REVISION. If any of the
@@ -804,6 +813,22 @@
* Use @a pool for any temporary allocation.
*/
svn_error_t *
+svn_client_blame2 (const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *start,
+ const svn_opt_revision_t *end,
+ svn_client_blame_receiver_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/**
+ * @deprecated Provided for backward compatibility with the 1.1 API.
+ *
+ * Similar to svn_client_blame except that @a peg_revision is always
+ * left unspecified.
+ */
+svn_error_t *
svn_client_blame (const char *path_or_url,
const svn_opt_revision_t *start,
const svn_opt_revision_t *end,
@@ -1185,7 +1210,10 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool);
-/** Set @a *props to a hash table whose keys are `<tt>char *</tt>' paths,
+/**
+ * @since New in 1.2.
+ *
+ * Set @a *props to a hash table whose keys are `<tt>char *</tt>' paths,
* prefixed by @a target (a working copy path or a URL), of items on
* which property @a propname is set, and whose values are `@c svn_string_t
* *' representing the property value for @a propname at that path.
@@ -1196,10 +1224,14 @@
* property named @a propname.
*
* If @a revision->kind is @c svn_opt_revision_unspecified, then: get
- * properties from the working copy if @a target is a working copy path,
- * or from the repository head if @a target is a URL. Else get the
- * properties as of @a revision. Use the authentication baton in @a ctx
- * for authentication if contacting the repository.
+ * properties from the working copy if @a target is a working copy
+ * path, or from the repository head if @a target is a URL. Else get
+ * the properties as of @a revision. The actual node revision
+ * selected is determined by the path as it exists in @a peg_revision.
+ * If @a peg_revision is @c svn_opt_revision_unspecified, then it
+ * defaults to @c svn_opt_revision_head for URLs or @c
+ * svn_opt_revision_working for WC targets. Use the authentication
+ * baton in @a ctx for authentication if contacting the repository.
*
* If @a target is a file or @a recurse is false, @a *props will have
* at most one element.
@@ -1208,6 +1240,22 @@
* even if empty.
*/
svn_error_t *
+svn_client_propget2 (apr_hash_t **props,
+ const char *propname,
+ const char *target,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/**
+ * @deprecated Provided for backward compatibility with the 1.1 API.
+ *
+ * Similar to svn_client_propget2, except that the peg revision is
+ * always left as @c svn_opt_revision_unspecified.
+ */
+svn_error_t *
svn_client_propget (apr_hash_t **props,
const char *propname,
const char *target,
@@ -1236,7 +1284,10 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool);
-/** Set @a *props to the regular properties of @a target, a URL or working
+/**
+ * @since New in 1.2.
+ *
+ * Set @a *props to the regular properties of @a target, a URL or working
* copy path.
*
* Each element of the returned array is (@c svn_client_proplist_item_t *).
@@ -1259,8 +1310,23 @@
* If @a target is not found, return the error @c SVN_ERR_ENTRY_NOT_FOUND.
*/
svn_error_t *
+svn_client_proplist2 (apr_array_header_t **props,
+ const char *target,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/**
+ * @deprecated Provided for backward compatibility with the 1.1 API.
+ *
+ * Similar to svn_client_proplist2, except that the peg revision is
+ * always left as @c svn_opt_revision_unspecified.
+ */
+svn_error_t *
svn_client_proplist (apr_array_header_t **props,
- const char *target,
+ const char *target,
const svn_opt_revision_t *revision,
svn_boolean_t recurse,
svn_client_ctx_t *ctx,
@@ -1349,9 +1415,16 @@
apr_pool_t *pool);
-/** Set @a *dirents to a newly allocated hash of entries for @a path_or_url
- * at @a revision.
+/**
+ * @since New in 1.2.
*
+ * Set @a *dirents to a newly allocated hash of entries for @a
+ * path_or_url at @a revision. The actual node revision to read is
+ * located by @a peg_revision, history is then traced forward or
+ * backwards to @a revision. The default @a peg_revision is @c
+ * svn_opt_revision_head for URLs or @c svn_opt_revision_working for
+ * WC paths.
+ *
* If @a path_or_url is a directory, return all dirents in the hash. If
* @a path_or_url is a file, return only the dirent for the file. If @a
* path_or_url is non-existent, return @c SVN_ERR_FS_NOT_FOUND.
@@ -1366,6 +1439,21 @@
* be a recursive operation.
*/
svn_error_t *
+svn_client_ls2 (apr_hash_t **dirents,
+ const char *path_or_url,
+ svn_opt_revision_t *peg_revision,
+ svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/**
+ * @deprecated Provided for backward compatibility with the 1.1 API.
+ *
+ * Similar to svn_client_ls2 except that the peg revision is always
+ * left as @c svn_opt_revision_unspecified.
+ */
+svn_error_t *
svn_client_ls (apr_hash_t **dirents,
const char *path_or_url,
svn_opt_revision_t *revision,
@@ -1374,9 +1462,16 @@
apr_pool_t *pool);
-/** Output the content of file identified by @a path_or_url and @a
- * revision to the stream @a out.
+/**
+ * @since New in 1.2.
*
+ * Output the content of file identified by @a path_or_url and @a
+ * revision to the stream @a out. The actual node revision to read is
+ * located by @a peg_revision, history is then traced forward or
+ * backwards to @a revision. The default @a peg_revision is @c
+ * svn_opt_revision_head for URLs or @c svn_opt_revision_working for
+ * WC paths.
+ *
* If @a path_or_url is not a local path, then if @a revision is of
* kind @c svn_opt_revision_previous (or some other kind that requires
* a local path), an error will be returned, because the desired
@@ -1390,6 +1485,21 @@
* ### TODO: Add an expansion/translation flag?
*/
svn_error_t *
+svn_client_cat2 (svn_stream_t *out,
+ const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+
+/**
+ * @deprecated Provided for backward compatibility with the 1.1 API.
+ *
+ * Similar to svn_client_cat2 except that the peg revision is always
+ * left as @c svn_opt_revision_unspecified.
+ */
+svn_error_t *
svn_client_cat (svn_stream_t *out,
const char *path_or_url,
const svn_opt_revision_t *revision,
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 11562)
+++ subversion/libsvn_client/client.h (working copy)
@@ -156,9 +156,9 @@
/* 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.
+ absolute url, a peg revision PEG_REVISON, and a desired 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
@@ -175,6 +175,7 @@
svn_revnum_t *rev_p,
const char **url_p,
const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
const svn_opt_revision_t *revision,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
Index: subversion/libsvn_client/prop_commands.c
===================================================================
--- subversion/libsvn_client/prop_commands.c (revision 11562)
+++ subversion/libsvn_client/prop_commands.c (working copy)
@@ -488,13 +488,14 @@
/* Note: this implementation is very similar to svn_client_proplist. */
svn_error_t *
-svn_client_propget (apr_hash_t **props,
- const char *propname,
- const char *target,
- const svn_opt_revision_t *revision,
- svn_boolean_t recurse,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client_propget2 (apr_hash_t **props,
+ const char *propname,
+ const char *target,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
svn_wc_adm_access_t *adm_access;
const svn_wc_entry_t *node;
@@ -516,8 +517,8 @@
/* Get an RA plugin for this filesystem object. */
SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &revnum,
- &url, target, revision,
- ctx, pool));
+ &url, target, peg_revision,
+ revision, ctx, pool));
SVN_ERR (ra_lib->check_path (session, "", revnum, &kind, pool));
@@ -583,6 +584,23 @@
svn_error_t *
+svn_client_propget (apr_hash_t **props,
+ const char *propname,
+ const char *target,
+ const svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t peg_revision;
+
+ peg_revision.kind = svn_opt_revision_unspecified;
+
+ return svn_client_propget2 (props, propname, target, &peg_revision, revision,
+ recurse, ctx, pool);
+}
+
+svn_error_t *
svn_client_revprop_get (const char *propname,
svn_string_t **propval,
const char *URL,
@@ -832,12 +850,13 @@
/* Note: this implementation is very similar to svn_client_propget. */
svn_error_t *
-svn_client_proplist (apr_array_header_t **props,
- const char *target,
- const svn_opt_revision_t *revision,
- svn_boolean_t recurse,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client_proplist2 (apr_array_header_t **props,
+ const char *target,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
svn_wc_adm_access_t *adm_access;
const svn_wc_entry_t *node;
@@ -859,8 +878,8 @@
/* Get an RA plugin for this filesystem object. */
SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &revnum,
- &url, target, revision,
- ctx, pool));
+ &url, target, peg_revision,
+ revision, ctx, pool));
SVN_ERR (ra_lib->check_path (session, "", revnum, &kind, pool));
@@ -918,6 +937,23 @@
svn_error_t *
+svn_client_proplist (apr_array_header_t **props,
+ const char *target,
+ const svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t peg_revision;
+
+ peg_revision.kind = svn_opt_revision_unspecified;
+
+ return svn_client_proplist2 (props, target, &peg_revision, revision,
+ recurse, ctx, pool);
+}
+
+
+svn_error_t *
svn_client_revprop_list (apr_hash_t **props,
const char *URL,
const svn_opt_revision_t *revision,
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 11562)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -797,6 +797,7 @@
svn_revnum_t *rev_p,
const char **url_p,
const char *path_or_url,
+ const svn_opt_revision_t *peg_revision_p,
const svn_opt_revision_t *revision,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
@@ -805,7 +806,11 @@
void *ra_baton, *session;
svn_ra_plugin_t *ra_lib;
const svn_opt_revision_t *good_rev;
+ svn_opt_revision_t peg_revision, start_rev;
+ svn_opt_revision_t dead_end_rev;
+ svn_opt_revision_t *ignored_rev, *new_rev;
svn_revnum_t rev;
+ const char *ignored_url;
/* Get an RA library for the incoming path. */
SVN_ERR (svn_client_url_from_path (&initial_url, path_or_url, pool));
@@ -816,38 +821,54 @@
SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, initial_url, pool));
+ /* If a peg revision was specified, but a desired revision was not,
+ assume it is the same as the peg revision. */
+ if (revision->kind == svn_opt_revision_unspecified &&
+ peg_revision_p->kind != svn_opt_revision_unspecified)
+ revision = peg_revision_p;
+
if (svn_path_is_url (path_or_url))
{
- /* If an explicit URL was passed in, just use it. */
- good_rev = revision;
- url = initial_url;
+ /* URLs get a default starting rev of HEAD. */
+ if (revision->kind == svn_opt_revision_unspecified)
+ start_rev.kind = svn_opt_revision_head;
+ else
+ start_rev = *revision;
+
+ /* If an explicit URL was passed in, the default peg revision is
+ HEAD. */
+ if (peg_revision_p->kind == svn_opt_revision_unspecified)
+ peg_revision.kind = svn_opt_revision_head;
+ else
+ peg_revision = *peg_revision_p;
}
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_working;
-
+ /* And a default starting rev of 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, ctx, pool));
- good_rev = new_rev;
+
+ /* WC paths have a default peg revision of WORKING. */
+ if (peg_revision_p->kind == svn_opt_revision_unspecified)
+ peg_revision.kind = svn_opt_revision_working;
+ else
+ peg_revision = *peg_revision_p;
}
+
+ dead_end_rev.kind = svn_opt_revision_unspecified;
+
+ /* Run the history function to get the object's (possibly
+ different) url in REVISION. */
+ SVN_ERR (svn_client__repos_locations (&url, &new_rev,
+ &ignored_url, &ignored_rev,
+ /* peg coords are path@BASE: */
+ path_or_url, &peg_revision,
+ /* search range: */
+ &start_rev, &dead_end_rev,
+ ra_lib, ctx, pool));
+ good_rev = new_rev;
SVN_ERR (svn_client__open_ra_session (&session, ra_lib, url,
NULL, NULL, NULL, FALSE, FALSE,
Index: subversion/libsvn_client/cat.c
===================================================================
--- subversion/libsvn_client/cat.c (revision 11562)
+++ subversion/libsvn_client/cat.c (working copy)
@@ -38,11 +38,12 @@
/*** Code. ***/
svn_error_t *
-svn_client_cat (svn_stream_t *out,
- const char *path_or_url,
- const svn_opt_revision_t *revision,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client_cat2 (svn_stream_t *out,
+ const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
svn_ra_plugin_t *ra_lib;
void *session;
@@ -55,8 +56,8 @@
/* Get an RA plugin for this filesystem object. */
SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &rev,
- &url, path_or_url, revision,
- ctx, pool));
+ &url, path_or_url, peg_revision,
+ revision, ctx, pool));
/* Make sure the object isn't a directory. */
SVN_ERR (ra_lib->check_path (session, "", rev, &url_kind, pool));
@@ -139,3 +140,18 @@
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_client_cat (svn_stream_t *out,
+ const char *path_or_url,
+ const svn_opt_revision_t *revision,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t peg_revision;
+
+ peg_revision.kind = svn_opt_revision_unspecified;
+
+ return svn_client_cat2 (out, path_or_url, &peg_revision, revision,
+ ctx, pool);
+}
Index: subversion/libsvn_client/ls.c
===================================================================
--- subversion/libsvn_client/ls.c (revision 11562)
+++ subversion/libsvn_client/ls.c (working copy)
@@ -76,12 +76,13 @@
}
svn_error_t *
-svn_client_ls (apr_hash_t **dirents,
- const char *path_or_url,
- svn_opt_revision_t *revision,
- svn_boolean_t recurse,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client_ls2 (apr_hash_t **dirents,
+ const char *path_or_url,
+ svn_opt_revision_t *peg_revision,
+ svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
svn_ra_plugin_t *ra_lib;
void *session;
@@ -91,8 +92,8 @@
/* Get an RA plugin for this filesystem object. */
SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &rev,
- &url, path_or_url, revision,
- ctx, pool));
+ &url, path_or_url, peg_revision,
+ revision, ctx, pool));
/* Decide if the URL is a file or directory. */
SVN_ERR (ra_lib->check_path (session, "", rev, &url_kind, pool));
@@ -142,3 +143,19 @@
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_client_ls (apr_hash_t **dirents,
+ const char *path_or_url,
+ svn_opt_revision_t *revision,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t peg_revision;
+
+ peg_revision.kind = svn_opt_revision_unspecified;
+
+ return svn_client_ls2 (dirents, path_or_url, &peg_revision,
+ revision, recurse, ctx, pool);
+}
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 11562)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -505,13 +505,14 @@
struct file_rev_baton *frb);
svn_error_t *
-svn_client_blame (const char *target,
- const svn_opt_revision_t *start,
- const svn_opt_revision_t *end,
- svn_client_blame_receiver_t receiver,
- void *receiver_baton,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client_blame2 (const char *target,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *start,
+ const svn_opt_revision_t *end,
+ svn_client_blame_receiver_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
struct file_rev_baton frb;
svn_ra_plugin_t *ra_lib;
@@ -531,7 +532,7 @@
/* Get an RA plugin for this filesystem object. */
SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &end_revnum,
- &url, target, end,
+ &url, target, peg_revision, end,
ctx, pool));
SVN_ERR (svn_client__get_revision_number (&start_revnum, ra_lib, session,
@@ -623,6 +624,24 @@
}
+svn_error_t *
+svn_client_blame (const char *target,
+ const svn_opt_revision_t *start,
+ const svn_opt_revision_t *end,
+ svn_client_blame_receiver_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t peg_revision;
+
+ peg_revision.kind = svn_opt_revision_unspecified;
+
+ return svn_client_blame2 (target, &peg_revision, start, end,
+ receiver, receiver_baton, ctx, pool);
+}
+
+
/* This is used when there is no get_file_revs available. */
static svn_error_t *
old_blame (const char *target, const char *url,
Index: subversion/clients/cmdline/cat-cmd.c
===================================================================
--- subversion/clients/cmdline/cat-cmd.c (revision 11562)
+++ subversion/clients/cmdline/cat-cmd.c (working copy)
@@ -59,11 +59,19 @@
for (i = 0; i < targets->nelts; i++)
{
const char *target = ((const char **) (targets->elts))[i];
+ const char *truepath;
+ svn_opt_revision_t peg_revision;
svn_pool_clear (subpool);
SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
- SVN_ERR (svn_client_cat (out, target, &(opt_state->start_revision),
- ctx, subpool));
+
+ /* Get peg revisions. */
+ SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
+ subpool));
+
+ SVN_ERR (svn_client_cat2 (out, truepath, &peg_revision,
+ &(opt_state->start_revision),
+ ctx, subpool));
}
svn_pool_destroy (subpool);
Index: subversion/clients/cmdline/ls-cmd.c
===================================================================
--- subversion/clients/cmdline/ls-cmd.c (revision 11562)
+++ subversion/clients/cmdline/ls-cmd.c (working copy)
@@ -140,11 +140,18 @@
{
apr_hash_t *dirents;
const char *target = ((const char **) (targets->elts))[i];
+ const char *truepath;
+ svn_opt_revision_t peg_revision;
SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
- SVN_ERR (svn_client_ls (&dirents, target, &(opt_state->start_revision),
- opt_state->recursive, ctx, subpool));
+ /* Get peg revisions. */
+ SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
+ pool));
+ SVN_ERR (svn_client_ls2 (&dirents, truepath, &peg_revision,
+ &(opt_state->start_revision),
+ opt_state->recursive, ctx, subpool));
+
SVN_ERR (print_dirents (dirents, opt_state->verbose, ctx, subpool));
svn_pool_clear (subpool);
}
Index: subversion/clients/cmdline/blame-cmd.c
===================================================================
--- subversion/clients/cmdline/blame-cmd.c (revision 11562)
+++ subversion/clients/cmdline/blame-cmd.c (working copy)
@@ -138,6 +138,9 @@
{
svn_error_t *err;
const char *target = ((const char **) (targets->elts))[i];
+ const char *truepath;
+ svn_opt_revision_t peg_revision;
+
svn_pool_clear (subpool);
SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
if (is_head_or_base)
@@ -147,13 +150,19 @@
else
opt_state->end_revision.kind = svn_opt_revision_base;
}
- err = svn_client_blame (target,
- &opt_state->start_revision,
- &opt_state->end_revision,
- blame_receiver,
- &bl,
- ctx,
- subpool);
+
+ /* Check for a peg revision. */
+ SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
+ subpool));
+
+ err = svn_client_blame2 (truepath,
+ &peg_revision,
+ &opt_state->start_revision,
+ &opt_state->end_revision,
+ blame_receiver,
+ &bl,
+ ctx,
+ subpool);
if (err)
{
if (err->apr_err == SVN_ERR_CLIENT_IS_BINARY_FILE)
Index: subversion/clients/cmdline/propget-cmd.c
===================================================================
--- subversion/clients/cmdline/propget-cmd.c (revision 11562)
+++ subversion/clients/cmdline/propget-cmd.c (working copy)
@@ -143,13 +143,21 @@
apr_hash_index_t *hi;
svn_boolean_t print_filenames = FALSE;
svn_boolean_t is_url = svn_path_is_url (target);
+ const char *truepath;
+ svn_opt_revision_t peg_revision;
svn_pool_clear (subpool);
SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
- SVN_ERR (svn_client_propget (&props, pname_utf8, target,
- &(opt_state->start_revision),
- opt_state->recursive, ctx, subpool));
+
+ /* Check for a peg revision. */
+ SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
+ subpool));
+ SVN_ERR (svn_client_propget2 (&props, pname_utf8, truepath,
+ &peg_revision,
+ &(opt_state->start_revision),
+ opt_state->recursive, ctx, subpool));
+
/* Any time there is more than one thing to print, or where
the path associated with a printed thing is not obvious,
we'll print filenames. That is, unless we've been told
Index: subversion/clients/cmdline/proplist-cmd.c
===================================================================
--- subversion/clients/cmdline/proplist-cmd.c (revision 11562)
+++ subversion/clients/cmdline/proplist-cmd.c (working copy)
@@ -108,12 +108,19 @@
int j;
svn_error_t *err;
svn_boolean_t is_url = svn_path_is_url (target);
+ const char *truepath;
+ svn_opt_revision_t peg_revision;
svn_pool_clear (subpool);
SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
- err = svn_client_proplist (&props, target,
- &(opt_state->start_revision),
- opt_state->recursive, ctx, subpool);
+
+ /* Check for a peg revision. */
+ SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
+ subpool));
+
+ err = svn_client_proplist2 (&props, truepath, &peg_revision,
+ &(opt_state->start_revision),
+ opt_state->recursive, ctx, subpool);
if (err)
{
if (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c (revision 11562)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -185,12 +185,12 @@
{ "blame", svn_cl__blame, {"praise", "annotate", "ann"},
N_("Output the content of specified files or\n"
"URLs with revision and author information in-line.\n"
- "usage: blame TARGET...\n"),
+ "usage: blame TARGET[@REV]...\n"),
{'r', 'v', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
{ "cat", svn_cl__cat, {0},
N_("Output the content of specified files or URLs.\n"
- "usage: cat TARGET...\n"),
+ "usage: cat TARGET[@REV]...\n"),
{'r', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
{ "checkout", svn_cl__checkout, {"co"}, N_
@@ -339,7 +339,7 @@
{ "list", svn_cl__ls, {"ls"},
N_("List directory entries in the repository.\n"
- "usage: list [TARGET...]\n"
+ "usage: list [TARGET[@REV]...]\n"
"\n"
" List each TARGET file and the contents of each TARGET directory as\n"
" they exist in the repository. If TARGET is a working copy path, "
@@ -475,7 +475,7 @@
{ "propget", svn_cl__propget, {"pget", "pg"},
N_("Print value of PROPNAME on files, dirs, or revisions.\n"
- "usage: 1. propget PROPNAME [PATH...]\n"
+ "usage: 1. propget PROPNAME [PATH[@REV]...]\n"
" 2. propget PROPNAME --revprop -r REV [URL]\n"
"\n"
" 1. Prints versioned prop in working copy.\n"
@@ -492,7 +492,7 @@
{ "proplist", svn_cl__proplist, {"plist", "pl"},
N_("List all properties on files, dirs, or revisions.\n"
- "usage: 1. proplist [PATH...]\n"
+ "usage: 1. proplist [PATH[@REV]...]\n"
" 2. proplist --revprop -r REV [URL]\n"
"\n"
" 1. Lists versioned props in working copy.\n"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Tue, 2004-11-02 at 12:43, Julian Foad wrote:
>
>>Is this part of your proposal:
>>
>> PEG defaults to the current working version if any TARGET is a working copy
>>path, otherwise it defaults to HEAD.
Oops - I copied and pasted the second half of that sentence from the
description of ":M" in "svn help diff", without thinking about its new meaning.
> I believe PEG is separate for each target, so that's an odd way of
> putting it.
>
>> M defaults to PEG.
>
> ... which means M could vary per target. But I think that's fine; each
> target is a completely separate diff operation.
Huh. I wasn't making a proposal, I was trying to get you to clarify what your
proposal was.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-11-02 at 12:43, Julian Foad wrote:
> Is this part of your proposal:
>
> PEG defaults to the current working version if any TARGET is a working copy
> path, otherwise it defaults to HEAD.
I believe PEG is separate for each target, so that's an odd way of
putting it.
> M defaults to PEG.
... which means M could vary per target. But I think that's fine; each
target is a completely separate diff operation.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> > The rule you state should be taken with a grain of salt. There's no
>
> That's OK by me if others agree.
I agree with Greg. In order to fix legitimate useability bugs, we may
have to subtly change the meaning of certain syntaxes. Of course, we
have to use our judgement about when to do so (and we are).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Tue, 2004-11-02 at 08:50, Julian Foad wrote:
>>Objection: I don't think you can change the meaning of existing commands before
>>version 2.
>
> You mean, like we changed the meaning of "svn diff -rN filename" in svn
> 1.1 when we added history-tracking?
>
> The rule you state should be taken with a grain of salt. There's no
That's OK by me if others agree.
In that case, please could you state the proposal a bit more completely, say as
a patch to "svn help diff" and "svn help merge"?
The current help for "diff" isn't complete:
> diff (di): Display the differences between two paths.
> usage: 1. diff [-r N[:M]] [TARGET[@REV]...]
> 2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \
> [PATH...]
> 3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV]
>
> 1. Display the changes made to TARGETs as they are seen in REV between
> two revisions. TARGETs may be working copy paths or URLs.
>
> N defaults to BASE if any TARGET is a working copy path, otherwise it
> must be specified. M defaults to the current working version if any
> TARGET is a working copy path, otherwise it defaults to HEAD.
This doesn't say what PEG defaults to.
Is this part of your proposal:
PEG defaults to the current working version if any TARGET is a working copy
path, otherwise it defaults to HEAD.
M defaults to PEG.
?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-11-02 at 08:50, Julian Foad wrote:
> Objection: I don't think you can change the meaning of existing commands before
> version 2.
You mean, like we changed the meaning of "svn diff -rN filename" in svn
1.1 when we added history-tracking?
The rule you state should be taken with a grain of salt. There's no
excuse for changing an API function's signature so that every program
which uses the function suddenly fails to compile, but there are plenty
of good reasons to change the semantics of a particular call to be
better. Same for a command-line syntax.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Julian Foad <ju...@btopenworld.com>.
Josh Pieper wrote:
> Does anyone have objections to this change? Namely defaulting the
> ending rev of a diff to the peg revision if no end revision is
> present? I think it makes sense, so I'll go ahead and make it if no
> one objects.
You mean to do this for "merge" as well as "diff", don't you? It would be
confusing to make them different from one another.
Objection: I don't think you can change the meaning of existing commands before
version 2.
(When you add support for "@REV" to commands that didn't support it before, you
can have it mean whatever you like.)
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-11-02 at 10:17, C. Michael Pilato wrote:
> All subcommands which operate on specific revisions of objects
> accept the 'peg revision' syntax, which looks like ...
>
> Of course, there are exceptions to this. ..."
>
> GACK! Exceptions! Why in Heaven's name must there always be
> exceptions?!
>
> I'm not quite -1 (reeeeeeeeally close), but I canNOT stress enough the
> importance of consistency across the UI of a command-line tool.
I was proposing a change which would bring greater consistency, not
less. Can you be more specific in your objection? I'll be more
specific in my argument:
Assumption: "svn cat target@PEG" should be equivalent to "svn cat -r PEG
target@PEG". (This is already true.)
Conclusion: Therefore, "svn diff -rN target@PEG" should be equivalent to
"svn diff -rN:PEG target@PEG". (This is not already true; the upper
operative revision defaults to HEAD instead of PEG.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by "C. Michael Pilato" <cm...@collab.net>.
Josh Pieper <jj...@pobox.com> writes:
> > This defaulting would be consistent with what you've done with commands
> > like cat in your patch.
>
> Does anyone have objections to this change? Namely defaulting the
> ending rev of a diff to the peg revision if no end revision is
> present? I think it makes sense, so I'll go ahead and make it if no
> one objects.
"In Subversion, our subcommand targets are either local paths or remote
URLs. Because objects can be renamed throughout time, we use the
concept of a 'peg revision' to ...
All subcommands which operate on specific revisions of objects
accept the 'peg revision' syntax, which looks like ...
Of course, there are exceptions to this. ..."
GACK! Exceptions! Why in Heaven's name must there always be
exceptions?!
I'm not quite -1 (reeeeeeeeally close), but I canNOT stress enough the
importance of consistency across the UI of a command-line tool. Let
the GUIs choose different defaults for different subcommands, because
they *show those defaults* via UI widgets *every time* someone
performs that action.
Please. URL peg revisions default to HEAD. Working copy path peg
revisions default to WORKING. Period. You've even got code to figure
this out for you when you pass in "unspecified" peg revisions. Yes,
this may cause folks to have to think a little bit if their URL has
been renamed or removed from HEAD -- but the inconsistency is a higher
price to pay in my opinion.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Josh Pieper <jj...@pobox.com>.
Greg Hudson wrote:
> Just to raise another little issue, I think
>
> svn diff -r N TARGET@PEG
>
> should default the upper operative revision to PEG. Right now we
> default it without regards to the peg revision, so a command like:
>
> svn diff -r 7303 http://svn.collab.net/repos/svn/trunk/subversion/libsvn_fs/id.c@7304
>
> fails because it it tries to do a forward-lookup of
> subversion/libsvn_fs/id.c from peg-rev 7304 to head.
>
> This defaulting would be consistent with what you've done with commands
> like cat in your patch.
Does anyone have objections to this change? Namely defaulting the
ending rev of a diff to the peg revision if no end revision is
present? I think it makes sense, so I'll go ahead and make it if no
one objects.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> Usage: svn cat [-r N] TARGET[@REV]
>
> Displays the contents of TARGET as of revision N. If specified,
> REV controls where the target is looked up. If N is not
> specified, it defaults to REV if given, or to HEAD for URLs or the
> current working copy version for working copy paths.
>
> ... which raises another little side issue: right now "svn cat wcpath"
> seems to display the base text of wcpath, not the current text. This is
> inconsistent with our other commands. (Yeah, I know, it's anybody's
> guess why you'd want to use "svn cat" instead of just "cat" or "type"
> for that, but that's no excuse for second-guessing.)
You could raise this as a separate issue. It's closely related to the fact
that "svn cat" doesn't know how to display a local file - issue #1191 and issue
#1361.
But, then, does Josh's patch describe the behaviour of svn_client_cat correctly?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Greg Hudson <gh...@MIT.EDU>.
Just to raise another little issue, I think
svn diff -r N TARGET@PEG
should default the upper operative revision to PEG. Right now we
default it without regards to the peg revision, so a command like:
svn diff -r 7303 http://svn.collab.net/repos/svn/trunk/subversion/libsvn_fs/id.c@7304
fails because it it tries to do a forward-lookup of
subversion/libsvn_fs/id.c from peg-rev 7304 to head.
This defaulting would be consistent with what you've done with commands
like cat in your patch.
On Sat, 2004-10-23 at 06:32, Josh Pieper wrote:
> 1. Is this desirable in the first place? Adding tracing by default to
> sub-commands makes the most intuitive way to specify a specific
> revision, 'svn cat -r x URL', perform history tracing from HEAD.
> This could possibly take a long time, especially with the new authz
> security fixes. You would need to use 'svn cat URL@r' to get the
> same quick response as before.
I think it's desirable. Tracing doesn't generally take more than a few
seconds.
> 2. There are other sub-commands that reference specific revisions of a
> file/directory but don't currently perform any tracing. (copy,
> export, checkout). Should they be modified to use the new syntax
> and semantics?
Ideally, yes. Users should be able to rely on consistent meanings of
url@N and -r N url.
> 3. How do we describe this in help messages? My attached patch punts
> on that question, and just shows the extra possible field. In my
> attempts to describe peg revisions in the docstrings of the
> libsvn_client functions, I was unable to find a simple wording that
> would be easily understandable in the limited space of a help message.
Maybe this is too subtle for the help output. We already seem to punt
on describing the peg-rev in the case of diff. But I can suggest
wording like:
Usage: svn cat [-r N] TARGET[@REV]
Displays the contents of TARGET as of revision N. If specified,
REV controls where the target is looked up. If N is not
specified, it defaults to REV if given, or to HEAD for URLs or the
current working copy version for working copy paths.
... which raises another little side issue: right now "svn cat wcpath"
seems to display the base text of wcpath, not the current text. This is
inconsistent with our other commands. (Yeah, I know, it's anybody's
guess why you'd want to use "svn cat" instead of just "cat" or "type"
for that, but that's no excuse for second-guessing.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by "C. Michael Pilato" <cm...@collab.net>.
Josh Pieper <jj...@pobox.com> writes:
> Currently diff and merge accept a "peg revision", it gives the history
> tracing algorithm a place to start when looking for the actual node
> revisions to operate on. Other sub-commands that perform history
> tracing (cat, ls, propget, ...) don't accept peg revisions, and in
> fact don't currently perform history tracing for URLs, only for
> working copy paths.
>
> This is an inconsistency in our UI that probably should be resolved.
> I have a patch (attached) to add peg revisions to commands that used
> to trace history but didn't take the peg revision.
In my design for peg and range revisions, I intended for all *all*
subcommands which accepted paths or URLs to recognize the PATH@PEG
syntax -- not just diff and merge. Until this is true, I consider the
feature/fix only partially complete.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Josh Pieper <jj...@pobox.com>.
> >--------------------------------------
> >
> >Add peg-revision options to blame, cat, ls, propget, and proplist. As
>
> propget/list but not propset/edit/del? That sounds like a recipe for
> confusion. Is there a technical reason for this or have you just not done
> the others yet?
Yes, there was a reason, namely that propset, edit, and del (when not
operating on revprops) cannot operate on any revision other than
HEAD. When using revprops, no history tracing is necessary since they
are not operating on a specific filesystem object. I do like the
consistency argument though, so while the functionality would be of
limited utility, I will try and add support for peg revisions to those
commands as well.
> What happens if one of these commands is asked to search forward in
> history? Especially if there are copies.
All history tracing is done with the same function,
svn_client__ra_lib_from_path. When forward tracing, it basically just
tries the same path in the future and checks to see if it is the same
node. If so, it returns success, otherwise it returns an error. The
assumption is that when better forward history tracing is available,
it can be dropped in place.
> >Index: subversion/libsvn_client/client.h
> >===================================================================
> >--- subversion/libsvn_client/client.h (revision 11562)
> >+++ subversion/libsvn_client/client.h (working copy)
> >@@ -156,9 +156,9 @@
> >
> >
> > /* 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.
> >+ absolute url, a peg revision PEG_REVISON, and a desired revision
> >+ REVISION, create an RA connection to that object as it exists in
> >+ that revision, following copy history if necessary.
>
> I think this needs to say what happens if it is asked to follow history
> forwards, and so do all of the public functions that use it.
The problem is that the forward history tracing behavior is not
implemented in this function, it is instead implemented by the server
using the RA get_locations callback. I will try and clarify it
according to how all current code paths can operate.
> >Index: subversion/libsvn_client/ra.c
> >===================================================================
> >--- subversion/libsvn_client/ra.c (revision 11562)
> >+++ subversion/libsvn_client/ra.c (working copy)
> >@@ -797,6 +797,7 @@
> > svn_revnum_t *rev_p,
> > const char **url_p,
> > const char *path_or_url,
> >+ const svn_opt_revision_t *peg_revision_p,
> > const svn_opt_revision_t *revision,
> > svn_client_ctx_t *ctx,
> > apr_pool_t *pool)
>
> >@@ -816,38 +821,54 @@
> > SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
> > SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, initial_url, pool));
> >
> >+ /* If a peg revision was specified, but a desired revision was not,
> >+ assume it is the same as the peg revision. */
>
> Is this necessary? If so, it should be documented.
>
> It is generally better to start with a tight API (e.g. requiring that
> "desired" be specified if "peg" is) as you can then loosen it later without
> breaking compatibility, if such loosening of the interface is found to be
> really useful.
No it is not necessary, but it improves usability a lot in the command
line client, and presumably in all other clients that call as well.
The desired revision has to default to something. Previously it would
default to HEAD for URLs or WORKING for WC path. If a peg revision is
present, that makes a much more sane default.
> >+ if (revision->kind == svn_opt_revision_unspecified &&
> >+ peg_revision_p->kind != svn_opt_revision_unspecified)
> >+ revision = peg_revision_p;
> >+
> > if (svn_path_is_url (path_or_url))
> > {
> >- /* If an explicit URL was passed in, just use it. */
> >- good_rev = revision;
> >- url = initial_url;
> >+ /* URLs get a default starting rev of HEAD. */
> >+ if (revision->kind == svn_opt_revision_unspecified)
> >+ start_rev.kind = svn_opt_revision_head;
> >+ else
> >+ start_rev = *revision;
> >+
> >+ /* If an explicit URL was passed in, the default peg revision is
> >+ HEAD. */
> >+ if (peg_revision_p->kind == svn_opt_revision_unspecified)
> >+ peg_revision.kind = svn_opt_revision_head;
> >+ else
> >+ peg_revision = *peg_revision_p;
> > }
> > 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_working;
> >-
> >+ /* And a default starting rev of 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, ctx, pool));
> >- good_rev = new_rev;
> >+
> >+ /* WC paths have a default peg revision of WORKING. */
> >+ if (peg_revision_p->kind == svn_opt_revision_unspecified)
> >+ peg_revision.kind = svn_opt_revision_working;
> >+ else
> >+ peg_revision = *peg_revision_p;
> > }
> >+
> >+ dead_end_rev.kind = svn_opt_revision_unspecified;
> >+
> >+ /* Run the history function to get the object's (possibly
> >+ different) url in REVISION. */
> >+ SVN_ERR (svn_client__repos_locations (&url, &new_rev,
> >+ &ignored_url, &ignored_rev,
> >+ /* peg coords are path@BASE: */
> >+ path_or_url, &peg_revision,
>
> Comment on previous line: not "@BASE" any more, is it?
Will fix.
Thanks for reviewing this Julian!
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Josh Pieper <jj...@pobox.com>.
C. Michael Pilato wrote:
> Okay, so, actually, I could probably be convinced that if there was a
> rule *across ALL subcommands* whereby a -r[X:]Y was present, and there
> was not an explicit peg revision noted, that the default peg revision
> changes to Y instead of HEAD or WORKING. That kind of one-line
> exception that is universal in its application is soooooooooo much
> more acceptable to me than per-subcommand exceptions.
Actually, the proposed exception was that if a peg reivision was
present, but the end destination revision was not, then the end
destination revision would default to the peg revision. That would
allow you to do things like:
svn diff -rOLD_REV URL@NEW_REV
Where the diff generated would be from OLD_REV to NEW_REV. Currently
to get that behavior you need to specify:
svn diff -rOLD_REV:NEW_REV URL@NEW_REV
This would be the same across all commands that accept a revision
range, so consistency would not be a problem.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by "C. Michael Pilato" <cm...@collab.net>.
Julian Foad <ju...@btopenworld.com> writes:
> > Adding tracing by default to
> > sub-commands makes the most intuitive way to specify a specific
> > revision, 'svn cat -r x URL', perform history tracing from HEAD.
> > This could possibly take a long time, especially with the new authz
> > security fixes. You would need to use 'svn cat URL@r' to get the
> > same quick response as before.
>
> Not necessarily. 'svn cat -r x URL' doesn't have to mean 'URL@HEAD'.
> We could make it mean 'URL@x'. This would be backward-compatible. If
> fact, you could argue that such compatibility is essential, even if it
> isn't the "nicest" meaning.
Or you could argue that we've had a bug all this time that prevented
folks from seeing the thing they were supposed to be seeing, and we've
finally fixed it.
> propget/list but not propset/edit/del? That sounds like a recipe for
> confusion. Is there a technical reason for this or have you just not
> done the others yet?
Don't get me started on recipes for confusion, Mr. "-rX doesn't have
to mean URL@HEAD; it could mean URL@X". :-)
Okay, so, actually, I could probably be convinced that if there was a
rule *across ALL subcommands* whereby a -r[X:]Y was present, and there
was not an explicit peg revision noted, that the default peg revision
changes to Y instead of HEAD or WORKING. That kind of one-line
exception that is universal in its application is soooooooooo much
more acceptable to me than per-subcommand exceptions.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Should all sub-commands accept a peg-revision?
Posted by Julian Foad <ju...@btopenworld.com>.
Josh, good work. Sorry I didn't review it earlier. It's a bit big, so I was
waiting for someone else to do it!
Josh Pieper wrote:
> 1. Is this desirable in the first place?
Yes.
> Adding tracing by default to
> sub-commands makes the most intuitive way to specify a specific
> revision, 'svn cat -r x URL', perform history tracing from HEAD.
> This could possibly take a long time, especially with the new authz
> security fixes. You would need to use 'svn cat URL@r' to get the
> same quick response as before.
Not necessarily. 'svn cat -r x URL' doesn't have to mean 'URL@HEAD'. We could
make it mean 'URL@x'. This would be backward-compatible. If fact, you could
argue that such compatibility is essential, even if it isn't the "nicest" meaning.
> --------------------------------------
>
> Add peg-revision options to blame, cat, ls, propget, and proplist. As
propget/list but not propset/edit/del? That sounds like a recipe for
confusion. Is there a technical reason for this or have you just not done the
others yet?
What happens if one of these commands is asked to search forward in history?
Especially if there are copies.
More comments below.
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 11562)
> +++ subversion/include/svn_client.h (working copy)
> @@ -792,9 +792,18 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
>
> -/** Invoke @a receiver with @a receiver_baton on each line-blame item
> +/**
> + * @since New in 1.2.
> + *
> + * Invoke @a receiver with @a receiver_baton on each line-blame item
> * associated with revision @a end of @a path_or_url, using @a start
> - * as the default source of all blame.
> + * as the default source of all blame. @a peg_revision indicates
> + * which in which revision @a path_or_url is valid. The actual file
Too many "which"s. It must be Hallowe'en.
(Actually I make a distinction between the pronunciation of "which" and
"witch", but the pun still works ... lamely.)
> Index: subversion/libsvn_client/client.h
> ===================================================================
> --- subversion/libsvn_client/client.h (revision 11562)
> +++ subversion/libsvn_client/client.h (working copy)
> @@ -156,9 +156,9 @@
>
>
> /* 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.
> + absolute url, a peg revision PEG_REVISON, and a desired revision
> + REVISION, create an RA connection to that object as it exists in
> + that revision, following copy history if necessary.
I think this needs to say what happens if it is asked to follow history
forwards, and so do all of the public functions that use it.
> Index: subversion/libsvn_client/ra.c
> ===================================================================
> --- subversion/libsvn_client/ra.c (revision 11562)
> +++ subversion/libsvn_client/ra.c (working copy)
> @@ -797,6 +797,7 @@
> svn_revnum_t *rev_p,
> const char **url_p,
> const char *path_or_url,
> + const svn_opt_revision_t *peg_revision_p,
> const svn_opt_revision_t *revision,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> @@ -816,38 +821,54 @@
> SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
> SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, initial_url, pool));
>
> + /* If a peg revision was specified, but a desired revision was not,
> + assume it is the same as the peg revision. */
Is this necessary? If so, it should be documented.
It is generally better to start with a tight API (e.g. requiring that "desired"
be specified if "peg" is) as you can then loosen it later without breaking
compatibility, if such loosening of the interface is found to be really useful.
> + if (revision->kind == svn_opt_revision_unspecified &&
> + peg_revision_p->kind != svn_opt_revision_unspecified)
> + revision = peg_revision_p;
> +
> if (svn_path_is_url (path_or_url))
> {
> - /* If an explicit URL was passed in, just use it. */
> - good_rev = revision;
> - url = initial_url;
> + /* URLs get a default starting rev of HEAD. */
> + if (revision->kind == svn_opt_revision_unspecified)
> + start_rev.kind = svn_opt_revision_head;
> + else
> + start_rev = *revision;
> +
> + /* If an explicit URL was passed in, the default peg revision is
> + HEAD. */
> + if (peg_revision_p->kind == svn_opt_revision_unspecified)
> + peg_revision.kind = svn_opt_revision_head;
> + else
> + peg_revision = *peg_revision_p;
> }
> 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_working;
> -
> + /* And a default starting rev of 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, ctx, pool));
> - good_rev = new_rev;
> +
> + /* WC paths have a default peg revision of WORKING. */
> + if (peg_revision_p->kind == svn_opt_revision_unspecified)
> + peg_revision.kind = svn_opt_revision_working;
> + else
> + peg_revision = *peg_revision_p;
> }
> +
> + dead_end_rev.kind = svn_opt_revision_unspecified;
> +
> + /* Run the history function to get the object's (possibly
> + different) url in REVISION. */
> + SVN_ERR (svn_client__repos_locations (&url, &new_rev,
> + &ignored_url, &ignored_rev,
> + /* peg coords are path@BASE: */
> + path_or_url, &peg_revision,
Comment on previous line: not "@BASE" any more, is it?
> + /* search range: */
> + &start_rev, &dead_end_rev,
> + ra_lib, ctx, pool));
> + good_rev = new_rev;
>
> SVN_ERR (svn_client__open_ra_session (&session, ra_lib, url,
> NULL, NULL, NULL, FALSE, FALSE,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org