[PATCH] Issue 3501: Repositories mounted on NFS don't work

[Peter Samuelson]
> Something like the following (untested) against 1.6.x.

Well, it almost worked. Here's one, also for 1.6.x, that seems to pass
the tests. Edgar, can you test this on your NFS client?

I'll commit the trunk version of this soon if no one objects. It
shouldn't need a new test; issue 3501 causes lots of test failures
already.

-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
[[[
Fix issue #3501: svn_io_remove_dir2 failing due to NFS caching.
* subversion/libsvn_subr/io.c
  (svn_io_remove_dir2): Remove rewinddir() magic from Issue 1896
   fix.  Instead, build linked lists of files and subdirs to delete,
   then delete them in separate loops.
]]]
--- subversion/libsvn_subr/io.c
+++ subversion/libsvn_subr/io.c
@@ -1862,17 +1862,13 @@
 
  Similar problem has been observed on FreeBSD.
 
- See http://subversion.tigris.org/issues/show_bug.cgi?id=1896 for more
- discussion and an initial solution.
+ The workaround is to collect filenames in the readdir loop, then
+ delete them in a separate loop (two, actually, one for subdirs and one
+ for other files).  A previous workaround involving rewinddir is
+ problematic on Win32 and some NFS clients, notably NetBSD.
 
- To work around the problem, we do a rewinddir after we delete all files
- and see if there's anything left. We repeat the steps untill there's
- nothing left to delete.
-
- This workaround causes issues on Windows where delete's are asynchronous,
- however, so we never rewind if we're on Windows (the delete says it is
- complete, we rewind, we see the same file and try to delete it again,
- we fail.
+ See http://subversion.tigris.org/issues/show_bug.cgi?id=1896 and
+ http://subversion.tigris.org/issues/show_bug.cgi?id=3501.
 */
 
 /* Neither windows nor unix allows us to delete a non-empty
@@ -1890,7 +1886,8 @@
   apr_pool_t *subpool;
   apr_int32_t flags = APR_FINFO_TYPE | APR_FINFO_NAME;
   const char *path_apr;
-  int need_rewind;
+  struct path_list { char *path; struct path_list *next; }
+    *del_files = NULL, *del_dirs = NULL, *del_tmp;
 
   /* Check for pending cancellation request.
      If we need to bail out, do so early. */
@@ -1922,72 +1919,36 @@
 
   subpool = svn_pool_create(pool);
 
-  do
+  while ((status = apr_dir_read(&this_entry, flags, this_dir)) == APR_SUCCESS)
     {
-      need_rewind = FALSE;
-
-      for (status = apr_dir_read(&this_entry, flags, this_dir);
-           status == APR_SUCCESS;
-           status = apr_dir_read(&this_entry, flags, this_dir))
+      const char *entry_utf8;
+      if ((this_entry.filetype == APR_DIR)
+          && ((this_entry.name[0] == '.')
+              && ((this_entry.name[1] == '\0')
+                  || ((this_entry.name[1] == '.')
+                      && (this_entry.name[2] == '\0')))))
         {
-          svn_pool_clear(subpool);
-          if ((this_entry.filetype == APR_DIR)
-              && ((this_entry.name[0] == '.')
-                  && ((this_entry.name[1] == '\0')
-                      || ((this_entry.name[1] == '.')
-                          && (this_entry.name[2] == '\0')))))
-            {
-              continue;
-            }
-          else  /* something other than "." or "..", so proceed */
-            {
-              const char *fullpath, *entry_utf8;
+          /* skip "." and ".." */
+          continue;
+        }
+      SVN_ERR(entry_name_to_utf8(&entry_utf8, this_entry.name,
+                                 path_apr, subpool));
 
-#ifndef WIN32
-              need_rewind = TRUE;
-#endif
+      del_tmp = apr_palloc(subpool, sizeof *del_tmp);
+      del_tmp->path = svn_path_join(path, entry_utf8, subpool);
 
-              SVN_ERR(entry_name_to_utf8(&entry_utf8, this_entry.name,
-                                         path_apr, subpool));
-
-              fullpath = svn_path_join(path, entry_utf8, subpool);
-
-              if (this_entry.filetype == APR_DIR)
-                {
-                  /* Don't check for cancellation, the callee
-                     will immediately do so */
-                  SVN_ERR(svn_io_remove_dir2(fullpath, FALSE,
-                                             cancel_func, cancel_baton,
-                                             subpool));
-                }
-              else
-                {
-                  svn_error_t *err;
-
-                  if (cancel_func)
-                    SVN_ERR((*cancel_func)(cancel_baton));
-
-                  err = svn_io_remove_file(fullpath, subpool);
-                  if (err)
-                    return svn_error_createf
-                      (err->apr_err, err, _("Can't remove '%s'"),
-                       svn_path_local_style(fullpath, subpool));
-                }
-            }
+      if (this_entry.filetype == APR_DIR)
+        {
+          del_tmp->next = del_dirs;
+          del_dirs = del_tmp;
         }
-
-      if (need_rewind)
+      else
         {
-          status = apr_dir_rewind(this_dir);
-          if (status)
-            return svn_error_wrap_apr(status, _("Can't rewind directory '%s'"),
-                                      svn_path_local_style (path, pool));
+          del_tmp->next = del_files;
+          del_files = del_tmp;
         }
     }
-  while (need_rewind);
 
-  svn_pool_destroy(subpool);
-
   if (!APR_STATUS_IS_ENOENT(status))
     return svn_error_wrap_apr(status, _("Can't read directory '%s'"),
                               svn_path_local_style(path, pool));
@@ -1997,6 +1958,29 @@
     return svn_error_wrap_apr(status, _("Error closing directory '%s'"),
                               svn_path_local_style(path, pool));
 
+  for (; del_dirs; del_dirs = del_dirs->next)
+    {
+      /* Don't check for cancellation, the callee will immediately do so */
+      SVN_ERR(svn_io_remove_dir2(del_dirs->path, FALSE,
+                                 cancel_func, cancel_baton,
+                                 subpool));
+    }
+  for (; del_files; del_files = del_files->next)
+    {
+      svn_error_t *err;
+
+      if (cancel_func)
+        SVN_ERR((*cancel_func)(cancel_baton));
+
+      err = svn_io_remove_file(del_files->path, subpool);
+      if (err)
+        return svn_error_createf
+          (err->apr_err, err, _("Can't remove '%s'"),
+           svn_path_local_style(del_files->path, subpool));
+    }
+
+  svn_pool_destroy(subpool);
+
   status = apr_dir_remove(path_apr, pool);
   WIN32_RETRY_LOOP(status, apr_dir_remove(path_apr, pool));
   if (status)