[First question from the patch review...]
Philip, did you ever get any response to your mail below?
I haven't reviewed it yet; first want to find out if maybe you already
gotten responses and decided not to apply it.
-K
Philip Martin <philip@codematters.co.uk> writes:
> This is a patch for 'svn diff -rREV1:REV2' that replaces the two
> independent RA sessions with a cooperating pair. This allows the REV1
> fulltext to be used as the basis for the REV1 to REV2 diff allowing
> the server to send a diff.
>
> I don't know if this is the right way to implement it, but it appears
> to do what I expect and it passes the tests. There is one possible
> problem, I had to add the SVN_RA_DAV__LP_VSN_URL property to the
> properties hash returned by svn_ra_dav__get_file.
>
> Is this OK to apply?
>
> Log/patch follow:
>
>
> Replace the two independent RA sessions used by 'svn diff -rREV1:REV2'
> with two sessions that cooperate to allow the server to send REV1 to
> REV2 diffs. They allow a file obtained by the RA get_file callback to
> temporarily override the base_dir version.
>
> * subversion/libsvn_client/ra.c
> (get_wc_prop_double_ra): New svn_ra_get_wc_prop_func_t used by the
> cooperating RA sessions.
> (svn_client__open_double_ra_session): New function to open two
> cooperating sessions.
> (svn_client__double_ra_get_file): New function to get an overriding
> REV2 file using the cooperating sessions.
> (svn_client__discard_file_from_double_ra): New function to discard
> the overriding file.
>
> * subversion/libsvn_client/diff.c (svn_client_diff): Use the new
> cooperating RA sessions.
>
> * subversion/libsvn_client/repos_diff.c
> (edit_baton): Add callback_baton for cooperating RA sessions.
> (temp_file_cleanup_s): Add callback_baton to determine whether the
> file needs to be discarded by the cooperating RA sessions.
> (temp_file_plain_cleanup_handler): Discard file if required.
> (temp_file_cleanup_register): Add callback_baton parameter.
> (get_file_from_ra): Set callback_baton in call to temp_file_cleanup_register.
> Call svn_client__double_ra_get_file.
> (get_empty_file): Pass null callback_baton.
> (apply_textdelta) : Pass null callback_baton.
> (svn_client__get_diff_editor): Add callback_baton parameter.
>
> * subversion/libsvn_client/client.h
> (svn_client__callback_baton_t): Add name, props, props_pool to
> support cooperating RA sessions.
> (svn_client__open_double_ra_session, svn_client__double_ra_get_file,
> svn_client__discard_file_from_double_ra) New prototypes.
>
> * subversion/libsvn_ra_dav/fetch.c (svn_ra_dav__get_file): Set
> SVN_RA_DAV__LP_VSN_URL property.
>
>
> Index: ../svn/subversion/libsvn_client/ra.c
> ===================================================================
> --- ../svn/subversion/libsvn_client/ra.c
> +++ ../svn/subversion/libsvn_client/ra.c Thu Jan 24 15:00:52 2002
> @@ -91,6 +91,40 @@
> return svn_wc_get_wc_prop(&ccb, relpath, name, value);
> }
>
> +/* An svn_ra_get_wc_prop_func_t function for the double RA session. If an
> + * overriding file is active then return the property for that file,
> + * otherwise return the normal property for the base_dir version of the
> + * file.
> + */
> +static svn_error_t *
> +get_wc_prop_double_ra(void *baton,
> + const char *relpath,
> + const char *name,
> + const svn_string_t **value)
> +{
> + svn_client__callback_baton_t *cb = baton;
> +
> + /* ### BUG: Need better path comparison!! This is a hack to get round
> + what repos_diff.c is passing in. */
> + if (relpath[0] == '.' && relpath[1] == '/')
> + relpath += 2;
> +
> + if (!cb->name || strcmp (relpath, cb->name))
> + {
> + /* This is not the overriding file */
> + SVN_ERR (get_wc_prop (baton, relpath, name, value));
> + }
> + else
> + {
> + /* Get the property from the overriding file */
> + svn_stringbuf_t *prop = apr_hash_get (cb->props, name,
> + APR_HASH_KEY_STRING);
> + *value = prop ? svn_string_create_from_buf (prop, cb->props_pool) : NULL;
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
>
> svn_error_t * svn_client__open_ra_session (void **session_baton,
> const svn_ra_plugin_t *ra_lib,
> @@ -117,7 +151,69 @@
>
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t * svn_client__open_double_ra_session (void **session_baton1,
> + void **session_baton2,
> + void **callback_baton,
> + const svn_ra_plugin_t *ra_lib,
> + svn_stringbuf_t *repos_URL,
> + svn_stringbuf_t *base_dir,
> + svn_boolean_t do_store,
> + svn_boolean_t use_admin,
> + void *auth_baton,
> + apr_pool_t *pool)
> +{
> + svn_ra_callbacks_t *cbtable = apr_pcalloc (pool, sizeof(*cbtable));
> + svn_client__callback_baton_t *cb = apr_pcalloc (pool, sizeof(*cb));
> +
> + cbtable->open_tmp_file = use_admin ? open_admin_tmp_file : open_tmp_file;
> + cbtable->get_authenticator = svn_client__get_authenticator;
> + cbtable->get_wc_prop = get_wc_prop_double_ra;
> +
> + cb->auth_baton = auth_baton;
> + cb->base_dir = base_dir;
> + cb->do_store = do_store;
> + cb->name = NULL;
> + cb->pool = pool;
> +
> + SVN_ERR (ra_lib->open (session_baton1, repos_URL, cbtable, cb, pool));
> + SVN_ERR (ra_lib->open (session_baton2, repos_URL, cbtable, cb, pool));
> +
> + *callback_baton = cb;
> +
> + return SVN_NO_ERROR;
> +}
>
> +svn_error_t *
> +svn_client__double_ra_get_file (const svn_ra_plugin_t *ra_lib,
> + void *session_baton,
> + void *callback_baton,
> + const char *path,
> + svn_revnum_t revision,
> + svn_stream_t *fstream,
> + apr_pool_t *pool)
> +{
> + svn_client__callback_baton_t *cb = callback_baton;
> +
> + /* Disable any overriding file before requesting the file */
> + cb->name = NULL;
> +
> + SVN_ERR (ra_lib->get_file (session_baton, path, revision, fstream, NULL,
> + &cb->props));
> +
> + /* Enable the overriding file */
> + cb->name = path;
> + cb->props_pool = pool;
> +
> + return SVN_NO_ERROR;
> +}
> +
> +void
> +svn_client__discard_file_from_double_ra (void *callback_baton)
> +{
> + svn_client__callback_baton_t *cb = callback_baton;
> + cb->name = NULL;
> +}
>
>
> /*
> Index: ../svn/subversion/libsvn_client/diff.c
> ===================================================================
> --- ../svn/subversion/libsvn_client/diff.c
> +++ ../svn/subversion/libsvn_client/diff.c Thu Jan 24 03:38:44 2002
> @@ -259,27 +259,22 @@
> {
> /* Pure repository comparison if two revisions are given */
>
> - /* Open a second session used to request individual file
> - contents. Although a session can be used for multiple requests, it
> - appears that they must be sequential. Since the first request, for
> - the diff, is still being processed the first session cannot be
> - reused. This applies to ra_dav, ra_local does not appears to have
> - this limitation. */
> - void *session2;
> + void *session2, *callback_baton;
>
> - /* ### TODO: Forcing the base_dir to null. It might be possible to
> - use a special ra session that cooperates with the editor to enable
> - get_wc_prop to return values when the file is in the wc */
> - SVN_ERR (svn_client__open_ra_session (&session2, ra_lib, URL,
> - NULL,
> - FALSE, FALSE,
> - auth_baton, pool));
> + /* Close the first session and open a double session */
> + SVN_ERR (ra_lib->close (session));
> + SVN_ERR (svn_client__open_double_ra_session (&session, &session2,
> + &callback_baton,
> + ra_lib,
> + URL, NULL, FALSE, FALSE,
> + auth_baton, pool));
>
> SVN_ERR (svn_client__get_diff_editor (target,
> svn_client__diff_cmd,
> &diff_cmd_baton,
> recurse,
> ra_lib, session2,
> + callback_baton,
> start_revision,
> &diff_editor, &diff_edit_baton,
> pool));
> Index: ../svn/subversion/libsvn_client/repos_diff.c
> ===================================================================
> --- ../svn/subversion/libsvn_client/repos_diff.c
> +++ ../svn/subversion/libsvn_client/repos_diff.c Thu Jan 24 15:13:14 2002
> @@ -41,7 +41,7 @@
> /* TARGET represent the base of the hierarchy to be compared. */
> svn_stringbuf_t *target;
>
> - /* The callback and calback argument that implement the file comparison
> + /* The callback and callback argument that implement the file comparison
> function */
> svn_wc_diff_cmd_t diff_cmd;
> void *diff_cmd_baton;
> @@ -55,6 +55,10 @@
> svn_ra_plugin_t *ra_lib;
> void *ra_session;
>
> + /* CALLBACK_BATON is the baton that relates the RA session to the other
> + cooperating one */
> + void *callback_baton;
> +
> /* The rev1 from the '-r Rev1:Rev2' command line option */
> svn_revnum_t revision;
>
> @@ -122,6 +126,8 @@
> svn_stringbuf_t *path;
> /* The pool to which the deletion of the file is linked. */
> apr_pool_t *pool;
> + /* If set this then this file should be discarded by the double ra session */
> + void *callback_baton;
> };
>
> /* Create a new directory baton. NAME is the directory name sans
> @@ -192,6 +198,9 @@
> {
> struct temp_file_cleanup_s *s = arg;
>
> + if (s->callback_baton)
> + svn_client__discard_file_from_double_ra (s->callback_baton);
> +
> return apr_file_remove (s->path->data, s->pool);
> }
>
> @@ -219,10 +228,12 @@
> */
> static svn_error_t *
> temp_file_cleanup_register (svn_stringbuf_t *path,
> + void *callback_baton,
> apr_pool_t *pool)
> {
> struct temp_file_cleanup_s *s = apr_palloc (pool, sizeof (*s));
> s->path = path;
> + s->callback_baton = callback_baton;
> s->pool = pool;
> apr_pool_cleanup_register (s->pool, s, temp_file_plain_cleanup_handler,
> temp_file_child_cleanup_handler);
> @@ -252,13 +263,18 @@
> tmp_name, "", FALSE, b->pool));
>
> /* Install a pool cleanup handler to delete the file */
> - SVN_ERR (temp_file_cleanup_register (b->path_start_revision, b->pool));
> + SVN_ERR (temp_file_cleanup_register (b->path_start_revision,
> + b->edit_baton->callback_baton,
> + b->pool));
>
> fstream = svn_stream_from_aprfile (file, b->pool);
> - SVN_ERR (b->edit_baton->ra_lib->get_file (b->edit_baton->ra_session,
> - b->path->data,
> - b->edit_baton->revision,
> - fstream, NULL, NULL));
> + SVN_ERR (svn_client__double_ra_get_file (b->edit_baton->ra_lib,
> + b->edit_baton->ra_session,
> + b->edit_baton->callback_baton,
> + b->path->data,
> + b->edit_baton->revision,
> + fstream,
> + b->pool));
>
> status = apr_file_close (file);
> if (status)
> @@ -305,7 +321,7 @@
> SVN_ERR (create_empty_file (&b->empty_file, b->pool));
>
> /* Install a pool cleanup handler to delete the file */
> - SVN_ERR (temp_file_cleanup_register (b->empty_file, b->pool));
> + SVN_ERR (temp_file_cleanup_register (b->empty_file, NULL, b->pool));
> }
>
> *empty_file = b->empty_file;
> @@ -543,7 +559,7 @@
> /* Open the file that will become the second revision after applying the
> text delta, it starts empty */
> SVN_ERR (create_empty_file (&b->path_end_revision, b->pool));
> - SVN_ERR (temp_file_cleanup_register (b->path_end_revision, b->pool));
> + SVN_ERR (temp_file_cleanup_register (b->path_end_revision, NULL, b->pool));
> status = apr_file_open (&b->file_end_revision, b->path_end_revision->data,
> APR_WRITE, APR_OS_DEFAULT, b->pool);
> if (status)
> @@ -599,6 +615,7 @@
> svn_boolean_t recurse,
> svn_ra_plugin_t *ra_lib,
> void *ra_session,
> + void *callback_baton,
> svn_revnum_t revision,
> const svn_delta_edit_fns_t **editor,
> void **edit_baton,
> @@ -614,6 +631,7 @@
> eb->recurse = recurse;
> eb->ra_lib = ra_lib;
> eb->ra_session = ra_session;
> + eb->callback_baton = callback_baton;
> eb->revision = revision;
> eb->empty_file = NULL;
> eb->pool = subpool;
> Index: ../svn/subversion/libsvn_client/client.h
> ===================================================================
> --- ../svn/subversion/libsvn_client/client.h
> +++ ../svn/subversion/libsvn_client/client.h Thu Jan 24 12:26:47 2002
> @@ -51,6 +51,13 @@
> /* Record whether we should store the user/pass into the WC */
> svn_boolean_t do_store;
>
> + /* If this is one of a double RA session then NAME is the relative URL of
> + a file obtained over one of the sessions. PROPS is the properties
> + associated with the file. POOL is a pool associated with the file. */
> + const char *name;
> + apr_hash_t *props;
> + apr_pool_t *props_pool;
> +
> /* The pool to use for session-related items. */
> apr_pool_t *pool;
>
> @@ -87,6 +94,78 @@
> apr_pool_t *pool);
>
>
> +/* Open two cooperating RA sessions. They are intended to be used as
> + follows: one of the sessions is used to make an RA request. This
> + request is used to drive an editor, and the editor can use the other
> + session to request a file using svn_client__double_ra_get_file. The
> + properties for this file will then override the properties visible in
> + base_dir.
> +
> + SESSION_BATON1, SESSION_BATON2 are the two RA sessions opened. Either
> + can be used to make the initial request or to get overriding files.
> +
> + RA_LIB is the RA library to use to open the sessions.
> +
> + BASE_DIR/REPOS_URL are the root of the session.
> +
> + DO_STORE indicates whether the RA layer should store authentication info.
> +
> + USE_ADMIN indicates that the RA layer should create tempfiles in the admin
> + area instead of the working copy.
> +
> + BASE_DIR may be NULL if the RA operation does not correspond to a
> + working copy (in which case, DO_STORE and USE_ADMIN should both be
> + FALSE).
> +
> + AUTH_BATON is used to carry out application authentication.
> + */
> +svn_error_t * svn_client__open_double_ra_session (void **session_baton1,
> + void **session_baton2,
> + void **callback_baton,
> + const svn_ra_plugin_t *ra_lib,
> + svn_stringbuf_t *repos_URL,
> + svn_stringbuf_t *base_dir,
> + svn_boolean_t do_store,
> + svn_boolean_t use_admin,
> + void *auth_baton,
> + apr_pool_t *pool);
> +
> +
> +/* Use one of the sessions opened by svn_client__open_double_ra_session to
> + get a file from RA. The properties for this file will then override the
> + properties of the correspond file in BASE_DIR.
> +
> + SESSION_BATON2/RA_LIB are the session and RA library to use for the
> + request.
> +
> + PATH is the URL relative to the root of the session.
> +
> + REVISION is the revision of the file required.
> +
> + FSTREAM is where the fulltext of the file is written.
> +
> + The caller should ensure that POOL and PATH remain valid until
> + svn_client__discard_file_from_double_ra is called.
> + */
> +svn_error_t *
> +svn_client__double_ra_get_file (const svn_ra_plugin_t *ra_lib,
> + void *session_baton,
> + void *callback_baton,
> + const char *path,
> + svn_revnum_t revision,
> + svn_stream_t *fstream,
> + apr_pool_t *pool);
> +
> +
> +/* Indicate the the overriding file for a double RA session is to be
> + discarded. This means that the file properties in BASE_DIR will become
> + visible.
> +
> + SESSION_BATON is either of the two session batons.
> + */
> +void svn_client__discard_file_from_double_ra (void *callback_baton);
> +
> +
> /* Retrieve an AUTHENTICATOR/AUTH_BATON pair from the client,
> which represents the protocol METHOD. */
> svn_error_t * svn_client__get_authenticator (void **authenticator,
> @@ -146,7 +225,7 @@
> * TARGET represents the base of the hierarchy to be compared. TARGET can
> * be a working copy path, or an URL.
> *
> - * DIFF_CMD/DIFF_CMD_BATON represent the callback and calback argument that
> + * DIFF_CMD/DIFF_CMD_BATON represent the callback and callback argument that
> * implement the file comparison function
> *
> * RECURSE is set if the diff is to be recursive.
> @@ -165,6 +244,7 @@
> svn_boolean_t recurse,
> svn_ra_plugin_t *ra_lib,
> void *ra_session,
> + void *callback_baton,
> svn_revnum_t revision,
> const svn_delta_edit_fns_t **editor,
> void **edit_baton,
> Index: ../svn/subversion/libsvn_ra_dav/fetch.c
> ===================================================================
> --- ../svn/subversion/libsvn_ra_dav/fetch.c
> +++ ../svn/subversion/libsvn_ra_dav/fetch.c Thu Jan 24 14:59:32 2002
> @@ -856,6 +856,12 @@
> svn_string_create(val, ras->pool));
> #undef NSLEN
> }
> + /* ### TODO: Need this to make the double ra session override file
> + work. Can I just add properties like this? */
> + apr_hash_set(*props,
> + SVN_RA_DAV__LP_VSN_URL,
> + APR_HASH_KEY_STRING,
> + svn_string_create(get_vsn_url(rsrc), ras->pool));
> }
>
> return SVN_NO_ERROR;
>
>
> --
> Philip
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 27 18:44:02 2002