You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/06/23 03:22:01 UTC

svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs/ subversion/libsvn_fs_base/ subversion/libsvn_fs_base/bdb/ subversion/libsvn_fs_fs/ subversion/libsvn_ra/ subversion/libsvn_ra_local/ subv...

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.

Add depth support to the process of querying repository locks in the
repository and RA layers for the sake of performance.  Prior to this
change, 'svn ls -v SOME_PATH' (non-recursive) asked the repository for
the locks on all paths in and under SOME_PATH (fully recursive),
resulting in potentially far more information being transmitted across
the network than is necessary.

* subversion/include/svn_fs.h
  (svn_fs_get_locks2): New.
  (svn_fs_get_locks): Deprecate this.

* subversion/libsvn_fs/fs-loader.h
  (fs_vtable_t): Add 'depth' to get_locks() vtable function.

* subversion/libsvn_fs/fs-loader.c
  (svn_fs_get_locks2): New.
  (svn_fs_get_locks): Make this just a wrapper around svn_fs_get_locks2().

* subversion/libsvn_fs_base/lock.h
  (svn_fs_base__get_locks): Add 'depth' parameter.

* subversion/libsvn_fs_base/lock.c
  (struct locks_get_args): Add 'depth' member.
  (svn_fs_base__allow_locked_operation, txn_body_get_locks): Update
    call to svn_fs_bdb__locks_get().
  (svn_fs_base__get_locks): Add 'depth' parameter, used to populate
    new baton member.

* subversion/libsvn_fs_base/bdb/locks-table.h
  (svn_fs_bdb__locks_get): Add 'depth' parameter.

* subversion/libsvn_fs_base/bdb/locks-table.c
  (svn_fs_bdb__locks_get): Add 'depth' parameter, and use it to filter
    the reported depths.

* subversion/libsvn_fs_fs/lock.h
  (svn_fs_fs__get_locks): Add 'depth' paramater.

* subversion/libsvn_fs_fs/lock.c
  (get_locks_filter_baton_t): New baton structure.
  (get_locks_filter_func): New callback wrapper function.
  (svn_fs_fs__get_locks): Add 'depth' paramater, and employ the
    get_locks_filter_func and baton when calling walk_digest_files().

* subversion/include/svn_repos.h
  (svn_repos_fs_get_locks2): New revision of svn_repos_fs_get_locks()
    which adds support for a specified 'depth'.
  (svn_repos_fs_get_locks): Deprecate this.

* subversion/libsvn_repos/fs-wrap.c
  (svn_repos_fs_get_locks2): Was svn_repos_fs_get_locks().  Now calls
    svn_fs_get_locks2().
  (svn_repos_fs_get_locks): Make this just a wrapper around
    svn_repos_fs_get_locks2(), now.

* subversion/mod_dav_svn/reports/get-locks.c
  (dav_svn__get_locks_report): Look for new (optional) depth
    parameter, and use it to drive what's now a call to
    svn_repos_fs_get_locks2().

* 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/include/svn_ra.h
  (svn_ra_get_locks2): New flavor of svn_ra_get_lock(), this time with
    depth support.
  (svn_ra_get_lock): Deprecate.

* subversion/libsvn_ra/ra_loader.h
  (svn_ra__vtable_t): Add 'depth' parameter to 'get_locks' function.

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_get_locks2): New.
  (svn_ra_get_locks): Make this just a wrapper around
    svn_ra_get_locks2().

* subversion/libsvn_ra_local/ra_plugin.c
  (svn_ra_local__get_locks): Add 'depth' parameter, and now use
    svn_repos_fs_get_locks2().

* subversion/libsvn_ra_neon/ra_neon.h
  (svn_ra_neon__get_locks): Add 'depth' parameter.

* subversion/libsvn_ra_neon/get_locks.c
  (get_locks_baton_t): Add 'path' and 'requested_depth' members.
  (getlocks_end_element): Filter unwanted locks out of the hash.
  (svn_ra_neon__get_locks): Add 'depth' parameter, embed it into the
    get-locks request, and populate the new baton members.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__get_locks): Add 'depth' parameter.

* subversion/libsvn_ra_serf/getlocks.c
  (lock_context_t): Add 'path' and 'requested_depth' members.
  (end_getlocks): Filter unwanted locks out of the hash.
  (create_getlocks_body): Stuff depth value from baton into the
    request body.
  (svn_ra_serf__get_locks): Add 'depth' parameter, 

* 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).

* subversion/libsvn_client/list.c
  (svn_client_list2): Now use svn_ra_get_locks2(), passing depth along.

Modified:
    subversion/trunk/build.conf
    subversion/trunk/subversion/include/svn_fs.h
    subversion/trunk/subversion/include/svn_ra.h
    subversion/trunk/subversion/include/svn_repos.h
    subversion/trunk/subversion/libsvn_client/list.c
    subversion/trunk/subversion/libsvn_fs/fs-loader.c
    subversion/trunk/subversion/libsvn_fs/fs-loader.h
    subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.c
    subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.h
    subversion/trunk/subversion/libsvn_fs_base/lock.c
    subversion/trunk/subversion/libsvn_fs_base/lock.h
    subversion/trunk/subversion/libsvn_fs_fs/lock.c
    subversion/trunk/subversion/libsvn_fs_fs/lock.h
    subversion/trunk/subversion/libsvn_ra/ra_loader.c
    subversion/trunk/subversion/libsvn_ra/ra_loader.h
    subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
    subversion/trunk/subversion/libsvn_ra_neon/get_locks.c
    subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h
    subversion/trunk/subversion/libsvn_ra_serf/getlocks.c
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_svn/client.c
    subversion/trunk/subversion/libsvn_repos/fs-wrap.c
    subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c
    subversion/trunk/subversion/svnserve/serve.c

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/include/svn_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_fs.h (original)
+++ subversion/trunk/subversion/include/svn_fs.h Wed Jun 23 01:22:00 2010
@@ -2126,6 +2126,11 @@ typedef svn_error_t *(*svn_fs_get_locks_
  * get_locks_func / @a get_locks_baton.  Use @a pool for necessary
  * allocations.
  *
+ * @depth limits the reported locks to those associated with paths
+ * within the specified depth of @path, and must be one of the
+ * following values:  #svn_depth_empty, #svn_depth_files,
+ * #svn_depth_immediates, or #svn_depth_infinity.
+ *
  * If the @a get_locks_func callback implementation returns an error,
  * lock iteration will terminate and that error will be returned by
  * this function.
@@ -2137,7 +2142,24 @@ typedef svn_error_t *(*svn_fs_get_locks_
  * start a new Berkeley DB transaction (which is most of this svn_fs
  * API).  Yes, this is a nasty implementation detail to have to be
  * aware of.  We hope to fix this problem in the future.
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_fs_get_locks2(svn_fs_t *fs,
+                  const char *path,
+                  svn_depth_t depth,
+                  svn_fs_get_locks_callback_t get_locks_func,
+                  void *get_locks_baton,
+                  apr_pool_t *pool);
+
+/** 
+ * Similar to svn_fs_get_locks2(), but with @a depth always passed as
+ * svn_depth_infinity.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
  */
+SVN_DEPRECATED
 svn_error_t *
 svn_fs_get_locks(svn_fs_t *fs,
                  const char *path,

Modified: subversion/trunk/subversion/include/svn_ra.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_ra.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_ra.h (original)
+++ subversion/trunk/subversion/include/svn_ra.h Wed Jun 23 01:22:00 2010
@@ -1689,6 +1689,11 @@ svn_ra_get_lock(svn_ra_session_t *sessio
  * Set @a *locks to a hashtable which represents all locks on or
  * below @a path.
  *
+ * @depth limits the returned locks to those associated with paths
+ * within the specified depth of @path, and must be one of the
+ * following values:  #svn_depth_empty, #svn_depth_files,
+ * #svn_depth_immediates, or #svn_depth_infinity.
+ *
  * The hashtable maps (const char *) absolute fs paths to (const
  * svn_lock_t *) structures.  The hashtable -- and all keys and
  * values -- are allocated in @a pool.
@@ -1700,8 +1705,23 @@ svn_ra_get_lock(svn_ra_session_t *sessio
  * server doesn't implement it, an @c SVN_ERR_RA_NOT_IMPLEMENTED error is
  * returned.
  *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_ra_get_locks2(svn_ra_session_t *session,
+                  apr_hash_t **locks,
+                  const char *path,
+                  svn_depth_t depth,
+                  apr_pool_t *pool);
+
+/**
+ * Similar to svn_ra_get_locks2(), but with @a depth always passed as
+ * #svn_depth_infinity.
+ *
  * @since New in 1.2.
+ * @deprecated Provided for backward compatibility with the 1.6 API.
  */
+SVN_DEPRECATED
 svn_error_t *
 svn_ra_get_locks(svn_ra_session_t *session,
                  apr_hash_t **locks,

Modified: subversion/trunk/subversion/include/svn_repos.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_repos.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_repos.h (original)
+++ subversion/trunk/subversion/include/svn_repos.h Wed Jun 23 01:22:00 2010
@@ -1954,7 +1954,30 @@ svn_repos_fs_unlock(svn_repos_t *repos,
  * authz_read_func and @a authz_read_baton to "screen" all returned
  * locks.  That is: do not return any locks on any paths that are
  * unreadable in HEAD, just silently omit them.
+ *
+ * @depth limits the returned locks to those associated with paths
+ * within the specified depth of @path, and must be one of the
+ * following values:  #svn_depth_empty, #svn_depth_files,
+ * #svn_depth_immediates, or #svn_depth_infinity.
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_repos_fs_get_locks2(apr_hash_t **locks,
+                        svn_repos_t *repos,
+                        const char *path,
+                        svn_depth_t depth,
+                        svn_repos_authz_func_t authz_read_func,
+                        void *authz_read_baton,
+                        apr_pool_t *pool);
+
+/** 
+ * Similar to svn_repos_fs_get_locks2(), but with @a depth always
+ * passed as svn_depth_infinity.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
  */
+SVN_DEPRECATED
 svn_error_t *
 svn_repos_fs_get_locks(apr_hash_t **locks,
                        svn_repos_t *repos,

Modified: subversion/trunk/subversion/libsvn_client/list.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/list.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/list.c (original)
+++ subversion/trunk/subversion/libsvn_client/list.c Wed Jun 23 01:22:00 2010
@@ -245,7 +245,7 @@ svn_client_list2(const char *path_or_url
     {
       /* IMPORTANT: If locks are stored in a more temporary pool, we need
          to fix store_dirent below to duplicate the locks. */
-      err = svn_ra_get_locks(ra_session, &locks, "", pool);
+      err = svn_ra_get_locks2(ra_session, &locks, "", depth, pool);
 
       if (err && err->apr_err == SVN_ERR_RA_NOT_IMPLEMENTED)
         {

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Wed Jun 23 01:22:00 2010
@@ -1301,16 +1301,29 @@ svn_fs_get_lock(svn_lock_t **lock, svn_f
 }
 
 svn_error_t *
+svn_fs_get_locks2(svn_fs_t *fs, const char *path, svn_depth_t depth,
+                  svn_fs_get_locks_callback_t get_locks_func,
+                  void *get_locks_baton, apr_pool_t *pool)
+{
+  SVN_ERR_ASSERT((depth == svn_depth_empty) ||
+                 (depth == svn_depth_files) ||
+                 (depth == svn_depth_immediates) ||
+                 (depth == svn_depth_infinity));
+  return svn_error_return(fs->vtable->get_locks(fs, path, depth,
+                                                get_locks_func,
+                                                get_locks_baton, pool));
+}
+
+svn_error_t *
 svn_fs_get_locks(svn_fs_t *fs, const char *path,
                  svn_fs_get_locks_callback_t get_locks_func,
-                 void *get_locks_baton,
-                 apr_pool_t *pool)
+                 void *get_locks_baton, apr_pool_t *pool)
 {
-  return svn_error_return(fs->vtable->get_locks(fs, path, get_locks_func,
-                                                get_locks_baton, pool));
+  return svn_error_return(svn_fs_get_locks2(fs, path, svn_depth_infinity,
+                                            get_locks_func, get_locks_baton,
+                                            pool));
 }
 
-
 
 /* --- History functions --- */
 

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.h (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.h Wed Jun 23 01:22:00 2010
@@ -190,7 +190,7 @@ typedef struct fs_vtable_t
                          svn_boolean_t break_lock, apr_pool_t *pool);
   svn_error_t *(*get_lock)(svn_lock_t **lock, svn_fs_t *fs,
                            const char *path, apr_pool_t *pool);
-  svn_error_t *(*get_locks)(svn_fs_t *fs, const char *path,
+  svn_error_t *(*get_locks)(svn_fs_t *fs, const char *path, svn_depth_t depth,
                             svn_fs_get_locks_callback_t get_locks_func,
                             void *get_locks_baton,
                             apr_pool_t *pool);

Modified: subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.c (original)
+++ subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.c Wed Jun 23 01:22:00 2010
@@ -191,6 +191,7 @@ get_lock(svn_lock_t **lock_p,
 svn_error_t *
 svn_fs_bdb__locks_get(svn_fs_t *fs,
                       const char *path,
+                      svn_depth_t depth,
                       svn_fs_get_locks_callback_t get_locks_func,
                       void *get_locks_baton,
                       trail_t *trail,
@@ -225,9 +226,11 @@ svn_fs_bdb__locks_get(svn_fs_t *fs,
         SVN_ERR(get_locks_func(get_locks_baton, lock, pool));
     }
 
+  /* If we're only looking at PATH itself (depth = empty), stop here. */
+  if (depth == svn_depth_empty)
+    return SVN_NO_ERROR;
+
   /* Now go hunt for possible children of PATH. */
-  if (strcmp(path, "/") != 0)
-    lookup_path = apr_pstrcat(pool, path, "/", NULL);
 
   svn_fs_base__trail_debug(trail, "lock-tokens", "cursor");
   db_err = bfd->lock_tokens->cursor(bfd->lock_tokens, trail->db_txn,
@@ -246,6 +249,8 @@ svn_fs_bdb__locks_get(svn_fs_t *fs,
 
   /* As long as the prefix of the returned KEY matches LOOKUP_PATH we
      know it is either LOOKUP_PATH or a decendant thereof.  */
+  if (strcmp(path, "/") != 0)
+    lookup_path = apr_pstrcat(pool, path, "/", NULL);
   while ((! db_err)
          && strncmp(lookup_path, key.data, strlen(lookup_path)) == 0)
     {
@@ -260,6 +265,18 @@ svn_fs_bdb__locks_get(svn_fs_t *fs,
       child_path = apr_pstrmemdup(subpool, key.data, key.size);
       lock_token = apr_pstrmemdup(subpool, value.data, value.size);
 
+      if ((depth == svn_depth_files) || (depth == svn_depth_immediates))
+        {
+          /* On the assumption that we only store locks for files,
+             depth=files and depth=immediates should boil down to the
+             same set of results.  So just see if CHILD_PATH is an
+             immediate child of PATH.  If not, we don't care about
+             this item.   */
+          const char *rel_uri = svn_uri_is_child(path, child_path, subpool);
+          if (!rel_uri || (svn_path_component_count(rel_uri) != 1))
+            goto loop_it;
+        }
+
       /* Get the lock for CHILD_PATH.  */
       err = get_lock(&lock, fs, child_path, lock_token, trail, subpool);
       if (err)
@@ -279,6 +296,7 @@ svn_fs_bdb__locks_get(svn_fs_t *fs,
             }
         }
 
+    loop_it:
       svn_fs_base__result_dbt(&key);
       svn_fs_base__result_dbt(&value);
       db_err = svn_bdb_dbc_get(cursor, &key, &value, DB_NEXT);

Modified: subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.h (original)
+++ subversion/trunk/subversion/libsvn_fs_base/bdb/locks-table.h Wed Jun 23 01:22:00 2010
@@ -86,12 +86,16 @@ svn_error_t *svn_fs_bdb__lock_get(svn_lo
    in FS. Pass each lock to GET_LOCKS_FUNC callback along with
    GET_LOCKS_BATON.
 
+   Use DEPTH to filter the reported locks to only those within the
+   requested depth of PATH.
+
    This function promises to auto-expire any locks encountered while
    building the hash.  That means that the caller can trust that each
    returned lock hasn't yet expired.
 */
 svn_error_t *svn_fs_bdb__locks_get(svn_fs_t *fs,
                                    const char *path,
+                                   svn_depth_t depth,
                                    svn_fs_get_locks_callback_t get_locks_func,
                                    void *get_locks_baton,
                                    trail_t *trail,

Modified: subversion/trunk/subversion/libsvn_fs_base/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/lock.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/lock.c (original)
+++ subversion/trunk/subversion/libsvn_fs_base/lock.c Wed Jun 23 01:22:00 2010
@@ -396,6 +396,7 @@ svn_fs_base__get_lock(svn_lock_t **lock,
 struct locks_get_args
 {
   const char *path;
+  svn_depth_t depth;
   svn_fs_get_locks_callback_t get_locks_func;
   void *get_locks_baton;
 };
@@ -405,7 +406,7 @@ static svn_error_t *
 txn_body_get_locks(void *baton, trail_t *trail)
 {
   struct locks_get_args *args = baton;
-  return svn_fs_bdb__locks_get(trail->fs, args->path,
+  return svn_fs_bdb__locks_get(trail->fs, args->path, args->depth,
                                args->get_locks_func, args->get_locks_baton,
                                trail, trail->pool);
 }
@@ -414,6 +415,7 @@ txn_body_get_locks(void *baton, trail_t 
 svn_error_t *
 svn_fs_base__get_locks(svn_fs_t *fs,
                        const char *path,
+                       svn_depth_t depth,
                        svn_fs_get_locks_callback_t get_locks_func,
                        void *get_locks_baton,
                        apr_pool_t *pool)
@@ -422,6 +424,7 @@ svn_fs_base__get_locks(svn_fs_t *fs,
 
   SVN_ERR(svn_fs__check_fs(fs, TRUE));
   args.path = svn_fs__canonicalize_abspath(path, pool);
+  args.depth = depth;
   args.get_locks_func = get_locks_func;
   args.get_locks_baton = get_locks_baton;
   return svn_fs_base__retry_txn(fs, txn_body_get_locks, &args, FALSE, pool);
@@ -490,7 +493,8 @@ svn_fs_base__allow_locked_operation(cons
   if (recurse)
     {
       /* Discover all locks at or below the path. */
-      SVN_ERR(svn_fs_bdb__locks_get(trail->fs, path, get_locks_callback,
+      SVN_ERR(svn_fs_bdb__locks_get(trail->fs, path, svn_depth_infinity,
+                                    get_locks_callback,
                                     trail->fs, trail, pool));
     }
   else

Modified: subversion/trunk/subversion/libsvn_fs_base/lock.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/lock.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/lock.h (original)
+++ subversion/trunk/subversion/libsvn_fs_base/lock.h Wed Jun 23 01:22:00 2010
@@ -64,6 +64,7 @@ svn_error_t *svn_fs_base__get_lock(svn_l
 svn_error_t *
 svn_fs_base__get_locks(svn_fs_t *fs,
                        const char *path,
+                       svn_depth_t depth,
                        svn_fs_get_locks_callback_t get_locks_func,
                        void *get_locks_baton,
                        apr_pool_t *pool);

Modified: subversion/trunk/subversion/libsvn_fs_fs/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/lock.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/lock.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/lock.c Wed Jun 23 01:22:00 2010
@@ -901,20 +901,82 @@ svn_fs_fs__get_lock(svn_lock_t **lock_p,
 }
 
 
+/* Baton for get_locks_filter_func(). */
+typedef struct
+{
+  const char *path;
+  svn_depth_t requested_depth;
+  svn_fs_get_locks_callback_t get_locks_func;
+  void *get_locks_baton;
+
+} get_locks_filter_baton_t;
+
+
+/* A wrapper for the GET_LOCKS_FUNC passed to svn_fs_fs__get_locks()
+   which filters out locks on paths that aren't within
+   BATON->requested_depth of BATON->path before called
+   BATON->get_locks_func() with BATON->get_locks_baton.
+
+   NOTE: See issue #3660 for details about how the FSFS lock
+   management code is inconsistent.  Until that inconsistency is
+   resolved, we take this filtering approach rather than honoring
+   depth requests closer to the crawling code.  In other words, once
+   we decide how to resolve issue #3660, there might be a more
+   performant way to honor the depth passed to svn_fs_fs__get_locks().  */
+static svn_error_t *
+get_locks_filter_func(void *baton,
+                      svn_lock_t *lock,
+                      apr_pool_t *pool)
+{
+  get_locks_filter_baton_t *b = baton;
+
+  /* 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(b->path, lock->path) == 0) 
+      || (b->requested_depth == svn_depth_infinity))
+    {
+      SVN_ERR(b->get_locks_func(b->get_locks_baton, lock, pool));
+    }
+  else if ((b->requested_depth == svn_depth_files) ||
+           (b->requested_depth == svn_depth_immediates))
+    {
+      const char *rel_uri = svn_uri_is_child(b->path, lock->path, pool);
+      if (rel_uri && (svn_path_component_count(rel_uri) == 1))
+        SVN_ERR(b->get_locks_func(b->get_locks_baton, lock, pool));
+    }
+
+  return SVN_NO_ERROR; 
+}
+
 svn_error_t *
 svn_fs_fs__get_locks(svn_fs_t *fs,
                      const char *path,
+                     svn_depth_t depth,
                      svn_fs_get_locks_callback_t get_locks_func,
                      void *get_locks_baton,
                      apr_pool_t *pool)
 {
   const char *digest_path;
+  get_locks_filter_baton_t glfb;
 
   SVN_ERR(svn_fs__check_fs(fs, TRUE));
   path = svn_fs__canonicalize_abspath(path, pool);
 
+  glfb.path = path;
+  glfb.requested_depth = depth; 
+  glfb.get_locks_func = get_locks_func;
+  glfb.get_locks_baton = get_locks_baton;
+
   /* Get the top digest path in our tree of interest, and then walk it. */
   digest_path = digest_path_from_path(fs, path, pool);
-  return walk_digest_files(fs, digest_path, get_locks_func,
-                           get_locks_baton, FALSE, pool);
+  return walk_digest_files(fs, digest_path, get_locks_filter_func, &glfb,
+                           FALSE, pool);
 }

Modified: subversion/trunk/subversion/libsvn_fs_fs/lock.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/lock.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/lock.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/lock.h Wed Jun 23 01:22:00 2010
@@ -60,6 +60,7 @@ svn_error_t *svn_fs_fs__get_lock(svn_loc
 
 svn_error_t *svn_fs_fs__get_locks(svn_fs_t *fs,
                                   const char *path,
+                                  svn_depth_t depth,
                                   svn_fs_get_locks_callback_t get_locks_func,
                                   void *get_locks_baton,
                                   apr_pool_t *pool);

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.c (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.c Wed Jun 23 01:22:00 2010
@@ -1014,13 +1014,26 @@ svn_error_t *svn_ra_get_lock(svn_ra_sess
   return session->vtable->get_lock(session, lock, path, pool);
 }
 
+svn_error_t *svn_ra_get_locks2(svn_ra_session_t *session,
+                               apr_hash_t **locks,
+                               const char *path,
+                               svn_depth_t depth,
+                               apr_pool_t *pool)
+{
+  SVN_ERR_ASSERT(*path != '/');
+  SVN_ERR_ASSERT((depth == svn_depth_empty) ||
+                 (depth == svn_depth_files) ||
+                 (depth == svn_depth_immediates) ||
+                 (depth == svn_depth_infinity));
+  return session->vtable->get_locks(session, locks, path, depth, pool);
+}
+
 svn_error_t *svn_ra_get_locks(svn_ra_session_t *session,
                               apr_hash_t **locks,
                               const char *path,
                               apr_pool_t *pool)
 {
-  SVN_ERR_ASSERT(*path != '/');
-  return session->vtable->get_locks(session, locks, path, pool);
+  return svn_ra_get_locks2(session, locks, path, svn_depth_infinity, pool);
 }
 
 svn_error_t *svn_ra_replay(svn_ra_session_t *session,

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.h (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.h Wed Jun 23 01:22:00 2010
@@ -230,6 +230,7 @@ typedef struct svn_ra__vtable_t {
   svn_error_t *(*get_locks)(svn_ra_session_t *session,
                             apr_hash_t **locks,
                             const char *path,
+                            svn_depth_t depth,
                             apr_pool_t *pool);
   svn_error_t *(*replay)(svn_ra_session_t *session,
                          svn_revnum_t revision,

Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
+++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Wed Jun 23 01:22:00 2010
@@ -1323,6 +1323,7 @@ static svn_error_t *
 svn_ra_local__get_locks(svn_ra_session_t *session,
                         apr_hash_t **locks,
                         const char *path,
+                        svn_depth_t depth,
                         apr_pool_t *pool)
 {
   svn_ra_local__session_baton_t *sess = session->priv;
@@ -1330,8 +1331,8 @@ svn_ra_local__get_locks(svn_ra_session_t
 
   /* Kinda silly to call the repos wrapper, since we have no authz
      func to give it.  But heck, why not. */
-  return svn_repos_fs_get_locks(locks, sess->repos, abs_path,
-                                NULL, NULL, pool);
+  return svn_repos_fs_get_locks2(locks, sess->repos, abs_path, depth,
+                                 NULL, NULL, pool);
 }
 
 

Modified: subversion/trunk/subversion/libsvn_ra_neon/get_locks.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_neon/get_locks.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_neon/get_locks.c (original)
+++ subversion/trunk/subversion/libsvn_ra_neon/get_locks.c Wed Jun 23 01:22:00 2010
@@ -85,7 +85,7 @@ static const svn_ra_neon__xml_elm_t getl
  * The get-locks-report xml request body is super-simple.
  * The server doesn't need anything but the URI in the REPORT request line.
  *
- *    <S:get-locks-report xmlns...>
+ *    <S:get-locks-report [depth=DEPTH] xmlns...>
  *    </S:get-locks-report>
  *
  * The get-locks-report xml response is just a list of svn_lock_t's
@@ -121,6 +121,8 @@ static const svn_ra_neon__xml_elm_t getl
 
 /* Context for parsing server's response. */
 typedef struct {
+  const char *path;                /* target of the report */
+  svn_depth_t requested_depth;     /* requested depth of the report */
   svn_lock_t *current_lock;        /* the lock being constructed */
   svn_stringbuf_t *cdata_accum;    /* a place to accumulate cdata */
   const char *encoding;            /* normally NULL, else the value of
@@ -235,8 +237,33 @@ getlocks_end_element(void *userdata, int
         SVN_ERR(svn_error_create(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
                                  _("Incomplete lock data returned")));
 
-      apr_hash_set(baton->lock_hash, baton->current_lock->path,
-                   APR_HASH_KEY_STRING, baton->current_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(baton->path, baton->current_lock->path) == 0)
+          || (baton->requested_depth == svn_depth_infinity))
+        {
+          apr_hash_set(baton->lock_hash, baton->current_lock->path,
+                       APR_HASH_KEY_STRING, baton->current_lock);
+        }
+      else if ((baton->requested_depth == svn_depth_files) ||
+               (baton->requested_depth == svn_depth_immediates))
+        {
+          const char *rel_uri = svn_uri_is_child(baton->path,
+                                                 baton->current_lock->path,
+                                                 baton->scratchpool);
+          if (rel_uri && (svn_path_component_count(rel_uri) == 1))
+            apr_hash_set(baton->lock_hash, baton->current_lock->path,
+                         APR_HASH_KEY_STRING, baton->current_lock);
+          svn_pool_clear(baton->scratchpool);
+        }
       break;
 
     case ELEM_lock_path:
@@ -335,15 +362,25 @@ svn_error_t *
 svn_ra_neon__get_locks(svn_ra_session_t *session,
                        apr_hash_t **locks,
                        const char *path,
+                       svn_depth_t depth,
                        apr_pool_t *pool)
 {
   svn_ra_neon__session_t *ras = session->priv;
-  const char *body, *url;
+  const char *body, *url, *rel_path;
   svn_error_t *err;
   int status_code = 0;
   get_locks_baton_t baton;
 
+  /* We always run the report on the 'public' URL, which represents
+     HEAD anyway.  If the path doesn't exist in HEAD, then there can't
+     possibly be a lock, so we just return no locks. */
+  url = svn_path_url_add_component2(ras->url->data, path, pool);
+
+  SVN_ERR(svn_ra_get_path_relative_to_root(session, &rel_path, url, pool));
+
   baton.lock_hash = apr_hash_make(pool);
+  baton.path = apr_pstrcat(pool, "/", rel_path, NULL);
+  baton.requested_depth = depth;
   baton.pool = pool;
   baton.scratchpool = svn_pool_create(pool);
   baton.current_lock = NULL;
@@ -353,14 +390,10 @@ svn_ra_neon__get_locks(svn_ra_session_t 
   body = apr_psprintf(pool,
                       "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
                       "<S:get-locks-report xmlns:S=\"" SVN_XML_NAMESPACE "\" "
-                      "xmlns:D=\"DAV:\">"
-                      "</S:get-locks-report>");
-
+                      "xmlns:D=\"DAV:\" depth=\"%s\">"
+                      "</S:get-locks-report>",
+                      svn_depth_to_word(depth));
 
-  /* We always run the report on the 'public' URL, which represents
-     HEAD anyway.  If the path doesn't exist in HEAD, then there can't
-     possibly be a lock, so we just return no locks. */
-  url = svn_path_url_add_component(ras->url->data, path, pool);
 
   err = svn_ra_neon__parsed_request(ras, "REPORT", url,
                                     body, NULL, NULL,

Modified: subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h (original)
+++ subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h Wed Jun 23 01:22:00 2010
@@ -1022,6 +1022,7 @@ svn_error_t *
 svn_ra_neon__get_locks(svn_ra_session_t *session,
                        apr_hash_t **locks,
                        const char *path,
+                       svn_depth_t depth,
                        apr_pool_t *pool);
 
 /*

Modified: subversion/trunk/subversion/libsvn_ra_serf/getlocks.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/getlocks.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/getlocks.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/getlocks.c Wed Jun 23 01:22:00 2010
@@ -76,6 +76,10 @@ typedef struct {
 typedef struct {
   apr_pool_t *pool;
 
+  /* target and requested depth of the operation. */
+  const char *path; 
+  svn_depth_t requested_depth;
+
   /* return hash */
   apr_hash_t *hash;
 
@@ -180,8 +184,32 @@ end_getlocks(svn_ra_serf__xml_parser_t *
   else if (state == LOCK &&
            strcmp(name.name, "lock") == 0)
     {
-      apr_hash_set(lock_ctx->hash, info->lock->path, APR_HASH_KEY_STRING,
-                   info->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(lock_ctx->path, info->lock->path) == 0)
+          || (lock_ctx->requested_depth == svn_depth_infinity))
+        {
+          apr_hash_set(lock_ctx->hash, info->lock->path,
+                       APR_HASH_KEY_STRING, info->lock);
+        }
+      else if ((lock_ctx->requested_depth == svn_depth_files) ||
+               (lock_ctx->requested_depth == svn_depth_immediates))
+        {
+          const char *rel_uri = svn_uri_is_child(lock_ctx->path,
+                                                 info->lock->path,
+                                                 info->pool);
+          if (rel_uri && (svn_path_component_count(rel_uri) == 1))
+            apr_hash_set(lock_ctx->hash, info->lock->path,
+                         APR_HASH_KEY_STRING, info->lock);
+        }
 
       svn_ra_serf__xml_pop_state(parser);
     }
@@ -272,13 +300,14 @@ create_getlocks_body(void *baton,
                      serf_bucket_alloc_t *alloc,
                      apr_pool_t *pool)
 {
+  lock_context_t *lock_ctx = baton;
   serf_bucket_t *buckets;
 
   buckets = serf_bucket_aggregate_create(alloc);
 
-  svn_ra_serf__add_open_tag_buckets(buckets, alloc, "S:get-locks-report",
-                                    "xmlns:S", SVN_XML_NAMESPACE,
-                                    NULL);
+  svn_ra_serf__add_open_tag_buckets(
+    buckets, alloc, "S:get-locks-report", "xmlns:S", SVN_XML_NAMESPACE,
+    "depth", svn_depth_to_word(lock_ctx->requested_depth), NULL);
   svn_ra_serf__add_close_tag_buckets(buckets, alloc, "S:get-locks-report");
 
   return buckets;
@@ -288,22 +317,27 @@ svn_error_t *
 svn_ra_serf__get_locks(svn_ra_session_t *ra_session,
                        apr_hash_t **locks,
                        const char *path,
+                       svn_depth_t depth,
                        apr_pool_t *pool)
 {
   lock_context_t *lock_ctx;
   svn_ra_serf__session_t *session = ra_session->priv;
   svn_ra_serf__handler_t *handler;
   svn_ra_serf__xml_parser_t *parser_ctx;
-  const char *req_url;
+  const char *req_url, *rel_path;
   int status_code;
 
+  req_url = svn_path_url_add_component2(session->repos_url.path, path, pool);
+  SVN_ERR(svn_ra_serf__get_relative_path(&rel_path, req_url, session,
+                                         NULL, pool));
+
   lock_ctx = apr_pcalloc(pool, sizeof(*lock_ctx));
   lock_ctx->pool = pool;
+  lock_ctx->path = apr_pstrcat(pool, "/", rel_path, NULL);
+  lock_ctx->requested_depth = depth;
   lock_ctx->hash = apr_hash_make(pool);
   lock_ctx->done = FALSE;
 
-  req_url = svn_path_url_add_component2(session->repos_url.path, path, pool);
-
   handler = apr_pcalloc(pool, sizeof(*handler));
 
   handler->method = "REPORT";

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Wed Jun 23 01:22:00 2010
@@ -1347,6 +1347,7 @@ svn_error_t *
 svn_ra_serf__get_locks(svn_ra_session_t *ra_session,
                        apr_hash_t **locks,
                        const char *path,
+                       svn_depth_t depth,
                        apr_pool_t *pool);
 
 svn_error_t * svn_ra_serf__get_mergeinfo(svn_ra_session_t *ra_session,

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));
+  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/libsvn_repos/fs-wrap.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/fs-wrap.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/fs-wrap.c (original)
+++ subversion/trunk/subversion/libsvn_repos/fs-wrap.c Wed Jun 23 01:22:00 2010
@@ -528,10 +528,8 @@ get_locks_callback(void *baton,
 
   /* If there's auth to deal with, deal with it. */
   if (b->authz_read_func)
-    {
-      SVN_ERR(b->authz_read_func(&readable, b->head_root, lock->path,
-                                 b->authz_read_baton, pool));
-    }
+    SVN_ERR(b->authz_read_func(&readable, b->head_root, lock->path,
+                               b->authz_read_baton, pool));
 
   /* If we can read this lock path, add the lock to the return hash. */
   if (readable)
@@ -543,19 +541,23 @@ get_locks_callback(void *baton,
 
 
 svn_error_t *
-svn_repos_fs_get_locks(apr_hash_t **locks,
-                       svn_repos_t *repos,
-                       const char *path,
-                       svn_repos_authz_func_t authz_read_func,
-                       void *authz_read_baton,
-                       apr_pool_t *pool)
+svn_repos_fs_get_locks2(apr_hash_t **locks,
+                        svn_repos_t *repos,
+                        const char *path,
+                        svn_depth_t depth,
+                        svn_repos_authz_func_t authz_read_func,
+                        void *authz_read_baton,
+                        apr_pool_t *pool)
 {
   apr_hash_t *all_locks = apr_hash_make(pool);
   svn_revnum_t head_rev;
   struct get_locks_baton_t baton;
 
-  /* Locks are always said to apply to HEAD revision, so we'll check
-     to see if locked-paths are readable in HEAD as well. */
+  SVN_ERR_ASSERT((depth == svn_depth_empty) ||
+                 (depth == svn_depth_files) ||
+                 (depth == svn_depth_immediates) ||
+                 (depth == svn_depth_infinity));
+
   SVN_ERR(svn_fs_youngest_rev(&head_rev, repos->fs, pool));
 
   /* Populate our callback baton. */
@@ -567,8 +569,8 @@ svn_repos_fs_get_locks(apr_hash_t **lock
                                head_rev, pool));
 
   /* Get all the locks. */
-  SVN_ERR(svn_fs_get_locks(repos->fs, path, get_locks_callback,
-                           &baton, pool));
+  SVN_ERR(svn_fs_get_locks2(repos->fs, path, depth,
+                            get_locks_callback, &baton, pool));
 
   *locks = baton.locks;
   return SVN_NO_ERROR;
@@ -576,6 +578,21 @@ svn_repos_fs_get_locks(apr_hash_t **lock
 
 
 svn_error_t *
+svn_repos_fs_get_locks(apr_hash_t **locks,
+                       svn_repos_t *repos,
+                       const char *path,
+                       svn_repos_authz_func_t authz_read_func,
+                       void *authz_read_baton,
+                       apr_pool_t *pool)
+{
+  return svn_error_return(svn_repos_fs_get_locks2(locks, repos, path,
+                                                  svn_depth_infinity,
+                                                  authz_read_func,
+                                                  authz_read_baton, pool));
+}
+
+
+svn_error_t *
 svn_repos_fs_get_mergeinfo(svn_mergeinfo_catalog_t *mergeinfo,
                            svn_repos_t *repos,
                            const apr_array_header_t *paths,

Modified: subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c?rev=957094&r1=957093&r2=957094&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c Wed Jun 23 01:22:00 2010
@@ -186,6 +186,9 @@ dav_svn__get_locks_report(const dav_reso
   apr_status_t apr_err;
   apr_hash_t *locks;
   dav_svn__authz_read_baton arb;
+  svn_depth_t depth = svn_depth_unknown;
+  apr_pool_t *pool = resource->pool;
+  apr_xml_attr *this_attr;
 
   /* The request URI should be a public one representing an fs path. */
   if ((! resource->info->repos_path)
@@ -197,12 +200,33 @@ dav_svn__get_locks_report(const dav_reso
   arb.r = resource->info->r;
   arb.repos = resource->info->repos;
 
+  /* See if the client provided additional information for this request. */
+  for (this_attr = doc->root->attr; this_attr; this_attr = this_attr->next)
+    {
+      if (strcmp(this_attr->name, "depth") == 0)
+        {
+          depth = svn_depth_from_word(this_attr->value);
+          if ((depth != svn_depth_empty) &&
+              (depth != svn_depth_files) &&
+              (depth != svn_depth_immediates) &&
+              (depth != svn_depth_infinity))
+            return dav_new_error(resource->pool, HTTP_BAD_REQUEST, 0,
+                                 "Invalid 'depth' specified in "
+                                 "get-locks-report request.");
+          continue;
+        }
+    }
+
+  /* For compatibility, our default depth is infinity. */
+  if (depth == svn_depth_unknown)
+    depth = svn_depth_infinity;
+
   /* Fetch the locks, but allow authz_read checks to happen on each. */
-  if ((err = svn_repos_fs_get_locks(&locks,
-                                    resource->info->repos->repos,
-                                    resource->info->repos_path,
-                                    dav_svn__authz_read_func(&arb), &arb,
-                                    resource->pool)) != SVN_NO_ERROR)
+  if ((err = svn_repos_fs_get_locks2(&locks,
+                                     resource->info->repos->repos,
+                                     resource->info->repos_path, depth,
+                                     dav_svn__authz_read_func(&arb), &arb,
+                                     resource->pool)) != SVN_NO_ERROR)
     return dav_svn__convert_err(err, HTTP_INTERNAL_SERVER_ERROR,
                                 err->message, resource->pool);
 

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))



Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Fri, 25 Jun 2010 at 23:14 -0000:
> C. Michael Pilato wrote:
> > Arfrever Frehtes Taifersar Arahesis wrote:
> >> I suggest to create libsvn_ra_util library (similar to libsvn_fs_util).
> >> The code of svn_ra_get_path_relative_to_root() would be moved to
> >> svn_ra__get_path_relative_to_root(), which would be defined in libsvn_ra_util.
> >> libsvn_ra and libsvn_ra_svn would be linked against libsvn_ra_util.
> >> libsvn_ra_svn would use svn_ra__get_path_relative_to_root().
> >> svn_ra_get_path_relative_to_root() would be defined in libsvn_ra and would
> >> wrap svn_ra__get_path_relative_to_root().
> > 
> > But svn_ra_get_path_relative_to_root() and
> > svn_ra_get_path_relative_to_session() call in the RA providers.  So wouldn't
> > that make them dependent on libsvn_ra (another dependency cycle)?
> 
> Is there any way to make this work without just implementing these two
> functions in each of the RA provider libraries, and then only calling their
> library-internal implementations from inside a given provider?
> 

We could duplicate the prototype of svn_ra__vtable_t.get_session_url and
svn_ra__vtable_t.get_repos_root, perhaps?


Anyway, I see that's now fixed in trunk by r958113.

Thanks!

Daniel

> 

Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Arfrever Frehtes Taifersar Arahesis wrote:
>> I suggest to create libsvn_ra_util library (similar to libsvn_fs_util).
>> The code of svn_ra_get_path_relative_to_root() would be moved to
>> svn_ra__get_path_relative_to_root(), which would be defined in libsvn_ra_util.
>> libsvn_ra and libsvn_ra_svn would be linked against libsvn_ra_util.
>> libsvn_ra_svn would use svn_ra__get_path_relative_to_root().
>> svn_ra_get_path_relative_to_root() would be defined in libsvn_ra and would
>> wrap svn_ra__get_path_relative_to_root().
> 
> But svn_ra_get_path_relative_to_root() and
> svn_ra_get_path_relative_to_session() call in the RA providers.  So wouldn't
> that make them dependent on libsvn_ra (another dependency cycle)?

Is there any way to make this work without just implementing these two
functions in each of the RA provider libraries, and then only calling their
library-internal implementations from inside a given provider?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by "C. Michael Pilato" <cm...@collab.net>.
Arfrever Frehtes Taifersar Arahesis wrote:
> I suggest to create libsvn_ra_util library (similar to libsvn_fs_util).
> The code of svn_ra_get_path_relative_to_root() would be moved to
> svn_ra__get_path_relative_to_root(), which would be defined in libsvn_ra_util.
> libsvn_ra and libsvn_ra_svn would be linked against libsvn_ra_util.
> libsvn_ra_svn would use svn_ra__get_path_relative_to_root().
> svn_ra_get_path_relative_to_root() would be defined in libsvn_ra and would
> wrap svn_ra__get_path_relative_to_root().

But svn_ra_get_path_relative_to_root() and
svn_ra_get_path_relative_to_session() call in the RA providers.  So wouldn't
that make them dependent on libsvn_ra (another dependency cycle)?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2010-06-24 18:35:21 Daniel Shahaf napisaƂ(a):
> C. Michael Pilato wrote on Wed, 23 Jun 2010 at 22:43 -0000:
> > Daniel Shahaf wrote:
> > >> -  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).
> > 
> > Fine with me.  (The change I made was sufficient to fix the problem I was
> > seeing in my build.)
> > 
> 
> Looking further, the patch added svn_ra_get_path_relative_to_root() to
> all network-based RA layers.  However, when I try to add libsvn_ra in
> build.conf as suggested above, I just get errors from configure/make
> about circular dependencies :-(
> 
> I'm not sure what's going on here.  But if it breaks in the future,
> hopefully this thread is going to be useful...

This revision breaks building with --enable-disallowing-of-undefined-references
option passed to `configure`.

cd subversion/libsvn_ra_svn && /usr/bin/libtool --tag=CC --silent --mode=link x86_64-pc-linux-gnu-gcc  -march=core2 -pipe -O2   -pthread    -Werror=implicit-function-declaration  -Wl,-O1,--as-needed,--gc-sections,--hash-style=gnu,--sort-common   -L/usr/lib64/qt4  -rpath /usr/lib64 -Wl,--no-undefined -o libsvn_ra_svn-1.la  client.lo cram.lo cyrus_auth.lo editorp.lo internal_auth.lo marshal.lo streams.lo version.lo ../../subversion/libsvn_delta/libsvn_delta-1.la ../../subversion/libsvn_subr/libsvn_subr-1.la -laprutil-1 -lapr-1 -lsasl2 
.libs/client.o: In function `ra_svn_get_locks':
client.c:(.text+0x7ee): undefined reference to `svn_ra_get_path_relative_to_root'
collect2: ld returned 1 exit status
make: *** [subversion/libsvn_ra_svn/libsvn_ra_svn-1.la] Error 1

I suggest to create libsvn_ra_util library (similar to libsvn_fs_util).
The code of svn_ra_get_path_relative_to_root() would be moved to
svn_ra__get_path_relative_to_root(), which would be defined in libsvn_ra_util.
libsvn_ra and libsvn_ra_svn would be linked against libsvn_ra_util.
libsvn_ra_svn would use svn_ra__get_path_relative_to_root().
svn_ra_get_path_relative_to_root() would be defined in libsvn_ra and would
wrap svn_ra__get_path_relative_to_root().

-- 
Arfrever Frehtes Taifersar Arahesis

Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Wed, 23 Jun 2010 at 22:43 -0000:
> Daniel Shahaf wrote:
> >> -  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).
> 
> Fine with me.  (The change I made was sufficient to fix the problem I was
> seeing in my build.)
> 

Looking further, the patch added svn_ra_get_path_relative_to_root() to
all network-based RA layers.  However, when I try to add libsvn_ra in
build.conf as suggested above, I just get errors from configure/make
about circular dependencies :-(

I'm not sure what's going on here.  But if it breaks in the future,
hopefully this thread is going to be useful...

Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Wed, 23 Jun 2010 at 22:43 -0000:
> Daniel Shahaf wrote:
> >> -  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).
> 
> Fine with me.  (The change I made was sufficient to fix the problem I was
> seeing in my build.)
> 

Looking further, the patch added svn_ra_get_path_relative_to_root() to
all network-based RA layers.  However, when I try to add libsvn_ra in
build.conf as suggested above, I just get errors from configure/make
about circular dependencies :-(

I'm not sure what's going on here.  But if it breaks in the future,
hopefully this thread is going to be useful...

Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by "C. Michael Pilato" <cm...@collab.net>.
Daniel Shahaf wrote:
>> -  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).

Fine with me.  (The change I made was sufficient to fix the problem I was
seeing in my build.)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by "C. Michael Pilato" <cm...@collab.net>.
Daniel Shahaf wrote:
>> -  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).

Fine with me.  (The change I made was sufficient to fix the problem I was
seeing in my build.)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
cmpilato@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))
> 
> 
> 

Re: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs*/ subversion/libsvn_ra*/ ...

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
cmpilato@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))
> 
> 
> 

RE: svn commit: r957094 - in /subversion/trunk: ./ subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs/ subversion/libsvn_fs_base/ subversion/libsvn_fs_base/bdb/ subversion/libsvn_fs_fs/ subversion/libsvn_ra/ subversion/libsvn_ra_local/ sub

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
> Sent: woensdag 23 juni 2010 3:22
> To: commits@subversion.apache.org
> Subject: svn commit: r957094 - in /subversion/trunk: ./
> subversion/include/ subversion/libsvn_client/ subversion/libsvn_fs/
> subversion/libsvn_fs_base/ subversion/libsvn_fs_base/bdb/
> subversion/libsvn_fs_fs/ subversion/libsvn_ra/
> subversion/libsvn_ra_local/ subv...
> 
> 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.
> 
> Add depth support to the process of querying repository locks in the
> repository and RA layers for the sake of performance.  Prior to this
> change, 'svn ls -v SOME_PATH' (non-recursive) asked the repository for
> the locks on all paths in and under SOME_PATH (fully recursive),
> resulting in potentially far more information being transmitted across
> the network than is necessary.

Not really connected to resolving this issue, but I think this ra api can be optimized a bit further with the knowledge we build with the wc-ng code.

Currently the locks are always returned as full filesystem paths (in '/trunk/path/to/file' style) instead of as a session relative path (e.g. path/to/file). Switching to session relative paths would allow using the svn_relpath_*() apis on the files and would (slightly) reduce the amount of traffic.

In combination with this change we don't even need feature negotiation: On the client it will be easy to detect which style of paths is returned by just checking if the first path starts with a '/'. The server can switch to relative paths when a depth is passed on the request.

	Bert