You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2002/01/29 00:43:32 UTC
svn diff changes in progress
This patch is by no means in committable form,, but I wanted to give
people (Philip Martin especially) a chance to review early, while I'm
still working. Comments welcome:
$ cat log-msg.txt
Start doing things the new svn_client_revision_t way. Of course, this
will changed further when we make diff capable of taking two paths,
but first things first.
This is part of work on issue #422.
* subversion/include/svn_client.h (svn_client_revision_kind): Add
svn_client_revision_working kind.
(svn_client_diff): Change prototype to use two new
svn_client_revision_t start and end parameters, instead of separate
start_revision, start_date, end_revision, end_date params. Also, move
`target' parameter to be immediately after revision params.
* subversion/libsvn_client/diff.c (svn_client_diff): Adjust
accordingly.
* subversion/include/svn_error_codes.h (SVN_ERR_CLIENT_BAD_REVISION):
New error code.
* subversion/clients/cmdline/diff-cmd.c (svn_cl__diff): Add comment
about changes needed here.
* subversion/libsvn_client/revisions.c
(svn_client__get_revision_number): New file, new func.
$ svn st
M ./subversion/clients/cmdline/diff-cmd.c
M ./subversion/include/svn_client.h
M ./subversion/include/svn_error_codes.h
M ./subversion/libsvn_client/client.h
M ./subversion/libsvn_client/diff.c
A ./subversion/libsvn_client/revisions.c
$ svn diff
Index: ./subversion/include/svn_error_codes.h
===================================================================
--- ./subversion/include/svn_error_codes.h
+++ ./subversion/include/svn_error_codes.h Mon Jan 28 18:27:19 2002
@@ -366,6 +366,14 @@
SVN_ERRDEF (SVN_ERR_TEST_FAILED,
"Test failed")
+ /* BEGIN libsvn_client errors */
+
+ SVN_ERRDEF (SVN_ERR_CLIENT_BAD_REVISION,
+ "Bogus revision information given")
+
+ /* END libsvn_client errors */
+
+
/* BEGIN Client errors */
SVN_ERRDEF (SVN_ERR_CL_ARG_PARSING_ERROR,
Index: ./subversion/include/svn_client.h
===================================================================
--- ./subversion/include/svn_client.h
+++ ./subversion/include/svn_client.h Mon Jan 28 18:28:39 2002
@@ -57,12 +57,20 @@
-/*** Various ways of specifying revisions. ***/
+/* Various ways of specifying revisions.
+ *
+ * Note:
+ * In contexts where local mods are relevant, the `working' kind
+ * refers to the uncommitted "working" revision, which may be modified
+ * with respect to its base revision. In other contexts, `working'
+ * should behave the same as `committed' or `current'.
+ */
enum svn_client_revision_kind {
svn_client_revision_unspecified, /* No revision information given. */
svn_client_revision_number, /* revision given as number */
svn_client_revision_date, /* revision given as date */
+ svn_client_revision_working, /* working file, may have local mods */
svn_client_revision_commited, /* rev of most recent change */
svn_client_revision_previous, /* (rev of most recent change) - 1 */
svn_client_revision_current, /* .svn/entries revision */
@@ -473,13 +481,10 @@
/* Given a TARGET which is either a path in the working copy or an URL,
- compare it against the given repository version(s).
-
- START_REVISION/START_DATE and END_REVISION/END_DATE are the two
- repository versions, for each specify either the revision of the
- date. If the two revisions are the different the two repository versions
- are compared. If the two revisions are the same the working copy is
- compared against the repository.
+ compare it against the given repository version(s). START and END
+ are the two revisions to be compared; if either has a `kind' of
+ `svn_client_revision_unspecified', return the error
+ SVN_ERR_CLIENT_BAD_REVISION.
If TARGET is a directory and RECURSE is true, this will be a recursive
operation.
@@ -490,13 +495,11 @@
AUTH_BATON is used to communicate with the repository.
*/
-svn_error_t *svn_client_diff (svn_stringbuf_t *target,
- const apr_array_header_t *diff_options,
+svn_error_t *svn_client_diff (const apr_array_header_t *diff_options,
svn_client_auth_baton_t *auth_baton,
- svn_revnum_t start_revision,
- apr_time_t start_date,
- svn_revnum_t end_revision,
- apr_time_t end_date,
+ svn_client_revision_t *start,
+ svn_client_revision_t *end,
+ svn_stringbuf_t *target,
svn_boolean_t recurse,
apr_pool_t *pool);
Index: ./subversion/libsvn_client/diff.c
===================================================================
--- ./subversion/libsvn_client/diff.c
+++ ./subversion/libsvn_client/diff.c Mon Jan 28 18:36:34 2002
@@ -144,16 +144,15 @@
*/
svn_error_t *
-svn_client_diff (svn_stringbuf_t *path,
- const apr_array_header_t *diff_options,
+svn_client_diff (const apr_array_header_t *diff_options,
svn_client_auth_baton_t *auth_baton,
- svn_revnum_t start_revision,
- apr_time_t start_date,
- svn_revnum_t end_revision,
- apr_time_t end_date,
+ svn_client_revision_t *start,
+ svn_client_revision_t *end,
+ svn_stringbuf_t *path,
svn_boolean_t recurse,
apr_pool_t *pool)
{
+ svn_revnum_t start_rev, end_rev;
svn_string_t path_str;
svn_boolean_t path_is_url;
svn_boolean_t use_admin;
@@ -168,13 +167,14 @@
void *diff_edit_baton;
struct diff_cmd_baton diff_cmd_baton;
- /* If both a revision and a date/time are specified, this is an error.
- They mostly likely contradict one another. */
- if ((SVN_IS_VALID_REVNUM(start_revision) && start_date)
- || (SVN_IS_VALID_REVNUM(end_revision) && end_date))
- return
- svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, 0, NULL, pool,
- "Cannot specify _both_ revision and time.");
+ /* Sanity check. */
+ if ((start->kind == svn_client_revision_unspecified)
+ || (end->kind == svn_client_revision_unspecified))
+ {
+ return svn_error_create
+ (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+ "svn_client_diff: caller failed to specify any revisions");
+ }
diff_cmd_baton.options = diff_options;
diff_cmd_baton.pool = pool;
@@ -201,18 +201,25 @@
URL = svn_stringbuf_create (entry->url->data, pool);
}
- /* ### TODO: awful revision handling */
- if (!path_is_url
- && !SVN_IS_VALID_REVNUM(start_revision) && end_revision == 1
- && !start_date && !end_date)
+ if ((! path_is_url)
+ && ((start->kind == svn_client_revision_committed)
+ || (start->kind == svn_client_revision_current))
+ && (end->kind == svn_client_revision_working))
{
- /* Not an url and no revisions, this is the 'quick' diff that does
- not contact the repository and simply uses the text base */
+ /* This is the 'quick' diff that does not contact the repository
+ and simply uses the text base. */
return svn_wc_diff (anchor,
target,
diff_cmd, &diff_cmd_baton,
recurse,
pool);
+ /* ### todo: see comments issue #422 about how libsvn_client's
+ diff implementation prints to stdout. Apparently same is
+ true for libsvn_wc! Both will need adjusting; strongly
+ suspect that the callback abstraction will be exactly what is
+ needed to make merge share code -- that is, the same library
+ functions will be called, but with different callbacks (maybe
+ svn_delta_edit_fns_t editors?) that Do The Right Thing. */
}
/* Establish the RA session */
@@ -226,6 +233,15 @@
wc, and null otherwise. */
SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL, NULL,
FALSE, FALSE, auth_baton, pool));
+
+ SVN_ERR (svn_client__get_revision_number
+ (ra_lib, session, &start, target->data, &revnum, pool));
+
+ fooo; /* intentionally prevent compilation */
+
+ /* ### todo: use the revnum obtained above to do the right things
+ below. And some of the code below will be used in
+ svn_client__get_revision_number(), it looks like. */
if (start_date)
SVN_ERR (ra_lib->get_dated_revision (session, &start_revision, start_date));
Index: ./subversion/libsvn_client/revisions.c
===================================================================
--- ./subversion/libsvn_client/revisions.c
+++ ./subversion/libsvn_client/revisions.c Mon Jan 28 18:39:59 2002
@@ -0,0 +1,52 @@
+/*
+ * revisions.c: discovering revisions
+ *
+ * ====================================================================
+ * Copyright (c) 2000-2001 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+
+
+#include <apr_pools.h>
+
+#include "svn_error.h"
+#include "svn_string.h"
+#include "svn_ra.h"
+#include "svn_wc.h"
+#include "svn_client.h"
+#include "svn_path.h"
+#include "client.h"
+
+
+
+
+
+svn_error_t *
+svn_client__get_revision_number (svn_ra_plugin_t *ra_lib,
+ void *session,
+ svn_client_revision_t *revision,
+ const char *path,
+ svn_revnum_t *revnum,
+ apr_pool_t *pool)
+{
+ abort ();
+ return SVN_NO_ERROR;
+}
+
+
+
+/*
+ * local variables:
+ * eval: (load-file "../svn-dev.el")
+ * end: */
Index: ./subversion/libsvn_client/client.h
===================================================================
--- ./subversion/libsvn_client/client.h
+++ ./subversion/libsvn_client/client.h Mon Jan 28 17:43:26 2002
@@ -30,6 +30,24 @@
#include "svn_client.h"
+
+/* Set *NUMBER to the revision identified by REVISION for PATH.
+ *
+ * If *NUMBER can be identified purely based on the working copy's
+ * administrative information for PATH, then do not contact the
+ * repository; otherwise, use RA_LIB and SESSION to retrieve the
+ * information from the repository.
+ *
+ * Use POOL for any temporary allocation.
+ */
+svn_error_t *svn_client__get_revision_number (svn_ra_plugin_t *ra_lib,
+ void *session,
+ svn_client_revision_t *revision,
+ const char *path,
+ svn_revnum_t *revnum,
+ apr_pool_t *pool);
+
+
/* ---------------------------------------------------------------- */
/*** RA callbacks ***/
Index: ./subversion/clients/cmdline/diff-cmd.c
===================================================================
--- ./subversion/clients/cmdline/diff-cmd.c
+++ ./subversion/clients/cmdline/diff-cmd.c Mon Jan 28 18:32:35 2002
@@ -54,6 +54,18 @@
targets,
pool));
+ /* ### todo: adjust below to match svn_client_diff's new calling
+ discipline. */
+ fooo; /* intentionally prevent compilation */
+
+ /* If both a revision and a date/time are specified, this is an error.
+ They mostly likely contradict one another. */
+ if ((SVN_IS_VALID_REVNUM(start_revision) && start_date)
+ || (SVN_IS_VALID_REVNUM(end_revision) && end_date))
+ return
+ svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, 0, NULL, pool,
+ "Cannot specify _both_ revision and time.");
+
auth_baton = svn_cl__make_auth_baton (opt_state, pool);
for (i = 0; i < condensed_targets->nelts; ++i)
$
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff changes in progress
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> > + /* Sanity check. */
> > + if ((start->kind == svn_client_revision_unspecified)
> > + || (end->kind == svn_client_revision_unspecified))
> > + {
> > + return svn_error_create
> > + (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
> > + "svn_client_diff: caller failed to specify any revisions");
> > + }
>
> This is why I had second thoughts about adding unspecified to the
> client interface. Using unspecified means that every client function
> needs this sort of check, and yet in practice the application will
> never pass in such a revision. Typical application pseudo-code
Not really. Most client code can fall through to the default
"unrecognized revision kind" case, that is, the final `else' in the
chain. Only functions that promise a particular behavior in the case
of `unspecified' (a behavior different from the unrecognized case)
need to have an explicit check.
Whether the above code actually does that I don't know, it's all
ripped up now anyway probably. There is at least one function in my
patch as of right now that promises different behaviors for the two,
however: svn_client__get_revision() will return SVN_INVALID_REVNUM for
`unspecified', but error outright on an unrecognized kind. That seems
like the Right Thing for the internal interface, though I'd be hard
pressed to articulate why. :-)
> Diff is awkward because at present it can take unspecified revisions,
> however I would be happy to see code like
>
> if start_revision is unspecified and end_revision is not unspecified
> start_revision = CURRENT
>
> if start_revision is unspecified and end_revision is unspecified
> svn_diff_text_base_to_wc()
>
> else if end_revision is unspecified
> svn_diff_repos_to_wc( start_revision )
>
> else
> svn_diff_repos_to_repos( start_revision, end_revision )
>
> in the application, so that diff doesn't require unspecified
> revisions.
The svn_client_diff() interface doesn't do anything useful with
`unspecified' revisions, but it promises to error on them as on
unrecognized kinds.
I don't think having `unspecified' directly in the libsvn_client type
actually results in more code, in practice, for the reasons given
above. But it does allow the client library to handle `unspecified'
explicitly in certain cases, which may result in smaller code in the
calling client. So I do think it's better this way.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff changes in progress
Posted by Philip Martin <ph...@codematters.co.uk>.
<kf...@collab.net> writes:
> + /* Sanity check. */
> + if ((start->kind == svn_client_revision_unspecified)
> + || (end->kind == svn_client_revision_unspecified))
> + {
> + return svn_error_create
> + (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
> + "svn_client_diff: caller failed to specify any revisions");
> + }
This is why I had second thoughts about adding unspecified to the
client interface. Using unspecified means that every client function
needs this sort of check, and yet in practice the application will
never pass in such a revision. Typical application pseudo-code
if revision is unspecified
revision = HEAD
svn_client_update( revision )
and so the code in the library
if revision is unspecified
throw error
will never trigger.
Diff is awkward because at present it can take unspecified revisions,
however I would be happy to see code like
if start_revision is unspecified and end_revision is not unspecified
start_revision = CURRENT
if start_revision is unspecified and end_revision is unspecified
svn_diff_text_base_to_wc()
else if end_revision is unspecified
svn_diff_repos_to_wc( start_revision )
else
svn_diff_repos_to_repos( start_revision, end_revision )
in the application, so that diff doesn't require unspecified
revisions.
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org