Re: svn commit: r12365 - in trunk/subversion: clients/cmdline include libsvn_client tests/clients/cmdline

[kf...@collab.net]

jpieper@tigris.org writes:
> Modified:
>    trunk/subversion/clients/cmdline/checkout-cmd.c
>    trunk/subversion/clients/cmdline/main.c
>    trunk/subversion/include/svn_client.h
>    trunk/subversion/libsvn_client/checkout.c
>    trunk/subversion/libsvn_client/client.h
>    trunk/subversion/libsvn_client/copy.c
>    trunk/subversion/libsvn_client/externals.c
>    trunk/subversion/tests/clients/cmdline/basic_tests.py
>    trunk/subversion/tests/clients/cmdline/stat_tests.py
>    trunk/subversion/tests/clients/cmdline/update_tests.py
> Log:
> Make the checkout subcommand accept peg revisions and follow history.
> Update the test suite so that tests which refer to past revisions do
> not rely on the default value of the peg revision in order to pass.
> 
> * subversion/include/svn_client.h
>   (svn_client_checkout2): New API that provides a peg revision parameter.
>   (svn_client_checkout): Deprecated.
>   
> * subversion/libsvn_client/client.h
>   (svn_client__checkout_internal): Extend to accept a peg revision
>     parameter.
>   
> * subversion/libsvn_client/externals.c
>   (handle_external_item_change): Use the new form of
>     svn_client__checkout_internal.  Set the peg revision to be the
>     same as the desired revision, so as to maintain backwards
>     compatible externals definitions.
>   
> * subversion/libsvn_client/checkout.c
>   (svn_client__checkout_internal): Accept a peg revision parameter and
>     use svn_client__ra_lib_from_path to get the RA connection.
>   (svn_client_checkout2): New, implement.
>   (svn_client_checkout): Implement in terms of svn_client_checkout2.
>   
> * subversion/libsvn_client/copy.c
>   (repos_to_wc_copy): Update the the new
>     svn_client__checkout_internal.  For now set the peg revision the
>     same as the desired revision, until the 'svn copy' subcommand
>     starts accepting peg revisions.
> 
> * subversion/clients/cmdline/checkout-cmd.c
>   (svn_cl__checkout): Accept peg revisions and use
>     svn_client_checkout2.
> 
> * subversion/clients/cmdline/main.c
>   (svn_cl__cmd_table): Update the help text for 'svn checkout'.
> 
> * subversion/tests/clients/cmdline/stat_tests.py: (status_on_forward_deletion)
> * subversion/tests/clients/cmdline/basic_tests.py: (basic_checkout_deleted)
> * subversion/tests/clients/cmdline/update_tests.py: (nested_in_read_only)
>   Specify the appropriate peg revisions to 'svn checkout'.
> 
> 
> --- trunk/subversion/include/svn_client.h	(original)
> +++ trunk/subversion/include/svn_client.h	Sat Dec 18 05:57:45 2004
> @@ -417,11 +417,12 @@
>  svn_client_create_context (svn_client_ctx_t **ctx,
>                             apr_pool_t *pool);
>  
> -/** Checkout a working copy of @a URL at @a revision, using @a path as
> - * the root directory of the newly checked out working copy, and
> - * authenticating with the authentication baton cached in @a ctx.  If
> - * @a result_rev is not @c NULL, set @a *result_rev to the value of
> - * the revision actually checked out from the repository.
> +/** Checkout a working copy of @a URL at @a revision, looked up at @a
> + * peg_revision, using @a path as the root directory of the newly
> + * checked out working copy, and authenticating with the
> + * authentication baton cached in @a ctx.  If @a result_rev is not @c
> + * NULL, set @a *result_rev to the value of the revision actually
> + * checked out from the repository.

It might be good to say that if peg_revision->kind is
svn_opt_revision_unspecified, then the head revision is used.

> --- trunk/subversion/libsvn_client/checkout.c	(original)
> +++ trunk/subversion/libsvn_client/checkout.c	Sat Dec 18 05:57:45 2004
> @@ -42,8 +42,9 @@
>  
>  svn_error_t *
>  svn_client__checkout_internal (svn_revnum_t *result_rev,
> -                               const char *URL,
> +                               const char *url,
>                                 const char *path,
> +                               const svn_opt_revision_t *peg_revision,
>                                 const svn_opt_revision_t *revision,
>                                 svn_boolean_t recurse,
>                                 svn_boolean_t *timestamp_sleep,
> @@ -55,6 +56,7 @@
>    svn_revnum_t revnum;
>    svn_boolean_t sleep_here = FALSE;
>    svn_boolean_t *use_sleep = timestamp_sleep ? timestamp_sleep : &sleep_here;
> +  const char *URL;

This is the place where, in an earlier mail, I pointed out that having
both "url" and "URL" in the same function is confusing.  I don't mean
to beat you up on the point :-)!  But since I'm now reviewing the
commit that introduced it, I guess I should mention it again.

>    /* Sanity check.  Without these, the checkout is meaningless. */
>    assert (path != NULL);
> @@ -67,27 +69,19 @@
>      return svn_error_create (SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
>  
>    /* Canonicalize the URL. */
> -  URL = svn_path_canonicalize (URL, pool);
> +  URL = svn_path_canonicalize (url, pool);

What a bizarre-looking pair of diff lines, heh.

>      {
> -      void *ra_baton, *session;
> +      void *session;
>        svn_ra_plugin_t *ra_lib;
>        svn_node_kind_t kind;
>        const char *uuid;
>  
> -      /* Get the RA vtable that matches URL. */
> -      SVN_ERR (svn_ra_init_ra_libs (&ra_baton, pool));
> -      SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, URL, pool));
> -
> -      /* Open an RA session to URL. Note that we do not have an admin area
> -         for storing temp files. */
> -      SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL, NULL,
> -                                            NULL, NULL, FALSE, TRUE,
> -                                            ctx, pool));
> -
> -      SVN_ERR (svn_client__get_revision_number
> -               (&revnum, ra_lib, session, revision, path, pool));
> -
> +      /* Get the RA connection. */
> +      SVN_ERR (svn_client__ra_lib_from_path (&ra_lib, &session, &revnum,
> +                                             &URL, url, peg_revision,
> +                                             revision, ctx, pool));
> +      

More repeats from my earlier mail: the first initialization of 'URL'
gets overwritten by this call to svn_client__ra_lib_from_path().  And,
the whole curly-braced block is misindented relative to the code above
and below it.

> --- trunk/subversion/libsvn_client/copy.c	(original)
> +++ trunk/subversion/libsvn_client/copy.c	Sat Dec 18 05:57:45 2004
> @@ -893,9 +893,10 @@
>    }
>  
>    if (src_kind == svn_node_dir)
> -    {    
> +    {
>        SVN_ERR (svn_client__checkout_internal
> -               (NULL, src_url, dst_path, &revision, TRUE, NULL, ctx, pool));
> +               (NULL, src_url, dst_path, &revision, &revision,
> +                TRUE, NULL, ctx, pool));
>  
>        if ((revision.kind == svn_opt_revision_head) && same_repositories)
>          {

A comment like 

   /* ### FIXME: second 'revision' argument will be 'peg_revision',
      someday when copy takes a peg_revision. */

...might be good here.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org