Re: Issue 3501: Repositories mounted on NFS don't work

[Edgar Fuß]
> To summarise the problem: recursive directory deletion fails because entries
> re-appear after dir rewinding. The fix may be as easy as ignoring ENOENT.

Or not calling rewind. Something like the following (untested) against
1.6.x. Same patch applies to trunk except for some conflicts involving
path vs. dirent functions.

-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
[[[
* subversion/libsvn_subr/io.c
  (svn_io_remove_dir2): Remove rewinddir() magic (see Issues 1896 and
   3501).  Instead, build linked lists of files and subdirs to delete,
   then delete them in separate loops.
]]]
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c	(revisione 887406)
+++ subversion/libsvn_subr/io.c	(copia locale)
@@ -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;
+      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')))))
+	{
+	  /* skip "." and ".." */
+	  continue;
+	}
+      SVN_ERR(entry_name_to_utf8(&entry_utf8, this_entry.name,
+				 path_apr, subpool));
 
-      for (status = apr_dir_read(&this_entry, flags, this_dir);
-           status == APR_SUCCESS;
-           status = apr_dir_read(&this_entry, flags, this_dir))
-        {
-          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;
+      del_tmp = apr_palloc(subpool, sizeof *del_tmp);
+      del_tmp->path = svn_path_join(path, entry_utf8, subpool);
 
-#ifndef WIN32
-              need_rewind = TRUE;
-#endif
-
-              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 (need_rewind)
-        {
-          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));
-        }
+      if (this_entry.filetype == APR_DIR)
+	{
+	  del_tmp->next = del_dirs;
+	  del_dirs = del_tmp;
+	}
+      else
+	{
+	  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,31 @@
     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_file2(del_files->path, FALSE, 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)