You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sir Woody Hackswell <si...@hackswell.com> on 2003/10/22 17:36:47 UTC
RFC: (Issue 1213) svn cmd -r PREVn:HEAD
Hello, all!
I was just looking at issue 1213 and the patches that were submitted.
Here is my $0.03:
1. PREVn should be defined as 'n' revisions before COMMITTED. PREV and
PREV1 therefore being the same.
2. The current PREV just takes COMMITED-1, which HAPPENS to get the
previous, but, say in a diff command, the revision number is COMMITTED-1,
not PREV. Sometimes it's nice to know EXACTLY what you are diffing.
THEREFORE...
I propose that PREVn always contacts the repository to get the actual
revision numbers. Could this be done by grabbing the log revisions? Or
would that be too much overhead?
Would we want to re-do all the svn_opt_revision_previous, or add a new type,
and keep the default of assuming PREV == COMMITTED-1? OR... if we are
getting exact revision numbers, set the type to svn_opt_revision_number?
I'm thinking that we could do almost all of this in libsvn_subr/opt.c
If we're doing PREV or PREVn, we still must contact the repository ANYWAY
for the operation we are doing, so why not do the Right Thing(tm) and get
the real revision numers?
Comments? Questions? Flames? Suggestions?
-Richard Balint
-----
Sir.Woody@Hackswell.com http://sir.woody.hackswell.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] libsvn_ra->get_revisions()
Posted by "C. Michael Pilato" <cm...@collab.net>.
Sir Woody Hackswell <si...@hackswell.com> writes:
> I'd like to request an addition to the libsvn_ra API.
>
> I'd like to add a function similar to get_log, but instead of returning
> logs, return revision numbers and paths. These routines follow
> the get_log() interface very closely, but return just the revision numbers
> and paths. (revision node). These routines will always traverse path
> changes.
You are trying to basically marshal the output of the new FS history
code (specifically, the results of svn_fs_history_location) back to
the client, which is a worthy cause. But I have some problems with
several points of this proposal.
> These routines could be used to help diff and log out when
> traversing back among renames. These routines are also required to
> implement the -r PREVn interface I'm working on. But one thing at a
> time, no? :)
I'm not excited in the least about the PREVn concept. But sure, one
thing at a time.
> <svn_ra.h> (and ra_* plugins)
> /** Get at most "maxrevisions" from each path in "paths", starting at
> * "start" and ending at "end", following path changes.
> *
> * Results are returned in the APR array "revisions". Elements are of
> * type svn_revnode_t.
> */
> svn_error_t *(*get_revisions) (void *session_baton,
> const apr_array_header_t *paths,
> svn_revnum_t start,
> svn_revnum_t end,
> int maxrevisions,
> apr_array_header_t *revisions,
> apr_pool_t *pool);
You don't need the END -- your bounded by MAXREVISIONS. And having
only a single START is asking for the same trouble we're running into
with the get_log() interface. You probably a START for each PATH.
Finally, do you really need all the intermediate revisions? If
someone does PREV1000, all you really care about is the result that's
1000 revisions back in time, right? So way make the network marshal
back all the extra info, to be accumulated into a big ol' array by the
RA layer, then only to get discarded by the client?
Further, you have a bunch of PATHS going into this function, and a
bunch of REVISIONS coming out. But there is a 1-to-many mapping of
these things as you've defined the interface. How are you going to
keep track of which REVISIONS go with which PATH, especially since the
paths can change? That's right -- you're not. A better interface
would return a hash, keyed on the original PATH inputs, with arrays of
svn_revnode_t's as values, and probably promise something about the
sorting of those arrays. Or, if you don't need the intermediate
revisions, a hash keyed on original PATH inputs with a single
svn_revnode_t value per key.
> <libsvn_repos/revisions.c>
> svn_error_t *
> svn_repos_get_revisions (svn_repos_t *repos,
> const apr_array_header_t *paths,
> svn_revnum_t start,
> svn_revnum_t end,
> int maxrevisions,
> apr_array_header_t *revisions,
> apr_pool_t *pool);
See above complaints.
You're definitely going to need to more thoroughly outline your PREVn
design ideas and your supposed enhancements to diff and log before
getting anyone to sign off on a brand new API this late in the
ballgame.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[RFC] libsvn_ra->get_revisions()
Posted by Sir Woody Hackswell <si...@hackswell.com>.
I'd like to request an addition to the libsvn_ra API.
I'd like to add a function similar to get_log, but instead of returning
logs, return revision numbers and paths. These routines follow
the get_log() interface very closely, but return just the revision numbers
and paths. (revision node). These routines will always traverse path
changes.
These routines could be used to help diff and log out when traversing back
among renames. These routines are also required to implement the -r PREVn
interface I'm working on. But one thing at a time, no? :)
<svn_types.h>
/** A revision number. */
typedef long int svn_revnum_t;
/** A revision type */
typedef struct svn_revnode_t
{
svn_revnum_t rev;
char *path;
} svn_revnode_t;
<svn_ra.h> (and ra_* plugins)
/** Get at most "maxrevisions" from each path in "paths", starting at
* "start" and ending at "end", following path changes.
*
* Results are returned in the APR array "revisions". Elements are of
* type svn_revnode_t.
*/
svn_error_t *(*get_revisions) (void *session_baton,
const apr_array_header_t *paths,
svn_revnum_t start,
svn_revnum_t end,
int maxrevisions,
apr_array_header_t *revisions,
apr_pool_t *pool);
Each ra->get_revisions() call will end up calling a new function in
libsvn_repos, which has very similar parameters.
**Question: Should "revisions" be allocated by caller, get_revisions, or
the following svn_repos_get_revisions?
<libsvn_repos/revisions.c>
svn_error_t *
svn_repos_get_revisions (svn_repos_t *repos,
const apr_array_header_t *paths,
svn_revnum_t start,
svn_revnum_t end,
int maxrevisions,
apr_array_header_t *revisions,
apr_pool_t *pool);
svn_repos_get_revisions goes to the fs interface, starts with "start", and
goes to "end" or back "maxrevisions" count, whichever comes first.
Again, "revisions" sends back the revnums and paths that
svn_fs_history_location returned.
Thoughts? Questions? Flames? Constructive criticism? Revisions?
-Richard Balint
-----
Sir.Woody@Hackswell.com http://sir.woody.hackswell.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] (Issue 1213) svn cmd -r PREVn:HEAD
Posted by Sir Woody Hackswell <si...@hackswell.com>.
Hello all!
Here's my first attempt at a PREVn patch. It's slow and inefficient, but it
works. :) The patch is against rev 7504. It was gratuitously copied and
enhanced from Mark Grosberg's patch in issue #1213. :)
PREVn should be defined as 'n' revisions before COMMITTED. PREV and
PREV1 are LOGICALLY the same, but PREV (as per previous behavior) was
COMMITTED-1, where PREV1 is the actual previous commit.
Details, caveats, and suckage:
To get a file's revision numbers, I call ra_lib->get_log(), with a start
revision of COMMITTED, and an end revision of 1. THIS SUCKS, but works.
I would like to propose a new ra_lib function or a modification of
get_log(). The new function would be get_revisions(), but would do basically
the same thing as get_log(), but without throwing as much data around.
In either case, get_log() or get_revisions() would take a start revision, an
end revision, and a count as to how many to go back at maximum (0 for going
all the way to the end). This way you can short-circuit out of the call if
you don't need ALL the revisions.
Runtime Examples:
[woody@sir trunk]$ svn diff -r PREV:COMMITTED COPYING
Index: COPYING
===================================================================
--- COPYING (revision 6226) // Note PREV is COMMITTED-1
+++ COPYING (revision 6227)
<<SNIP>>
[woody@sir trunk]$ svn diff -r PREV1:COMMITTED COPYING
Index: COPYING
===================================================================
--- COPYING (revision 4371) // Note this is the actual previous commit
+++ COPYING (revision 6227)
<<SNIP>>
[woody@sir trunk]$ svn diff -r PREV4:PREV2 COPYING
Index: COPYING
===================================================================
--- COPYING (revision 1219)
+++ COPYING (revision 4233)
<<SNIP>>
-Richard Balint
-----
./configure --prefix=/dev/mocha --enable-caffeine --enable-sugar
Sir.Woody@Hackswell.com http://sir.woody.hackswell.com