On Sat, 12 Mar 2005 kfogel@tigris.org wrote:
> Author: kfogel
> Date: Sat Mar 12 17:10:38 2005
> New Revision: 13392
>
> Modified: branches/locking/subversion/libsvn_client/locking_commands.c
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/libsvn_client/locking_commands.c?view=diff&rev=13392&p1=branches/locking/subversion/libsvn_client/locking_commands.c&r1=13391&p2=branches/locking/subversion/libsvn_client/locking_commands.c&r2=13392
> ==============================================================================
> --- branches/locking/subversion/libsvn_client/locking_commands.c (original)
> +++ branches/locking/subversion/libsvn_client/locking_commands.c Sat Mar 12 17:10:38 2005
> @@ -105,156 +114,214 @@
...
>
> - * setting PARENT_ENTRY_P and PARENT_ADM_ACCESS_P to the entry and
> - * adm_access of the common parent. PARENT_ADM_ACCESS_P is associated
> - * with all other adm_access's that are locked in the working copy
> - * while we lock the path in the repository. DO_LOCK should be TRUE
> - * if you're locking TARGETS, and FALSE when unlocking them. FORCE is
> - * TRUE if we're breaking or stealing locks, and FALSE otherwise.
> - *
> - * Each key in HASH_P is a path (relative to the common parent).
> - * If DO_LOCK is true, the value is the corresponding base_revision
> - * for the path. Otherwise, the value is the lock token ("" if no
> - * token is found in the wc).
> +/* Set *COMMON_PARENT to the nearest common parent of all TARGETS.
> + *
> + * If all the targets are local paths within the same wc, i.e.,
> + * they share a common parent at some level, set *PARENT_ENTRY_P
> + * and *PARENT_ADM_ACCESS_P to the entry and adm_access of that
> + * common parent. *PARENT_ADM_ACCESS_P will be associated with
> + * adm_access objects for all the other paths, which are locked in the
> + * working copy while we lock them in the repository.
> + *
> + * If all the targets are URLs in the same repository, i.e. sharing a
> + * common parent URL prefix, then set *PARENT_ENTRY_P and
> + * *PARENT_ADM_ACCESS_P to null.
> + *
> + * If there is no common parent, either because the targets are a
> + * mixture of URLs and local paths, or because they simply do not
> + * share a common parent, then return SVN_ERR_UNSUPPORTED_FEATURE.
> + * (The value of *COMMON_PARENT and other return parameters is
> + * undefined in this case.)
> + *
Do we really need to point out that other return values are undefined on
error? I think that's as evil as pointing out that arguments must not be
null, referring to another recent thread:-) We should point out when
return values *are* defined, despite an error return.
...
> + if (*common_parent == NULL || (*common_parent)[0] == '\0')
> + return svn_error_create
> + (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("No common parent found, unable to operate on disjoint arguments"));
> +
Maybe this is a little cryptic. If I understand this, it will happen, for
example, when you specify one URL and one WCPATH. And since I'm
complaining, I'll probably have to come up with something better;) I'll
think about it.
> +/* Set the value of each key in PATH_REVS to a pointer to the latest
> + revision (allocated in POOL) of the repository open in RA_SESSION. */
> +static svn_error_t *
> +store_head_revision (svn_ra_session_t *ra_session,
> + apr_hash_t *path_revs,
> + apr_pool_t *pool)
> +{
> + svn_revnum_t *latest = apr_palloc (pool, sizeof (*latest));
> + apr_hash_index_t *hi;
> +
> + SVN_ERR (svn_ra_get_latest_revnum (ra_session, latest, pool));
> + for (hi = apr_hash_first (pool, path_revs); hi; hi = apr_hash_next (hi))
> + {
> + const void *key;
> + apr_ssize_t klen;
> + apr_hash_this (hi, &key, &klen, NULL);
> + apr_hash_set (path_revs, key, klen, latest);
> + }
>
> return SVN_NO_ERROR;
> }
I wonder why this function is necessary (see also below).
> @@ -271,11 +338,14 @@
> apr_pool_t *pool)
> {
> svn_wc_adm_access_t *adm_access;
> + const char *common_parent;
> const svn_wc_entry_t *entry;
> + const char *url;
> svn_ra_session_t *ra_session;
> apr_array_header_t *locks;
> apr_hash_t *path_revs;
> struct lock_baton cb;
> + svn_boolean_t is_url;
>
> /* Enforce that the comment be xml-escapable. */
> if (comment)
> @@ -286,26 +356,41 @@
> _("Lock comment has illegal characters."));
> }
>
> - SVN_ERR (open_lock_targets (&entry, &adm_access, &path_revs,
> - targets, TRUE, force, ctx, pool));
> + SVN_ERR (organize_lock_targets (&common_parent, &entry, &adm_access,
> + &path_revs, targets, TRUE, force, ctx,
> + pool));
> +
> + is_url = svn_path_is_url (common_parent);
> +
> + if (is_url)
> + url = common_parent;
> + else
> + url = entry->url;
>
> /* Open an RA session to the common parent of TARGETS. */
> - SVN_ERR (svn_client__open_ra_session (&ra_session, entry->url,
> - svn_wc_adm_access_path (adm_access),
> - NULL, NULL, FALSE, FALSE,
> - ctx, pool));
> + SVN_ERR (svn_client__open_ra_session
> + (&ra_session, url, is_url ? NULL : common_parent,
> + adm_access, NULL, FALSE, FALSE, ctx, pool));
>
> cb.pool = pool;
> cb.nested_callback = lock_func;
> cb.nested_baton = lock_baton;
> cb.adm_access = adm_access;
>
> + /* ### This is a total crock. :-( */
> + if (is_url)
> + SVN_ERR (store_head_revision (ra_session, path_revs, pool));
> +
Does anyone know why Karl did this? In svn_ra_lock, it is documented that
SVN_INVALID_REVNUM means don't care about which revision is latest. That's
what we want to do for URLs. Is that case buggy in some ra layer?
Since Karl is away, I'll probably fix those myself when svn st -u is
finished, so this is ore of a TODO items list;)
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 14 08:25:27 2005