[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Should all sub-commands accept a peg-revision?

From: Josh Pieper <jjp_at_pobox.com>
Date: 2004-11-07 17:36:26 CET

> >--------------------------------------
> >
> >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
Received on Sun Nov 7 17:36:54 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.