On Sep 14, 2009, at 4:48 PM, Bert Huijben wrote:
>> -----Original Message-----
>> From: Hyrum K. Wright [mailto:hyrum_at_hyrumwright.org]
>> Sent: maandag 14 september 2009 21:50
>> To: svn_at_subversion.tigris.org
>> Subject: svn commit: r39312 - trunk/subversion/libsvn_client
>>
>> Author: hwright
>> Date: Mon Sep 14 12:49:53 2009
>> New Revision: 39312
>>
>> Log:
>> Rewrite the client locks organization code to remove the use of
>> access
>> batons.
>>
>> * subversion/libsvn_client/locking_commands.c
>> (organize_lock_targets): Pass back a base_directory in place of an
>> access
>> baton, and update the base_dir calculation. Also, rename the
>> common_parent
>> output variable to something a bit more descriptive.
>> (svn_client_lock, svn_client_unlock): Drop the use of an access
>> baton, and
>> just use the returned base_dir to get the desired information.
>>
>> Modified:
>> trunk/subversion/libsvn_client/locking_commands.c
>>
>> Modified: trunk/subversion/libsvn_client/locking_commands.c
>> URL:
>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/locking
>> _commands.c?pathrev=39312&r1=39311&r2=39312
>> =
>> =
>> =====================================================================
>> =======
>> --- trunk/subversion/libsvn_client/locking_commands.c Mon Sep 14
>> 12:25:09 2009 (r39311)
>> +++ trunk/subversion/libsvn_client/locking_commands.c Mon Sep 14
>> 12:49:53 2009 (r39312)
>> @@ -136,21 +136,11 @@ store_locks_callback(void *baton,
>> }
>>
>>
>> -/* Set *COMMON_PARENT to the nearest common parent of all
>> TARGETS. If
>> - * TARGETS are local paths, then the entry for each path is examined
>> +/* Set *COMMON_PARENT_URL to the nearest common parent URL of all
>> TARGETS.
>> + * If TARGETS are local paths, then the entry for each path is
>> examined
>> * and *COMMON_PARENT is set to the common parent URL for all the
>> * targets (as opposed to the common local path).
>> *
>> - * If all the targets are local paths within the same wc, i.e., they
>> - * share a common parent at some level, set *PARENT_ADM_ACCESS_P
>> - * to the 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_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.
>> @@ -175,8 +165,8 @@ store_locks_callback(void *baton,
>> * TARGETS may not be empty.
>> */
>> static svn_error_t *
>> -organize_lock_targets(const char **common_parent,
>> - svn_wc_adm_access_t **parent_adm_access_p,
>> +organize_lock_targets(const char **common_parent_url,
>> + const char **base_dir,
>> apr_hash_t **rel_targets_p,
>> apr_hash_t **rel_fs_paths_p,
>> const apr_array_header_t *targets,
>> @@ -192,30 +182,30 @@ organize_lock_targets(const char **commo
>> apr_pool_t *subpool = svn_pool_create(pool);
>>
>> /* Get the common parent and all relative paths */
>> - SVN_ERR(svn_path_condense_targets(common_parent, &rel_targets,
>> targets,
>> + SVN_ERR(svn_path_condense_targets(common_parent_url, &rel_targets,
>> targets,
>> FALSE, pool));
>
> If rel_targets are urls, you can use svn_uri_condense_targets(). If
> they are
> paths svn_dirent_condense_targets().
Saving that (as well as the pool cleanup) for a second, but possibly
not imminent, pass.
>> /* svn_path_condense_targets leaves paths empty if TARGETS only had
>> 1 member, so we special case that. */
>> if (apr_is_empty_array(rel_targets))
>> {
>> - const char *base_name = svn_uri_basename(*common_parent,
>> pool);
>> - *common_parent = svn_uri_dirname(*common_parent, pool);
>> + const char *base_name = svn_uri_basename(*common_parent_url,
>> pool);
>> + *common_parent_url = svn_uri_dirname(*common_parent_url,
>> pool);
>>
>> APR_ARRAY_PUSH(rel_targets, const char *) = base_name;
>> }
>>
>> - if (*common_parent == NULL || (*common_parent)[0] == '\0')
>> + if (*common_parent_url == NULL || (*common_parent_url)[0] == '\0')
>> return svn_error_create
>> (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
>> _("No common parent found, unable to operate on disjoint
>> arguments"));
>>
>> - if (svn_path_is_url(*common_parent))
>> + if (svn_path_is_url(*common_parent_url))
>
> This looks strange. Is it an _url, or isn't it?
>
> My guess would be that it can be either.. and in that case the url
> suffix is
> not right.
Good question. The output parameter eventually becomes a URL, but in
our infinite optimization wisdom, we use it internally in the function
to store who-knows-what. I maintain that the current name is correct,
since it's part of the function-external documentation, but that the
implementation could use some reorganization for clarity.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2394798
Received on 2009-09-14 23:54:24 CEST