On Monday 22 September 2003 09:34, Julian Foad wrote:
> This is the deepest I have dug into RA and WC adm stuff so far, so please
> tell me what I have missed and what I could do better. For one thing, I
> won't check it in with the "###...?" comment in it.
>
> I have a strong feeling that much of this logic is, or should be, common to
> subcommands like "diff", "export" and "copy" as well, and I would be
> grateful to be steered in that direction.
I agree. I've look at this stuff several times wondering if we should do
something like make a 'fake' ra_lib that will access the pristine/working
copies instead. It would avoid having any repeat logic in the code,
hopefully making it a little cleaner. Thoughts?
> My goal is to recognise not only "-rBASE" as being local, but any revision
> specifier that resolves to the cached text-base revision, such as dates and
> revision numbers that lie anywhere in the range from COMMITTED to BASE
> inclusive. And to do so in all situations that could benefit from it. But
> to start with, I decided to just implement "svn cat -rBASE". I don't think
> it is very important by itself, and am not especially bothered about
> checking it in if I can instead progress toward a more general
> implementation.
>
> Below the log message is a human-readable version of the patch, i.e. with
> intentation changes ignored and consequent word-wrapping edited out.
> Attached is the real patch, with which you can use your favourite
> space-insensitive diff tool.
>
> - Julian
>
>
> [[[
> Enable "svn cat -rBASE" to work without contacting the repository
> (issue 1361).
>
> * subversion/libsvn_client/cat.c
> (open_text_base_RO) New function.
> (copy_stream) New function.
> (svn_client_cat) If the BASE revision is requested, and a path is given
> rather than a URL, then get the file and all necessary properties etc.
> from the WC administrative area instead of from the repository.
> ]]]
>
> Index: subversion/libsvn_client/cat.c
> ===================================================================
> --- subversion/libsvn_client/cat.c (revision 7111)
> +++ subversion/libsvn_client/cat.c (working copy)
> @@ -29,11 +29,49 @@
> #include "svn_io.h"
> #include "svn_time.h"
> #include "svn_path.h"
> +#include "svn_wc.h"
> #include "client.h"
>
> ^L
> /*** Code. ***/
>
> +/* Open the text base of local file PATH as a read-only STREAM. */
> +static svn_error_t *
> +open_text_base_RO (svn_stream_t **stream, const char *path, apr_pool_t
> *pool) +{
> + const char *text_base_path;
> + apr_file_t *text_base_file;
> + SVN_ERR (svn_wc_get_pristine_copy_path (path,
> + &text_base_path, pool));
> + if (! text_base_path)
> + return svn_error_createf (SVN_ERR_ILLEGAL_TARGET, NULL,
> + "file '%s' has no text-base.",
> + path);
> + /* ### Do we need a lock before reading the text base? */
> + SVN_ERR (svn_io_file_open (&text_base_file, text_base_path,
> + APR_READ, APR_OS_DEFAULT, pool));
> + *stream = svn_stream_from_aprfile (text_base_file, pool);
> + return SVN_NO_ERROR;
> +}
> +
> +/* Copy stream IN to stream OUT. */
> +#define STREAM_COPY_BUF_SIZE 100
> +static svn_error_t *
> +copy_stream (svn_stream_t *out, svn_stream_t *in)
> +{
> + char buf[STREAM_COPY_BUF_SIZE];
> + apr_size_t len = STREAM_COPY_BUF_SIZE;
> +
> + while (len == STREAM_COPY_BUF_SIZE)
> + {
> + /* At EOF there will be a short read (0 or more bytes) which will
> lead + to a write of the same length and then a clean exit. */
> + SVN_ERR (svn_stream_read (in, buf, &len));
> + SVN_ERR (svn_stream_write (out, buf, &len));
> + }
> + return SVN_NO_ERROR;
> +}
> +
> svn_error_t *
> svn_client_cat (svn_stream_t *out,
> const char *path_or_url,
> @@ -51,13 +89,23 @@
> apr_hash_t *props;
> const char *auth_dir;
> const char *url;
> + svn_boolean_t from_repos;
> + svn_wc_adm_access_t *adm_access;
>
> + /* Decide whether we will access the repository or just get the BASE
> + revision from the WC. */
> + from_repos = ((revision->kind != svn_opt_revision_base)
> + || svn_path_is_url (path_or_url));
> +
> + /* URL is wanted for keyword expansion as well as for repository access.
> */ SVN_ERR (svn_client_url_from_path (&url, path_or_url, pool));
> +
> + if (from_repos)
> + {
> if (! url)
> return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
> "'%s' has no URL", path_or_url);
>
> -
> /* Get the RA library that handles URL. */
> SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
> SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, url, pool));
> @@ -85,6 +133,27 @@
> /* Grab some properties we need to know in order to figure out if
> anything special needs to be done with this file. */
> SVN_ERR (ra_lib->get_file (session, "", rev, NULL, NULL, &props, pool));
> + }
> + else
> + {
> + /* Access the local text base */
> + const char *anchor, *target;
> + svn_node_kind_t kind;
> +
> + SVN_ERR (svn_wc_get_actual_target (path_or_url, &anchor, &target,
> pool)); + SVN_ERR (svn_io_check_path (path_or_url, &kind, pool));
> + SVN_ERR (svn_wc_adm_open (&adm_access, NULL, anchor, FALSE,
> + FALSE, pool));
> +
> + if (kind == svn_node_dir)
> + return svn_error_createf(SVN_ERR_CLIENT_IS_DIRECTORY, NULL,
> + "Path \"%s\" refers to a directory",
> + path_or_url);
> +
> + /* Grab some properties we need to know in order to figure out if
> + anything special needs to be done with this file. */
> + SVN_ERR (svn_wc_prop_list (&props, path_or_url, adm_access, pool));
IIRC, this will grab the properties for the working copy, not the pristine
text-base. If the working copy's properties were modified, then you'll be
retreiving something different than expected. I think you need to do this
instead:
svn_wc_get_prop_diffs (NULL, &props, path_or_url, adm_access, pool)
> + }
[snip]
One other problem that I ran into was that when I tried 'svn cat -r BASE
filename | more' and then quit more after the first screen, the working copy
wasn't getting closed and I would end up having to run 'svn cleanup' on it.
Looking at your patch, I imagine you probably suffer from the same problem
(it looks very similar to something I did on my first cut at it).
I know 'svn diff' works as expected... so I'm not sure what the difference is.
I was also hesitant about closing the working copy and still using the
pristine base. It seemed like that was a surefire way to invite trouble. :-)
-John
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 22 16:30:38 2003