[prev in list] [next in list] [prev in thread] [next in thread] 

List:       subversion-cvs
Subject:    svn commit: rev 7592 - in trunk/subversion: include libsvn_wc
From:       kfogel () tigris ! org
Date:       2003-10-31 22:41:47
[Download RAW message or body]

Author: kfogel
Date: Fri Oct 31 16:41:46 2003
New Revision: 7592

Modified:
   trunk/subversion/include/svn_error_codes.h
   trunk/subversion/libsvn_wc/log.c
   trunk/subversion/libsvn_wc/log.h
Log:
Reduce the potential for un-cleanup-able working copies:

When running 'svn cleanup', if the first command in a log file throws
an error, then just remove the log file and clean out the rest of that
.svn/ area.  See issue #1581 for an example of how this helps.

* subversion/include/svn_error_codes.h
  (SVN_ERR_WC_BAD_ADM_LOG_START): New error code.

* subversion/libsvn_wc/log.h: Document that all log elements are
    self-closing tags with attributes.
  (svn_wc__run_log): Document return condition for new error code.

* subversion/libsvn_wc/log.c
  (struct log_runner): Add new count field.
  (start_handler): Increment the count before processing a command.
  (pick_error_code): New helper function.
  (signal_error): Return the new error if this is the first element,
    otherwise return the usual error.
  (everywhere): Use pick_error_code when constructing errors.
  (svn_wc_cleanup): If running a log produces the new error, just
    clear the error, remove the log, and continue with the cleanup.


Modified: trunk/subversion/include/svn_error_codes.h
==============================================================================
--- trunk/subversion/include/svn_error_codes.h	(original)
+++ trunk/subversion/include/svn_error_codes.h	Fri Oct 31 16:41:46 2003
@@ -343,6 +343,10 @@
               SVN_ERR_WC_CATEGORY_START + 19,
               "Invalid operation on the current working directory")  
 
+  SVN_ERRDEF (SVN_ERR_WC_BAD_ADM_LOG_START,
+              SVN_ERR_WC_CATEGORY_START + 20,
+              "Problem on first log entry in a working copy")
+
   /* fs errors */
 
   SVN_ERRDEF (SVN_ERR_FS_GENERAL,

Modified: trunk/subversion/libsvn_wc/log.c
==============================================================================
--- trunk/subversion/libsvn_wc/log.c	(original)
+++ trunk/subversion/libsvn_wc/log.c	Fri Oct 31 16:41:46 2003
@@ -50,6 +50,14 @@
   svn_boolean_t entries_modified;
   svn_wc_adm_access_t *adm_access;  /* the dir in which all this happens */
   const char *diff3_cmd;            /* external diff3 cmd, or null if none */
+
+  /* Which top-level log element we're on for this logfile.  Some
+     callers care whether a failure happened on the first element or
+     on some later element (e.g., 'svn cleanup').
+
+     This is initialized to 0 when the log_runner is created, and
+     incremented every time start_handler() is called. */
+  int count;
 };
 
 
@@ -291,12 +299,21 @@
 }
 
 
+/* Sometimes, documentation would only confuse matters. */
+static apr_status_t
+pick_error_code (struct log_runner *loggy)
+{
+  if (loggy->count <= 1)
+    return SVN_ERR_WC_BAD_ADM_LOG_START;
+  else
+    return SVN_ERR_WC_BAD_ADM_LOG;
+}
+
 static void
 signal_error (struct log_runner *loggy, svn_error_t *err)
 {
   svn_xml_signal_bailout
-    (svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG,
-                        err,
+    (svn_error_createf (pick_error_code (loggy), err,
                         "in directory '%s'",
                         svn_wc_adm_access_path (loggy->adm_access)),
      loggy->parser);
@@ -320,12 +337,12 @@
   /* NAME is the basename of our merge_target.  Pull out LEFT and RIGHT. */
   left = svn_xml_get_attr_value (SVN_WC__LOG_ATTR_ARG_1, atts);
   if (! left)
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+    return svn_error_createf (pick_error_code (loggy), NULL,
                               "missing 'left' attr in '%s'",
                               svn_wc_adm_access_path (loggy->adm_access));
   right = svn_xml_get_attr_value (SVN_WC__LOG_ATTR_ARG_2, atts);
   if (! right)
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+    return svn_error_createf (pick_error_code (loggy), NULL,
                               "missing 'right' attr in '%s'",
                               svn_wc_adm_access_path (loggy->adm_access));
 
@@ -366,7 +383,7 @@
   /* We have the name (src), and the destination is absolutely required. */
   dest = svn_xml_get_attr_value (SVN_WC__LOG_ATTR_DEST, atts);
   if (! dest)
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+    return svn_error_createf (pick_error_code (loggy), NULL,
                               "missing dest attr in '%s'",
                               svn_wc_adm_access_path (loggy->adm_access));
 
@@ -407,7 +424,7 @@
   const char *timestamp_string
     = svn_xml_get_attr_value (SVN_WC__LOG_ATTR_TIMESTAMP, atts);
   if (! timestamp_string)
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+    return svn_error_createf (pick_error_code (loggy), NULL,
                               "missing timestamp attribute in '%s'",
                               svn_wc_adm_access_path (loggy->adm_access));
   
@@ -472,13 +489,13 @@
       err = svn_io_check_path (tfile, &tfile_kind, loggy->pool);
       if (err)
         return svn_error_createf
-          (SVN_ERR_WC_BAD_ADM_LOG, err,
+          (pick_error_code (loggy), err,
            "error checking path '%s'", tfile);
           
       err = svn_io_file_affected_time (&text_time, tfile, loggy->pool);
       if (err)
         return svn_error_createf
-          (SVN_ERR_WC_BAD_ADM_LOG, err,
+          (pick_error_code (loggy), err,
            "error getting file affected time on '%s'", tfile);
 
       entry->text_time = text_time;
@@ -503,13 +520,13 @@
       err = svn_io_check_path (pfile, &pfile_kind, loggy->pool);
       if (err)
         return svn_error_createf
-          (SVN_ERR_WC_BAD_ADM_LOG, err,
+          (pick_error_code (loggy), err,
            "error checking path '%s'", pfile);
       
       err = svn_io_file_affected_time (&prop_time, pfile, loggy->pool);
       if (err)
         return svn_error_createf
-          (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+          (pick_error_code (loggy), NULL,
            "error getting file affected time on '%s'", pfile);
 
       entry->prop_time = prop_time;
@@ -519,7 +536,7 @@
   err = svn_wc__entry_modify (loggy->adm_access, name,
                               entry, modify_flags, FALSE, loggy->pool);
   if (err)
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+    return svn_error_createf (pick_error_code (loggy), err,
                               "error merge_syncing entry '%s'", name);
   loggy->entries_modified = TRUE;
 
@@ -643,7 +660,7 @@
 
   /* If no new post-commit revision was given us, bail with an error. */
   if (! rev)
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+    return svn_error_createf (pick_error_code (loggy), NULL,
                               "missing revision attr for '%s'", name);
       
   /* Read the entry for the affected item.  If we can't find the
@@ -655,7 +672,7 @@
   SVN_ERR (svn_wc_entry (&orig_entry, full_path, adm_access, TRUE, pool));
   if ((! orig_entry)
       || ((! is_this_dir) && (orig_entry->kind != svn_node_file)))
-    return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+    return svn_error_createf (pick_error_code (loggy), NULL,
                               "log command for dir '%s' is mislocated", name);
 
   entry = svn_wc_entry_dup (orig_entry, pool);
@@ -822,7 +839,7 @@
       /* Make sure our working file copy is present in the temp area. */
       tmpf = svn_wc__text_base_path (wf, 1, pool);
       if ((err = svn_io_check_path (tmpf, &kind, pool)))
-        return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+        return svn_error_createf (pick_error_code (loggy), err,
                                   "error checking existence: %s", name);
       if (kind == svn_node_file)
         {
@@ -833,7 +850,7 @@
           if ((err = svn_wc__versioned_file_modcheck (&modified, wf,
                                                       loggy->adm_access,
                                                       tmpf, pool)))
-            return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+            return svn_error_createf (pick_error_code (loggy), err,
                                       "error comparing '%s' and '%s'",
                                       wf, tmpf);
 
@@ -843,7 +860,7 @@
 
           /* Get the timestamp from our chosen file. */
           if ((err = svn_io_file_affected_time (&text_time, chosen, pool)))
-            return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+            return svn_error_createf (pick_error_code (loggy), err,
                                       "error getting affected time: %s",
                                       chosen);
         }
@@ -892,7 +909,7 @@
               ? svn_wc_adm_access_path (loggy->adm_access) : full_path,
               loggy->adm_access, TRUE, pool));
     if ((err = svn_io_check_path (tmpf, &kind, pool)))
-      return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+      return svn_error_createf (pick_error_code (loggy), err,
                                 "error checking existence: %s", name);
     if (kind == svn_node_file)
       {
@@ -902,7 +919,7 @@
         /* We need to decide which prop-timestamp to use, just like we
            did with text-time above. */
         if ((err = svn_io_files_contents_same_p (&same, wf, tmpf, pool)))
-          return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+          return svn_error_createf (pick_error_code (loggy), err,
                                     "error comparing '%s' and '%s'",
                                     wf, tmpf);
 
@@ -912,7 +929,7 @@
 
         /* Get the timestamp of our chosen file. */
         if ((err = svn_io_file_affected_time (&prop_time, chosen, pool)))
-          return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+          return svn_error_createf (pick_error_code (loggy), err,
                                     "error getting affected time: %s",
                                     chosen);
 
@@ -957,7 +974,7 @@
       /* Install the new file, which may involve expanding keywords. */
       if ((err = install_committed_file
            (&overwrote_working, loggy->adm_access, name, pool)))
-        return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+        return svn_error_createf (pick_error_code (loggy), err,
                                   "error replacing text-base: %s", name);
 
       /* The previous call will have run +x if the executable property
@@ -977,7 +994,7 @@
          timestamp instead. */
       if (overwrote_working)
         if ((err = svn_io_file_affected_time (&text_time, full_path, pool)))
-          return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+          return svn_error_createf (pick_error_code (loggy), err,
                                     "error getting affected time: %s",
                                     full_path);
     }
@@ -1017,7 +1034,7 @@
                                     | SVN_WC__ENTRY_MODIFY_FORCE),
                                    FALSE, pool)))
     return svn_error_createf
-      (SVN_ERR_WC_BAD_ADM_LOG, err,
+      (pick_error_code (loggy), err,
        "error modifying entry: %s", name);
   loggy->entries_modified = TRUE;
 
@@ -1065,7 +1082,7 @@
                                           | SVN_WC__ENTRY_MODIFY_DELETED
                                           | SVN_WC__ENTRY_MODIFY_FORCE),
                                          TRUE, pool)))
-          return svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG, err,
+          return svn_error_createf (pick_error_code (loggy), err,
                                     "error merge_syncing '%s'", name);
       }
 
@@ -1124,12 +1141,15 @@
     {
       signal_error
         (loggy, svn_error_createf 
-         (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+         (pick_error_code (loggy), NULL,
           "log entry missing name attribute (entry '%s' for dir '%s')",
           eltname, svn_wc_adm_access_path (loggy->adm_access)));
       return;
     }
   
+  /* Increment the top-level element count before processing any commands. */
+  loggy->count += 1;
+
   /* Dispatch. */
   if (strcmp (eltname, SVN_WC__LOG_MODIFY_ENTRY) == 0) {
     err = log_do_modify_entry (loggy, name, atts);
@@ -1173,7 +1193,7 @@
   else
     {
       signal_error
-        (loggy, svn_error_createf (SVN_ERR_WC_BAD_ADM_LOG,
+        (loggy, svn_error_createf (pick_error_code (loggy),
                                    NULL,
                                    "unrecognized logfile element in '%s': '%s'",
                                    svn_wc_adm_access_path (loggy->adm_access),
@@ -1184,7 +1204,7 @@
   if (err)
     signal_error
       (loggy, svn_error_createf
-       (SVN_ERR_WC_BAD_ADM_LOG, err,
+       (pick_error_code (loggy), err,
         "start_handler: error processing command '%s' in '%s'",
         eltname, svn_wc_adm_access_path (loggy->adm_access)));
   
@@ -1220,6 +1240,7 @@
   loggy->parser = parser;
   loggy->entries_modified = FALSE;
   loggy->diff3_cmd = diff3_cmd;
+  loggy->count = 0;
 
   /* Expat wants everything wrapped in a top-level form, so start with
      a ghost open tag. */
@@ -1396,7 +1417,31 @@
       /* 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, diff3_cmd, pool));
+        {
+          svn_error_t *err = svn_wc__run_log (adm_access, diff3_cmd, pool);
+
+          /* If a log run fails on the first element, it is safe for
+             cleanup to just remove the entire log file and clean up
+             the associated tmp areas, because the log is an entire
+             atomic unit that can either be all not run or all run.
+             
+             But if it fails somewhere after the first element, then
+             we can't even proceed with the cleanup. */
+          if (err && (err->apr_err == SVN_ERR_WC_BAD_ADM_LOG_START))
+            {
+              svn_error_clear (err);
+              err = NULL;  /* not used again, but paranoia pays */
+              SVN_ERR (svn_io_remove_file (log_path, pool));
+            }
+          else if (err)
+            return err;
+        }
+      else
+        {
+          return svn_error_createf
+            (SVN_ERR_WC_BAD_ADM_LOG, NULL,
+             "svn_wc_cleanup: '%s' should be a file, but is not", log_path);
+        }
     }
 
   /* Cleanup the tmp area of the admin subdir, if running the log has not

Modified: trunk/subversion/libsvn_wc/log.h
==============================================================================
--- trunk/subversion/libsvn_wc/log.h	(original)
+++ trunk/subversion/libsvn_wc/log.h	Fri Oct 31 16:41:46 2003
@@ -38,6 +38,8 @@
  * recovery, a given entry is "safe" in the sense that you can either
  * tell it has already been done (in which case, ignore it) or you can
  * do it again without ill effect.
+ *
+ * All log commands are self-closing tags with attributes.
  */
 
 /** Log actions. **/
@@ -134,7 +136,11 @@
 
 /* Process the instructions in the log file for ADM_ACCESS. 
    DIFF3_CMD is the external differ used by the 'SVN_WC__LOG_MERGE'
-   log entry.  It is always safe to pass null for this. */
+   log entry.  It is always safe to pass null for this.
+
+   If the log fails on its first command, return the error
+   SVN_ERR_WC_BAD_ADM_LOG_START.  If it fails on some subsequent
+   command, return SVN_ERR_WC_BAD_ADM_LOG. */
 svn_error_t *svn_wc__run_log (svn_wc_adm_access_t *adm_access,
                               const char *diff3_cmd,
                               apr_pool_t *pool);

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

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic