cmpil...@apache.org wrote on Wed, 23 Jun 2010 at 04:22 -0000:
> Author: cmpilato
> Date: Wed Jun 23 01:22:00 2010
> New Revision: 957094
>
> URL: http://svn.apache.org/viewvc?rev=957094&view=rev
> Log:
> Finish issue #3661: RA get-locks inefficiencies.
>
> * build.conf
> (svnserve): Add dependency on libsvn_ra.
>
> * subversion/svnserve/serve.c
> (get_locks): Look for optional depth in the get-locks request, and
> use it in what is now a call to svn_repos_fs_get_locks2().
>
> * subversion/libsvn_ra_svn/client.c
> (ra_svn_get_locks): Add 'depth' parameter, and pass it to the server
> in case the server can make use of it. Also, filter out unwanted
> locks (returned by servers that *can't* make use of it).
>
> Modified: subversion/trunk/build.conf
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/build.conf?rev=957094&r1=957093&r2=957094&view=diff
> ==============================================================================
> --- subversion/trunk/build.conf (original)
> +++ subversion/trunk/build.conf Wed Jun 23 01:22:00 2010
> @@ -147,7 +147,7 @@ type = exe
> path = subversion/svnserve
> install = bin
> manpages = subversion/svnserve/svnserve.8 subversion/svnserve/svnserve.conf.5
> -libs = libsvn_repos libsvn_fs libsvn_delta libsvn_subr libsvn_ra_svn
> +libs = libsvn_repos libsvn_fs libsvn_delta libsvn_subr libsvn_ra
> libsvn_ra_svn
> apriconv apr sasl
> msvc-libs = advapi32.lib ws2_32.lib
>
>
> Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/client.c?rev=957094&r1=957093&r2=957094&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/client.c Wed Jun 23 01:22:00
> 2010
> @@ -2210,14 +2210,23 @@ static svn_error_t *ra_svn_get_lock(svn_
> static svn_error_t *ra_svn_get_locks(svn_ra_session_t *session,
> apr_hash_t **locks,
> const char *path,
> + svn_depth_t depth,
> apr_pool_t *pool)
> {
> svn_ra_svn__session_baton_t *sess = session->priv;
> svn_ra_svn_conn_t* conn = sess->conn;
> apr_array_header_t *list;
> + const char *abs_path;
> int i;
>
> - SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "get-locks", "c", path));
> + /* Figure out the repository abspath from PATH. */
> + abs_path = svn_path_url_add_component2(sess->url, path, pool);
> + SVN_ERR(svn_ra_get_path_relative_to_root(session, &abs_path,
> + abs_path, pool));
I think this change means that, in build.conf, libsvn_ra should have
been added as a dependency to [libsvn_ra_svn]. (This patch added it only
to [svnserve].)
Unless objections, I'll make this change (while also committing the
ra_svn protocol bits noted on IRC and in the issue).
> + abs_path = apr_pstrcat(pool, "/", abs_path, NULL);
> +
> + SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "get-locks", "c?w", path,
> + svn_depth_to_word(depth)));
>
> /* Servers before 1.2 doesn't support locking. Check this here. */
> SVN_ERR(handle_unsupported_cmd(handle_auth_request(sess, pool),
> @@ -2237,7 +2246,27 @@ static svn_error_t *ra_svn_get_locks(svn
> return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> _("Lock element not a list"));
> SVN_ERR(parse_lock(elt->u.list, pool, &lock));
> - apr_hash_set(*locks, lock->path, APR_HASH_KEY_STRING, lock);
> +
> + /* Filter out unwanted paths. Since Subversion only allows
> + locks on files, we can treat depth=immediates the same as
> + depth=files for filtering purposes. Meaning, we'll keep
> + this lock if:
> +
> + a) its path is the very path we queried, or
> + b) we've asked for a fully recursive answer, or
> + c) we've asked for depth=files or depth=immediates, and this
> + lock is on an immediate child of our query path.
> + */
> + if ((strcmp(abs_path, lock->path) == 0) || (depth ==
> svn_depth_infinity))
> + {
> + apr_hash_set(*locks, lock->path, APR_HASH_KEY_STRING, lock);
> + }
> + else if ((depth == svn_depth_files) || (depth == svn_depth_immediates))
> + {
> + const char *rel_uri = svn_uri_is_child(abs_path, lock->path, pool);
> + if (rel_uri && (svn_path_component_count(rel_uri) == 1))
> + apr_hash_set(*locks, lock->path, APR_HASH_KEY_STRING, lock);
> + }
> }
>
> return SVN_NO_ERROR;
>
> Modified: subversion/trunk/subversion/svnserve/serve.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=957094&r1=957093&r2=957094&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnserve/serve.c (original)
> +++ subversion/trunk/subversion/svnserve/serve.c Wed Jun 23 01:22:00 2010
> @@ -2572,21 +2572,23 @@ static svn_error_t *get_locks(svn_ra_svn
> server_baton_t *b = baton;
> const char *path;
> const char *full_path;
> + const char *depth_word;
> + svn_depth_t depth;
> apr_hash_t *locks;
> apr_hash_index_t *hi;
>
> - SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "c", &path));
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "c?w", &path, &depth_word));
>
> - full_path = svn_uri_join(b->fs_path->data, svn_uri_canonicalize(path,
> - pool),
> - pool);
> + depth = depth_word ? svn_depth_from_word(depth_word) : svn_depth_infinity;
> + full_path = svn_uri_join(b->fs_path->data,
> + svn_uri_canonicalize(path, pool), pool);
>
> SVN_ERR(trivial_auth_request(conn, pool, b));
>
> SVN_ERR(log_command(b, conn, pool, "get-locks %s",
> svn_path_uri_encode(full_path, pool)));
> - SVN_CMD_ERR(svn_repos_fs_get_locks(&locks, b->repos, full_path,
> - authz_check_access_cb_func(b), b,
> pool));
> + SVN_CMD_ERR(svn_repos_fs_get_locks2(&locks, b->repos, full_path, depth,
> + authz_check_access_cb_func(b), b,
> pool));
>
> SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w((!", "success"));
> for (hi = apr_hash_first(pool, locks); hi; hi = apr_hash_next(hi))
>
>
>
