You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/01/10 21:54:40 UTC

svn commit: r1431633 - in /subversion/trunk/subversion: libsvn_wc/cleanup.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/cmdline/wc_tests.py

Author: brane
Date: Thu Jan 10 20:54:40 2013
New Revision: 1431633

URL: http://svn.apache.org/viewvc?rev=1431633&view=rev
Log:
Fix issue #4267 (better message when cleanup not run on lock root); but instead
of concocting a better error message, just always run cleanup on the lock root.

* subversion/libsvn_wc/wc_db.h (svn_wc__db_wclock_find_root): New.
* subversion/libsvn_wc/wc_db.c (find_wclock): Renamed from is_wclocked,
   and returns the path to the root of the locked subtree instead of a
   boolean flag.
  (is_wclocked): Reimplemented as a wrapper around find_wclock.
  (svn_wc__db_wclock_find_root): Find the root of a locked WC subtree.
* subversion/libsvn_wc/cleanup.c (cleanup_internal): If the cleanup target
   is locked, run cleanup from the root of the locked subtree.

* subversion/tests/cmdline/wc_tests.py (cleanup_below_wc_root): Remove XFail tag.

Modified:
    subversion/trunk/subversion/libsvn_wc/cleanup.c
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/tests/cmdline/wc_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/cleanup.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/cleanup.c?rev=1431633&r1=1431632&r2=1431633&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/cleanup.c (original)
+++ subversion/trunk/subversion/libsvn_wc/cleanup.c Thu Jan 10 20:54:40 2013
@@ -143,12 +143,17 @@ cleanup_internal(svn_wc__db_t *db,
 {
   int wc_format;
   svn_boolean_t is_wcroot;
+  const char *lock_abspath;
 
   /* Can we even work with this directory?  */
   SVN_ERR(can_be_cleaned(&wc_format, db, dir_abspath, scratch_pool));
 
-  /* ### This fails if ADM_ABSPATH is locked indirectly via a
-     ### recursive lock on an ancestor. */
+  /* We cannot obtain a lock on a directory that's within a locked
+     subtree, so always run cleanup from the lock owner. */
+  SVN_ERR(svn_wc__db_wclock_find_root(&lock_abspath, db, dir_abspath,
+                                      scratch_pool, scratch_pool));
+  if (lock_abspath)
+    dir_abspath = lock_abspath;
   SVN_ERR(svn_wc__db_wclock_obtain(db, dir_abspath, -1, TRUE, scratch_pool));
 
   /* Run our changes before the subdirectories. We may not have to recurse

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1431633&r1=1431632&r2=1431633&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Jan 10 20:54:40 2013
@@ -12899,12 +12899,12 @@ svn_wc__db_wclock_obtain(svn_wc__db_t *d
 }
 
 
-/* The body of svn_wc__db_wclocked().
- */
+/* The body of svn_wc__db_wclock_find_root() and svn_wc__db_wclocked(). */
 static svn_error_t *
-is_wclocked(svn_boolean_t *locked,
+find_wclock(const char **lock_relpath,
             svn_wc__db_wcroot_t *wcroot,
             const char *dir_relpath,
+            apr_pool_t *result_pool,
             apr_pool_t *scratch_pool)
 {
   svn_sqlite__stmt_t *stmt;
@@ -12948,7 +12948,7 @@ is_wclocked(svn_boolean_t *locked,
           if (locked_levels == -1
               || locked_levels + row_depth >= dir_depth)
             {
-              *locked = TRUE;
+              *lock_relpath = apr_pstrdup(result_pool, relpath);
               SVN_ERR(svn_sqlite__reset(stmt));
               return SVN_NO_ERROR;
             }
@@ -12957,11 +12957,54 @@ is_wclocked(svn_boolean_t *locked,
       SVN_ERR(svn_sqlite__step(&have_row, stmt));
     }
 
-  *locked = FALSE;
+  *lock_relpath = NULL;
 
   return svn_error_trace(svn_sqlite__reset(stmt));
 }
 
+static svn_error_t *
+is_wclocked(svn_boolean_t *locked,
+            svn_wc__db_wcroot_t *wcroot,
+            const char *dir_relpath,
+            apr_pool_t *scratch_pool)
+{
+  const char *lock_relpath;
+
+  SVN_ERR(find_wclock(&lock_relpath, wcroot, dir_relpath,
+                      scratch_pool, scratch_pool));
+  *locked = (lock_relpath != NULL);
+  return SVN_NO_ERROR;
+}
+
+
+svn_error_t*
+svn_wc__db_wclock_find_root(const char **lock_abspath,
+                            svn_wc__db_t *db,
+                            const char *local_abspath,
+                            apr_pool_t *result_pool,
+                            apr_pool_t *scratch_pool)
+{
+  svn_wc__db_wcroot_t *wcroot;
+  const char *local_relpath;
+  const char *lock_relpath;
+
+  SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &local_relpath, db,
+                              local_abspath, scratch_pool, scratch_pool));
+  VERIFY_USABLE_WCROOT(wcroot);
+
+  SVN_WC__DB_WITH_TXN(
+    find_wclock(&lock_relpath, wcroot, local_relpath,
+                scratch_pool, scratch_pool),
+    wcroot);
+
+  if (!lock_relpath)
+    *lock_abspath = NULL;
+  else
+    SVN_ERR(svn_wc__db_from_relpath(lock_abspath, db, wcroot->abspath,
+                                    lock_relpath, result_pool, scratch_pool));
+  return SVN_NO_ERROR;
+}
+
 
 svn_error_t *
 svn_wc__db_wclocked(svn_boolean_t *locked,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1431633&r1=1431632&r2=1431633&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Thu Jan 10 20:54:40 2013
@@ -2899,6 +2899,15 @@ svn_wc__db_wclock_obtain(svn_wc__db_t *d
                          svn_boolean_t steal_lock,
                          apr_pool_t *scratch_pool);
 
+/* Set LOCK_ABSPATH to the path of the the directory that owns the
+   lock on LOCAL_ABSPATH, or NULL, if LOCAL_ABSPATH is not locked. */
+svn_error_t*
+svn_wc__db_wclock_find_root(const char **lock_abspath,
+                            svn_wc__db_t *db,
+                            const char *local_abspath,
+                            apr_pool_t *result_pool,
+                            apr_pool_t *scratch_pool);
+
 /* Check if somebody has a wclock on LOCAL_ABSPATH */
 svn_error_t *
 svn_wc__db_wclocked(svn_boolean_t *locked,

Modified: subversion/trunk/subversion/tests/cmdline/wc_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/wc_tests.py?rev=1431633&r1=1431632&r2=1431633&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/wc_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/wc_tests.py Thu Jan 10 20:54:40 2013
@@ -192,7 +192,6 @@ def status_with_missing_wc_db_and_maybe_
 
 
 @Issue(4267)
-@XFail()
 def cleanup_below_wc_root(sbox):
   """cleanup from directory below WC root"""
 



RE: svn commit: r1431633 - in /subversion/trunk/subversion: libsvn_wc/cleanup.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/cmdline/wc_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: vrijdag 11 januari 2013 14:12
> To: dev@subversion.apache.org
> Subject: RE: svn commit: r1431633 - in /subversion/trunk/subversion:
> libsvn_wc/cleanup.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h
> tests/cmdline/wc_tests.py
> 
> 
> 
> > -----Original Message-----
> > From: brane@apache.org [mailto:brane@apache.org]
> > Sent: donderdag 10 januari 2013 21:55
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1431633 - in /subversion/trunk/subversion:
> > libsvn_wc/cleanup.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h
> > tests/cmdline/wc_tests.py
> >
> > Author: brane
> > Date: Thu Jan 10 20:54:40 2013
> > New Revision: 1431633
> >
> > URL: http://svn.apache.org/viewvc?rev=1431633&view=rev
> > Log:
> > Fix issue #4267 (better message when cleanup not run on lock root); but
> > instead
> > of concocting a better error message, just always run cleanup on the lock
> > root.
> 
> During WC-NG development we explicitly decided not to do this, because
> some users run operations on different parts of their working copy at once.
[Already answered on IRC]

No, we decided not to always run it on the wcroot. 

This change by itself is safe, and should really improve usability.

Thanks Brane,

	Bert


RE: svn commit: r1431633 - in /subversion/trunk/subversion: libsvn_wc/cleanup.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/cmdline/wc_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: brane@apache.org [mailto:brane@apache.org]
> Sent: donderdag 10 januari 2013 21:55
> To: commits@subversion.apache.org
> Subject: svn commit: r1431633 - in /subversion/trunk/subversion:
> libsvn_wc/cleanup.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h
> tests/cmdline/wc_tests.py
> 
> Author: brane
> Date: Thu Jan 10 20:54:40 2013
> New Revision: 1431633
> 
> URL: http://svn.apache.org/viewvc?rev=1431633&view=rev
> Log:
> Fix issue #4267 (better message when cleanup not run on lock root); but
> instead
> of concocting a better error message, just always run cleanup on the lock
> root.

During WC-NG development we explicitly decided not to do this, because some users run operations on different parts of their working copy at once.

$ svn up wc/branches/subdir 
$ svn up wc/tags/subdir

These can run simultaneously.

This was an explicitly supported scenario in 1.6 and earlier.

If one of these commands would break down, you would call 'svn cleanup' on just that target. For 1.7 this still removes the working copy lock and runs the workqueue.


For WC-NG there are certain operations you can't do without a full working copy lock: e.g. cleaning pristines while somebody else has a lock (or when there are wq items)

Your change made it much easier to perform these very dangerous operations, breaking the lock of the second process, while the normal cleanup probably already worked. 
(Any process is allowed to run the workqueue at any time)

	Bert