[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r15678 - in trunk/subversion: clients/cmdline clients/cmdline/dtd libsvn_client

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-08-11 22:20:47 CEST

On Thu, 11 Aug 2005 sussman@tigris.org wrote:

Hi,

Some details.

> Modified: trunk/subversion/clients/cmdline/ls-cmd.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/clients/cmdline/ls-cmd.c?rev=15678&p1=trunk/subversion/clients/cmdline/ls-cmd.c&p2=trunk/subversion/clients/cmdline/ls-cmd.c&r1=15677&r2=15678
> ==============================================================================
> --- trunk/subversion/clients/cmdline/ls-cmd.c (original)
> +++ trunk/subversion/clients/cmdline/ls-cmd.c Thu Aug 11 09:02:04 2005
> @@ -175,6 +183,7 @@
> utf8_entryname = item->key;
>
> dirent = apr_hash_get (dirents, utf8_entryname, item->klen);
> + lock = apr_hash_get (locks, utf8_entryname, APR_HASH_KEY_STRING);
>
> sb = svn_stringbuf_create ("", subpool);
>
> @@ -208,6 +217,49 @@
> /* "</commit>" */
> svn_xml_make_close_tag (&sb, subpool, "commit");
>
> + /* "<lock>" */
> + if (lock)
> + {
> + svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "lock", NULL);
> +
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "token", NULL);
> + svn_xml_escape_cdata_cstring (&sb, lock->token, pool);
> + svn_xml_make_close_tag (&sb, pool, "token");
> +
The above is now handled by svn_cl__xml_tagged_cdata. That's rather new.
Same for the other lock fields below.

> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "owner", NULL);
> + svn_xml_escape_cdata_cstring (&sb, lock->owner, pool);
> + svn_xml_make_close_tag (&sb, pool, "owner");
> +
> + if (lock->comment)
> + {
> + svn_xml_make_open_tag (&sb, pool, svn_xml_normal,
> + "comment", NULL);
> + svn_xml_escape_cdata_cstring (&sb, lock->comment, pool);
> + svn_xml_make_close_tag (&sb, pool, "comment");
> + }
> +
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "created", NULL);
> + svn_xml_escape_cdata_cstring (&sb, svn_time_to_cstring
> + (lock->creation_date, pool),
> + pool);
> + svn_xml_make_close_tag (&sb, pool, "created");
> +
> + if (lock->expiration_date != 0)
> + {
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "expires", NULL);
> + svn_xml_escape_cdata_cstring (&sb, svn_time_to_cstring
> + (lock->expiration_date, pool),
> + pool);
> + svn_xml_make_close_tag (&sb, pool, "expires");
> + }
> +
> + /* "<lock>" */

I think such comments are redundant, but if you keep it, it should be
"</lock>".

> @@ -292,14 +345,15 @@
> /* Get peg revisions. */
> SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
> subpool));
> - SVN_ERR (svn_client_ls2 (&dirents, truepath, &peg_revision,
> +
> + SVN_ERR (svn_client_ls3 (&dirents, &locks, truepath, &peg_revision,
> &(opt_state->start_revision),
> opt_state->recursive, ctx, subpool));
>

An improvement would be to pass NULL for the locks argument if
!opt_state->verbose. It avoids downloading all the locks if they aren't
displayed.

> if (opt_state->xml)
> - SVN_ERR (print_dirents_xml (dirents, truepath, ctx, subpool));
> + SVN_ERR (print_dirents_xml (dirents, locks, truepath, ctx, subpool));
> else
> - SVN_ERR (print_dirents (dirents, opt_state->verbose, ctx, subpool));
> + SVN_ERR (print_dirents (dirents, locks, opt_state->verbose, ctx, subpool));
> }
>
> svn_pool_destroy (subpool);
>
> Modified: trunk/subversion/clients/cmdline/main.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/clients/cmdline/main.c?rev=15678&p1=trunk/subversion/clients/cmdline/main.c&p2=trunk/subversion/clients/cmdline/main.c&r1=15677&r2=15678
> ==============================================================================
> --- trunk/subversion/clients/cmdline/main.c (original)
> +++ trunk/subversion/clients/cmdline/main.c Thu Aug 11 09:02:04 2005
> @@ -389,10 +389,11 @@
> "current\n"
> " working directory.\n"
> "\n"
> - " With --verbose, the following fields show the status of the item:\n"
> + " With --verbose, the following fields will be shown for each item:\n"
> "\n"
> " Revision number of the last commit\n"
> " Author of the last commit\n"
> + " If locked, the owner of the lock\n"
> " Size (in bytes)\n"
> " Date and time of the last commit\n"),
> {'r', 'v', 'R', svn_cl__incremental_opt, svn_cl__xml_opt,
>
> Modified: trunk/subversion/include/svn_client.h
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/include/svn_client.h?rev=15678&p1=trunk/subversion/include/svn_client.h&p2=trunk/subversion/include/svn_client.h&r1=15677&r2=15678
> ==============================================================================
> --- trunk/subversion/include/svn_client.h (original)
> +++ trunk/subversion/include/svn_client.h Thu Aug 11 09:02:04 2005
> @@ -1853,8 +1853,13 @@
> * @a path_or_url is a file, return only the dirent for the file. If @a
> * path_or_url is non-existent, return @c SVN_ERR_FS_NOT_FOUND.
> *
> - * The hash maps entry names (<tt>const char *</tt>) to @c svn_dirent_t *'s.
> - * Do all allocation in @a pool.
> + * The @a dirents hash maps entry names (<tt>const char *</tt>) to
> + * @c svn_dirent_t *'s. Do all allocation in @a pool.
> + *
> + * If @a locks is not @c NULL, set @a *locks to a hash table mapping
> + * entry names (<tt>const char *</tt>) to @c svn_lock_t *'s,
> + * allocating both @a *locks and everything inside it in @a pool.
> + * This hash represents any existing repository locks on entries.
> *
> * Use authentication baton cached in @a ctx to authenticate against the
> * repository.
> @@ -1862,7 +1867,22 @@
> * If @a recurse is true (and @a path_or_url is a directory) this will
> * be a recursive operation.
> *
> - * @since New in 1.2.
> + * @since New in 1.3.
> + */
> +svn_error_t *
> +svn_client_ls3 (apr_hash_t **dirents,
> + apr_hash_t **locks,
> + const char *path_or_url,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t recurse,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +/**
> + * Same as svn_client_ls3(), but always passes a NULL lock hash.
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> */
> svn_error_t *
> svn_client_ls2 (apr_hash_t **dirents,
>
This remoes the @since from svn_ls_client2().

> Modified: trunk/subversion/libsvn_client/ls.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_client/ls.c?rev=15678&p1=trunk/subversion/libsvn_client/ls.c&p2=trunk/subversion/libsvn_client/ls.c&r1=15677&r2=15678
> ==============================================================================
> --- trunk/subversion/libsvn_client/ls.c (original)
> +++ trunk/subversion/libsvn_client/ls.c Thu Aug 11 09:02:04 2005
> @@ -135,8 +148,57 @@
> _("URL '%s' non-existent in that revision"),
> url);
>
> + if (locks == NULL)
> + return SVN_NO_ERROR;
> +
> + if (rel_path == NULL || rel_path[0] == 0)
> + rel_path = "/";
> + else
> + rel_path = apr_psprintf (pool, "/%s/", rel_path);
> +

Hmmm... I don't understand what this above code is doing. When can
rel_path be NULL, and why are slashes added at each end of the string
unconditionally?

> + subpool = svn_pool_create (pool);
> +
> + /* Get lock. */
s/lock/locks/

> + SVN_ERR (svn_ra_get_locks (ra_session, locks, "", pool));
> +
> + new_locks = apr_hash_make (pool);
> + for (hi = apr_hash_first (pool, *locks); hi; hi = apr_hash_next (hi))
> + {
> + const void *key;
> + void *val;
> + const char *newkey;
> +
> + svn_pool_clear (subpool);
> +
> + apr_hash_this (hi, &key, NULL, &val);
> + newkey = svn_path_is_child (svn_path_canonicalize (rel_path, subpool),
> + svn_path_canonicalize (key, subpool),
> + pool);

Why canonicalize here? If rel_path needs that, it could be done outside of
the loop, but why are we creating non-canonical paths in the first place?
And regarding key, doesn't the API guarantee that that is canonicalized?

Best,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 11 22:22:58 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.