> -----Original Message-----
> From: Peter Samuelson [mailto:pe...@p12n.org]
> Sent: donderdag 7 januari 2010 2:30
> To: Edgar Fuß
> Cc: dev@subversion.apache.org
> Subject: 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));
> - }
I think it is easier to use svn_io_get_dirents2() then this block
replicating its implementation.
/** Read all of the disk entries in directory @a path, a utf8-encoded
* path. Set @a *dirents to a hash mapping dirent names (<tt>char *</tt>)
to
* #svn_io_dirent_t structures, allocated in @a pool.
*
* @note The `.' and `..' directories normally returned by
* apr_dir_read() are NOT returned in the hash.
*
* @note The kind field in the @a dirents is set according to the mapping
* as documented for svn_io_check_path()
*
* @since New in 1.3.
*/
svn_error_t *
svn_io_get_dirents2(apr_hash_t **dirents,
const char *path,
apr_pool_t *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)
