[PATCH] Issue 3501: Repositories mounted on NFS don't work
From: Peter Samuelson <peter_at_p12n.org>
Date: Thu, 7 Jan 2010 00:55:00 -0600
[Peter Samuelson]
Well, it almost worked. Here's one, also for 1.6.x, that seems to pass
I'll commit the trunk version of this soon if no one objects. It
-- 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)Received on 2010-01-07 07:55:40 CET |
This is an archived mail posted to the Subversion Dev mailing list.
This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.