Re: still inconsistent, file in repository, but not in wc

solo turn <soloturn99@yahoo.com> writes:

> example for lock:
> $ svn up # do this in root with wrong proxy passwort
> --> the whole directory gets locked and not unlocked
> any more

My lock removal during pool cleanup patch will fix that. Since
removing locks is hazardous (i.e. could lead to working copy
corruption) I haven't applied it yet, I asked whether simply checking
for the existance of a log file was sufficient to determine whether to
remove a lock. I'm happy with the patch myself, and nobody else has
had any comments, so I may decide to commit it soon.

Continuing issue 749. Allow pool cleanup to remove working copy locks
if possible.

* subversion/include/svn_wc.h (svn_wc_adm_open, svn_wc_adm_close): Document
  new behaviour.

* subversion/libsvn_wc/wc.h
  (svn_wc__adm_is_cleanup_required): New function.

* subversion/libsvn_wc/lock.c
  (svn_wc__adm_is_cleanup_required): New function.
  (pool_cleanup): Preserve physical locks only if required by
  svn_wc__is_cleanup_required.

* subversion/libsvn_wc/log.c
  (svn_wc_cleanup): Do critical cleanup only if svn_wc__is_cleanup_required
  requires it.

* subversion/tests/clients/cmdline/basic_tests.py
  (basic_corruption): Remove cleanup operation (reverts r2941).

* subversion/tests/clients/cmdline/copy_tests.py
  (no_wc_copy_overwrites): Remove cleanup operations (reverts r2941).

* subversion/tests/clients/cmdline/commit_tests.py (commit_with_lock): Use
  lock_admin_dir and cleanup.

Index: ../svn/subversion/include/svn_wc.h
===================================================================
--- ../svn/subversion/include/svn_wc.h
+++ ../svn/subversion/include/svn_wc.h Tue Aug 20 20:55:14 2002
@@ -83,7 +83,7 @@
    cached items. If ADM_ACCESS has not been closed when the pool is
    cleared, it will be closed automatically at that point, and removed from
    its set. A baton closed in this way will not remove physical locks from
- the working copy. */
+ the working copy if cleanup is required. */
 svn_error_t *svn_wc_adm_open (svn_wc_adm_access_t **adm_access,
                               svn_wc_adm_access_t *associated,
                               const char *path,
@@ -105,8 +105,9 @@
 
 /* Give up the access baton ADM_ACCESS, and its lock if any. This will
    recursively close any batons in the same set that are subdirectories of
- ADM_ACCESS. Any physical locks will be removed from the working
- copy. */
+ ADM_ACCESS. Any physical locks will be removed from the working copy.
+ Lock removal is unconditional, there is no check to determine if cleanup
+ is required. */
 svn_error_t *svn_wc_adm_close (svn_wc_adm_access_t *adm_access);
 
 /* Return the (canonicalized) path used to open the access baton
Index: ../svn/subversion/libsvn_wc/wc.h
===================================================================
--- ../svn/subversion/libsvn_wc/wc.h
+++ ../svn/subversion/libsvn_wc/wc.h Tue Aug 20 20:55:14 2002
@@ -142,7 +142,14 @@
 
 /* Tell ADM_ACCESS that we are removing its physical lock, so that it can
    update its state information. */
-void svn_wc__adm_forced_lock_removal(svn_wc_adm_access_t *adm_access);
+void svn_wc__adm_forced_lock_removal (svn_wc_adm_access_t *adm_access);
+
+
+/* Set *CLEANUP to TRUE if the directory ADM_ACCESS requires cleanup; set
+ *CLEANUP to FALSE otherwise. */
+svn_error_t *svn_wc__adm_is_cleanup_required (svn_boolean_t *cleanup,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool);
 
 #ifdef __cplusplus
 }
Index: ../svn/subversion/libsvn_wc/log.c
===================================================================
--- ../svn/subversion/libsvn_wc/log.c
+++ ../svn/subversion/libsvn_wc/log.c Tue Aug 20 21:13:40 2002
@@ -1291,7 +1291,7 @@
                                            SVN_WC__ADM_LOG, NULL);
   enum svn_node_kind kind;
   svn_wc_adm_access_t *adm_access;
- svn_boolean_t is_wc;
+ svn_boolean_t is_wc, cleanup;
 
   SVN_ERR (svn_wc_check_wc (path, &is_wc, pool));
   if (! is_wc)
@@ -1324,10 +1324,18 @@
         }
     }
 
- /* Is there a log? If so, run it. */
- SVN_ERR (svn_io_check_path (log_path, &kind, pool));
- if (kind == svn_node_file)
- SVN_ERR (svn_wc__run_log (adm_access, pool));
+ /* As an attempt to maintain consitency between the decisions made in
+ this function, and those made in the access baton lock-removal code,
+ we use the same test as the lock-removal code even though it is,
+ strictly speaking, redundant. */
+ SVN_ERR (svn_wc__adm_is_cleanup_required (&cleanup, adm_access, pool));
+ if (cleanup)
+ {
+ /* Is there a log? If so, run it. */
+ SVN_ERR (svn_io_check_path (log_path, &kind, pool));
+ if (kind == svn_node_file)
+ SVN_ERR (svn_wc__run_log (adm_access, pool));
+ }
 
   /* Cleanup the tmp area of the admin subdir, if running the log has not
      removed it! The logs have been run, so anything left here has no hope
Index: ../svn/subversion/libsvn_wc/lock.c
===================================================================
--- ../svn/subversion/libsvn_wc/lock.c
+++ ../svn/subversion/libsvn_wc/lock.c Tue Aug 20 20:55:14 2002
@@ -139,9 +139,12 @@
 pool_cleanup (void *p)
 {
   svn_wc_adm_access_t *lock = p;
+ svn_boolean_t cleanup;
   svn_error_t *err;
 
- err = do_close (lock, TRUE);
+ err = svn_wc__adm_is_cleanup_required (&cleanup, lock, lock->pool);
+ if (!err)
+ err = do_close (lock, cleanup);
 
   /* ### Is this the correct way to handle the error? */
   if (err)
@@ -508,6 +511,22 @@
 svn_wc__adm_forced_lock_removal (svn_wc_adm_access_t *adm_access)
 {
   adm_access->lock_exists = FALSE;
+}
+
+svn_error_t *
+svn_wc__adm_is_cleanup_required (svn_boolean_t *cleanup,
+ svn_wc_adm_access_t *adm_access,
+ apr_pool_t *pool)
+{
+ enum svn_node_kind kind;
+ const char *log_path = svn_wc__adm_path (svn_wc_adm_access_path (adm_access),
+ FALSE, pool, SVN_WC__ADM_LOG, NULL);
+
+ /* The presence of a log file demands cleanup */
+ SVN_ERR (svn_io_check_path (log_path, &kind, pool));
+ *cleanup = (kind == svn_node_file);
+
+ return SVN_NO_ERROR;
 }
 
 
Index: ../svn/subversion/tests/clients/cmdline/copy_tests.py
===================================================================
--- ../svn/subversion/tests/clients/cmdline/copy_tests.py
+++ ../svn/subversion/tests/clients/cmdline/copy_tests.py Tue Aug 20 20:55:14 2002
@@ -516,15 +516,12 @@
   outlines, errlines = svntest.main.run_svn(1, 'cp', pi_path, rho_path)
   if not errlines:
     return 1
- svntest.main.run_svn(None, 'cleanup', wc_dir)
   outlines, errlines = svntest.main.run_svn(1, 'cp', pi_path, tau_path)
   if not errlines:
     return 1
- svntest.main.run_svn(None, 'cleanup', wc_dir)
   outlines, errlines = svntest.main.run_svn(1, 'cp', pi_path, alpha_path)
   if not errlines:
     return 1
- svntest.main.run_svn(None, 'cleanup', wc_dir)
 
   # Status after failed copies should not have changed
   extra_files = [ 'tau' ]
Index: ../svn/subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- ../svn/subversion/tests/clients/cmdline/basic_tests.py
+++ ../svn/subversion/tests/clients/cmdline/basic_tests.py Tue Aug 20 20:55:14 2002
@@ -337,11 +337,8 @@
   os.chmod (tb_dir_path, tb_dir_saved_mode)
   os.chmod (mu_tb_path, mu_tb_saved_mode)
 
- outlines, errlines = svntest.main.run_svn(None, 'cleanup', other_wc)
- if errlines:
- return 1
-
- # This update should succeed.
+ # This update should succeed. (Actually, I'm kind of astonished
+ # that this works without even an intervening "svn cleanup".)
   return svntest.actions.run_and_verify_update (other_wc,
                                                 expected_output,
                                                 expected_disk,
Index: ../svn/subversion/tests/clients/cmdline/commit_tests.py
===================================================================
--- ../svn/subversion/tests/clients/cmdline/commit_tests.py
+++ ../svn/subversion/tests/clients/cmdline/commit_tests.py Tue Aug 20 20:55:14 2002
@@ -1277,10 +1277,10 @@
 
   # modify gamma and lock its directory
   wc_dir = sbox.wc_dir
- gamma_path = os.path.join(wc_dir, 'A', 'D', 'gamma')
- gamma_lock_path = os.path.join(wc_dir, 'A', 'D', '.svn', 'lock')
+ D_path = os.path.join(wc_dir, 'A', 'D')
+ gamma_path = os.path.join(D_path, 'gamma')
   svntest.main.file_append(gamma_path, "modified gamma")
- svntest.main.file_append(gamma_lock_path, "")
+ svntest.actions.lock_admin_dir(D_path)
 
   # this commit should fail
   if svntest.actions.run_and_verify_commit(wc_dir,
@@ -1293,7 +1293,9 @@
     return 1
                                            
   # unlock directory
- os.remove(gamma_lock_path)
+ outlines, errlines = svntest.main.run_svn(None, 'cleanup', D_path)
+ if errlines:
+ return 1
 
   # this commit should succeed
   expected_output = svntest.wc.State(wc_dir, {

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org