Re: svn commit: r1854072 - in /subversion/trunk/subversion: libsvn_subr/io.c tests/libsvn_subr/io-test.c

[Evgeny Kotkov]

Branko Čibej <br...@apache.org> writes:

> @@ -4493,6 +4571,12 @@ svn_io_dir_remove_nonrecursive(const cha
>
>    SVN_ERR(cstring_from_utf8(&dirname_apr, dirname, pool));
>
> +  /* On Windows, a read-only directory cannot be removed. */
> +#if defined(WIN32) || defined(__OS2__)
> +  SVN_ERR(io_set_readonly_flag(dirname_apr, dirname,
> +                               FALSE, FALSE, FALSE, pool));
> +#endif
> +
>    status = apr_dir_remove(dirname_apr, pool);

Would it be feasible for us to only attempt to remove the read-only
attribute after receiving an error? (along the lines of the attached patch)

The reason is that getting and setting file attributes usually results in
CreateFile() syscalls, whereas opening files and the I/O-related syscalls are
not cheap on Win32 in general.  So changing the directory attributes per every
remove could increase the amount of I/O operations and maybe slow down the
cases where we have to remove multiple directories, with the post-commit
transaction directory cleanup being an example of such case.


Thanks,
Evgeny Kotkov

Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1854090)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -4571,12 +4571,6 @@ svn_io_dir_remove_nonrecursive(const char *dirname
 
   SVN_ERR(cstring_from_utf8(&dirname_apr, dirname, pool));
 
-  /* On Windows, a read-only directory cannot be removed. */
-#if defined(WIN32) || defined(__OS2__)
-  SVN_ERR(io_set_readonly_flag(dirname_apr, dirname,
-                               FALSE, FALSE, FALSE, pool));
-#endif
-
   status = apr_dir_remove(dirname_apr, pool);
 
 #ifdef WIN32
@@ -4583,6 +4577,16 @@ svn_io_dir_remove_nonrecursive(const char *dirname
   {
     svn_boolean_t retry = TRUE;
 
+    if (APR_STATUS_IS_EACCES(status) || APR_STATUS_IS_EEXIST(status))
+      {
+        /* Set the destination directory writable because Windows will not
+           allow us to delete when path is read-only. */
+        SVN_ERR(io_set_readonly_flag(dirname_apr, dirname,
+                                     FALSE, FALSE, TRUE, pool));
+
+        status = apr_dir_remove(dirname_apr, pool);
+      }
+
     if (status == APR_FROM_OS_ERROR(ERROR_DIR_NOT_EMPTY))
     {
         apr_status_t empty_status = dir_is_empty(dirname_apr, pool);