subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From julianf...@apache.org
Subject svn commit: r1863299 - in /subversion/branches/1.10.x: ./ STATUS subversion/libsvn_subr/io.c subversion/tests/libsvn_subr/io-test.c
Date Thu, 18 Jul 2019 13:38:19 GMT
Author: julianfoad
Date: Thu Jul 18 13:38:19 2019
New Revision: 1863299

URL: http://svn.apache.org/viewvc?rev=1863299&view=rev
Log:
Merge the r1854072 group from trunk:

 * r1854072, r1854074, r1854216
   Fix issue #4806: Remove on-disk trees with read-only directories in them.
   Justification:
     Fixes an edge case in our tree removal code. If we clear read-only
     permissions on files in order to remove them, we should do the same for
     directories.
   Votes:
     +1: brane, stsp

Modified:
    subversion/branches/1.10.x/   (props changed)
    subversion/branches/1.10.x/STATUS
    subversion/branches/1.10.x/subversion/libsvn_subr/io.c
    subversion/branches/1.10.x/subversion/tests/libsvn_subr/io-test.c

Propchange: subversion/branches/1.10.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Jul 18 13:38:19 2019
@@ -103,4 +103,4 @@
 /subversion/branches/verify-at-commit:1462039-1462408
 /subversion/branches/verify-keep-going:1439280-1546110
 /subversion/branches/wc-collate-path:1402685-1480384
-/subversion/trunk:1817837,1817856,1818577-1818578,1818584,1818651,1818662,1818727,1818801,1818803,1818807,1818868,1818871,1819036-1819037,1819043,1819049,1819052,1819093,1819146,1819162,1819444,1819556-1819557,1819603,1819804,1819911,1820044,1820046-1820047,1820518,1820627,1820718,1820778,1821183,1821224,1821621,1821678,1822401,1822587,1822591,1822996,1823202-1823203,1823211,1823327,1823791,1823966,1823989,1824033,1825024,1825045,1825215,1825266,1825306,1825709,1825711,1825721,1825736,1825778,1825783,1825787-1825788,1825979,1826720-1826721,1826747,1826811,1826814,1826877,1826907,1826971,1827105,1827114,1827191,1827562,1827574,1827670,1828613,1829012,1829015,1829241,1829260,1829344,1830083,1830882-1830883,1830885,1830900-1830901,1831110,1831112,1831540,1833465,1833621,1833836,1833842,1833864,1833866,1833895,1833897,1833899,1833901,1835760,1836306,1836762,1836802,1836960,1836963,1836968,1836976,1837037,1837790,1838813,1839662,1839703,1839734,1839739,1840991,1842260,1842262,1842264,184
 3888,1844882,1844987,1845204,1845212,1845261,1845408,1845555,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621,1850651,1851676,1851687,1851791,1851920,1852013,1852436,1853450,1853483,1853761,1863262
+/subversion/trunk:1817837,1817856,1818577-1818578,1818584,1818651,1818662,1818727,1818801,1818803,1818807,1818868,1818871,1819036-1819037,1819043,1819049,1819052,1819093,1819146,1819162,1819444,1819556-1819557,1819603,1819804,1819911,1820044,1820046-1820047,1820518,1820627,1820718,1820778,1821183,1821224,1821621,1821678,1822401,1822587,1822591,1822996,1823202-1823203,1823211,1823327,1823791,1823966,1823989,1824033,1825024,1825045,1825215,1825266,1825306,1825709,1825711,1825721,1825736,1825778,1825783,1825787-1825788,1825979,1826720-1826721,1826747,1826811,1826814,1826877,1826907,1826971,1827105,1827114,1827191,1827562,1827574,1827670,1828613,1829012,1829015,1829241,1829260,1829344,1830083,1830882-1830883,1830885,1830900-1830901,1831110,1831112,1831540,1833465,1833621,1833836,1833842,1833864,1833866,1833895,1833897,1833899,1833901,1835760,1836306,1836762,1836802,1836960,1836963,1836968,1836976,1837037,1837790,1838813,1839662,1839703,1839734,1839739,1840991,1842260,1842262,1842264,184
 3888,1844882,1844987,1845204,1845212,1845261,1845408,1845555,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621,1850651,1851676,1851687,1851791,1851920,1852013,1852436,1853450,1853483,1853761,1854072,1854074,1854216,1863262

Modified: subversion/branches/1.10.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/STATUS?rev=1863299&r1=1863298&r2=1863299&view=diff
==============================================================================
--- subversion/branches/1.10.x/STATUS (original)
+++ subversion/branches/1.10.x/STATUS Thu Jul 18 13:38:19 2019
@@ -21,15 +21,6 @@ Veto-blocked changes:
 Approved changes:
 =================
 
- * r1854072, r1854074, r1854216
-   Fix issue #4806: Remove on-disk trees with read-only directories in them.
-   Justification:
-     Fixes an edge case in our tree removal code. If we clear read-only
-     permissions on files in order to remove them, we should do the same for
-     directories.
-   Votes:
-     +1: brane, stsp
-
  * r1855419
    Fix conflict resolver bug where local and incoming edits got swapped.
    Justification:

Modified: subversion/branches/1.10.x/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/libsvn_subr/io.c?rev=1863299&r1=1863298&r2=1863299&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/libsvn_subr/io.c (original)
+++ subversion/branches/1.10.x/subversion/libsvn_subr/io.c Thu Jul 18 13:38:19 2019
@@ -1622,13 +1622,14 @@ merge_default_file_perms(apr_file_t *fd,
    that attempts to honor the users umask when dealing with
    permission changes.  It is a no-op when invoked on a symlink. */
 static svn_error_t *
-io_set_file_perms(const char *path,
-                  svn_boolean_t change_readwrite,
-                  svn_boolean_t enable_write,
-                  svn_boolean_t change_executable,
-                  svn_boolean_t executable,
-                  svn_boolean_t ignore_enoent,
-                  apr_pool_t *pool)
+io_set_perms(const char *path,
+             svn_boolean_t is_file,
+             svn_boolean_t change_readwrite,
+             svn_boolean_t enable_write,
+             svn_boolean_t change_executable,
+             svn_boolean_t executable,
+             svn_boolean_t ignore_enoent,
+             apr_pool_t *pool)
 {
   apr_status_t status;
   const char *path_apr;
@@ -1648,9 +1649,16 @@ io_set_file_perms(const char *path,
                             || SVN__APR_STATUS_IS_ENOTDIR(status)))
         return SVN_NO_ERROR;
       else if (status != APR_ENOTIMPL)
-        return svn_error_wrap_apr(status,
-                                  _("Can't change perms of file '%s'"),
-                                  svn_dirent_local_style(path, pool));
+        {
+          if (is_file)
+            return svn_error_wrap_apr(status,
+                                      _("Can't change perms of file '%s'"),
+                                      svn_dirent_local_style(path, pool));
+          else
+            return svn_error_wrap_apr(status,
+                                      _("Can't change perms of directory '%s'"),
+                                      svn_dirent_local_style(path, pool));
+        }
       return SVN_NO_ERROR;
     }
 
@@ -1750,10 +1758,50 @@ io_set_file_perms(const char *path,
       status = apr_file_attrs_set(path_apr, attrs, attrs_values, pool);
     }
 
-  return svn_error_wrap_apr(status,
-                            _("Can't change perms of file '%s'"),
-                            svn_dirent_local_style(path, pool));
+  if (is_file)
+    {
+      return svn_error_wrap_apr(status,
+                                _("Can't change perms of file '%s'"),
+                                svn_dirent_local_style(path, pool));
+    }
+  else
+    {
+      return svn_error_wrap_apr(status,
+                                _("Can't change perms of directory '%s'"),
+                                svn_dirent_local_style(path, pool));
+    }
+}
+
+static svn_error_t *
+io_set_file_perms(const char *path,
+                  svn_boolean_t change_readwrite,
+                  svn_boolean_t enable_write,
+                  svn_boolean_t change_executable,
+                  svn_boolean_t executable,
+                  svn_boolean_t ignore_enoent,
+                  apr_pool_t *pool)
+{
+  return svn_error_trace(io_set_perms(path, TRUE,
+                                      change_readwrite, enable_write,
+                                      change_executable, executable,
+                                      ignore_enoent, pool));
+}
+
+static svn_error_t *
+io_set_dir_perms(const char *path,
+                 svn_boolean_t change_readwrite,
+                 svn_boolean_t enable_write,
+                 svn_boolean_t change_executable,
+                 svn_boolean_t executable,
+                 svn_boolean_t ignore_enoent,
+                 apr_pool_t *pool)
+{
+  return svn_error_trace(io_set_perms(path, FALSE,
+                                      change_readwrite, enable_write,
+                                      change_executable, executable,
+                                      ignore_enoent, pool));
 }
+
 #endif /* !WIN32 && !__OS2__ */
 
 #ifdef WIN32
@@ -2115,6 +2163,55 @@ svn_io_set_file_read_write_carefully(con
   return svn_io_set_file_read_only(path, ignore_enoent, pool);
 }
 
+#if defined(WIN32) || defined(__OS2__)
+/* Helper for svn_io_set_file_read_* */
+static svn_error_t *
+io_set_readonly_flag(const char *path_apr, /* file-system path */
+                     const char *path,     /* UTF-8 path */
+                     svn_boolean_t set_flag,
+                     svn_boolean_t is_file,
+                     svn_boolean_t ignore_enoent,
+                     apr_pool_t *pool)
+{
+  apr_status_t status;
+
+  status = apr_file_attrs_set(path_apr,
+                              (set_flag ? APR_FILE_ATTR_READONLY : 0),
+                              APR_FILE_ATTR_READONLY,
+                              pool);
+
+  if (status && status != APR_ENOTIMPL)
+    if (!(ignore_enoent && (APR_STATUS_IS_ENOENT(status)
+                            || SVN__APR_STATUS_IS_ENOTDIR(status))))
+      {
+        if (is_file)
+          {
+            if (set_flag)
+              return svn_error_wrap_apr(status,
+                                        _("Can't set file '%s' read-only"),
+                                        svn_dirent_local_style(path, pool));
+            else
+              return svn_error_wrap_apr(status,
+                                        _("Can't set file '%s' read-write"),
+                                        svn_dirent_local_style(path, pool));
+          }
+        else
+          {
+            if (set_flag)
+              return svn_error_wrap_apr(status,
+                                        _("Can't set directory '%s' read-only"),
+                                        svn_dirent_local_style(path, pool));
+            else
+              return svn_error_wrap_apr(status,
+                                        _("Can't set directory '%s' read-write"),
+                                        svn_dirent_local_style(path, pool));
+          }
+      }
+  return SVN_NO_ERROR;
+}
+#endif
+
+
 svn_error_t *
 svn_io_set_file_read_only(const char *path,
                           svn_boolean_t ignore_enoent,
@@ -2126,24 +2223,11 @@ svn_io_set_file_read_only(const char *pa
   return io_set_file_perms(path, TRUE, FALSE, FALSE, FALSE,
                            ignore_enoent, pool);
 #else
-  apr_status_t status;
   const char *path_apr;
 
   SVN_ERR(cstring_from_utf8(&path_apr, path, pool));
-
-  status = apr_file_attrs_set(path_apr,
-                              APR_FILE_ATTR_READONLY,
-                              APR_FILE_ATTR_READONLY,
-                              pool);
-
-  if (status && status != APR_ENOTIMPL)
-    if (!(ignore_enoent && (APR_STATUS_IS_ENOENT(status)
-                            || SVN__APR_STATUS_IS_ENOTDIR(status))))
-      return svn_error_wrap_apr(status,
-                                _("Can't set file '%s' read-only"),
-                                svn_dirent_local_style(path, pool));
-
-  return SVN_NO_ERROR;
+  return io_set_readonly_flag(path_apr, path,
+                              TRUE, TRUE, ignore_enoent, pool);
 #endif
 }
 
@@ -2159,23 +2243,11 @@ svn_io_set_file_read_write(const char *p
   return io_set_file_perms(path, TRUE, TRUE, FALSE, FALSE,
                            ignore_enoent, pool);
 #else
-  apr_status_t status;
   const char *path_apr;
 
   SVN_ERR(cstring_from_utf8(&path_apr, path, pool));
-
-  status = apr_file_attrs_set(path_apr,
-                              0,
-                              APR_FILE_ATTR_READONLY,
-                              pool);
-
-  if (status && status != APR_ENOTIMPL)
-    if (!ignore_enoent || !APR_STATUS_IS_ENOENT(status))
-      return svn_error_wrap_apr(status,
-                                _("Can't set file '%s' read-write"),
-                                svn_dirent_local_style(path, pool));
-
-  return SVN_NO_ERROR;
+  return io_set_readonly_flag(path_apr, path,
+                              FALSE, TRUE, ignore_enoent, pool);
 #endif
 }
 
@@ -2752,6 +2824,12 @@ svn_io_remove_dir2(const char *path, svn
       return svn_error_trace(err);
     }
 
+  /* On Unix, nothing can be removed from a non-writable directory. */
+#if !defined(WIN32) && !defined(__OS2__)
+  SVN_ERR(io_set_dir_perms(path, TRUE, TRUE, FALSE, FALSE,
+                           ignore_enoent, pool));
+#endif
+
   for (hi = apr_hash_first(subpool, dirents); hi; hi = apr_hash_next(hi))
     {
       const char *name = apr_hash_this_key(hi);
@@ -4490,8 +4568,17 @@ svn_io_dir_remove_nonrecursive(const cha
   {
     svn_boolean_t retry = TRUE;
 
+    if (APR_STATUS_IS_EACCES(status) || APR_STATUS_IS_EEXIST(status))
+      {
+        /* Make the destination directory writable because Windows
+           forbids deleting read-only items. */
+        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);
 
         if (APR_STATUS_IS_ENOTEMPTY(empty_status))

Modified: subversion/branches/1.10.x/subversion/tests/libsvn_subr/io-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/tests/libsvn_subr/io-test.c?rev=1863299&r1=1863298&r2=1863299&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/tests/libsvn_subr/io-test.c (original)
+++ subversion/branches/1.10.x/subversion/tests/libsvn_subr/io-test.c Thu Jul 18 13:38:19
2019
@@ -211,6 +211,38 @@ create_comparison_candidates(struct test
   return err;
 }
 
+/* Create an on-disk tree with optional read-only attributes on some
+   files and/or directories. */
+static svn_error_t *
+create_dir_tree(const char **dir_path,
+                const char *testname,
+                svn_boolean_t dir_readonly,
+                svn_boolean_t file_readonly,
+                apr_pool_t *pool)
+{
+  const char *test_dir_path;
+  const char *sub_dir_path;
+  const char *file_path;
+
+  SVN_ERR(svn_test_make_sandbox_dir(&test_dir_path, testname, pool));
+
+  sub_dir_path = svn_dirent_join(test_dir_path, "dir", pool);
+  SVN_ERR(svn_io_dir_make(sub_dir_path, APR_OS_DEFAULT, pool));
+
+  file_path = svn_dirent_join(sub_dir_path, "file", pool);
+  SVN_ERR(svn_io_file_create_empty(file_path, pool));
+
+  if (file_readonly)
+    SVN_ERR(svn_io_set_file_read_only(file_path, FALSE, pool));
+
+  if (dir_readonly)
+    SVN_ERR(svn_io_set_file_read_only(sub_dir_path, FALSE, pool));
+
+  *dir_path = sub_dir_path;
+  return SVN_NO_ERROR;
+}
+
+
 
 /* Functions to check the 2-way and 3-way file comparison functions.  */
 
@@ -1147,6 +1179,56 @@ test_apr_trunc_workaround(apr_pool_t *po
   return SVN_NO_ERROR;  
 }
 
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_all_writable(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_all_writable",
+                          FALSE, FALSE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_file_readonly(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_file_readonly",
+                          FALSE, TRUE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_dir_readonly(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_dir_readonly",
+                          TRUE, FALSE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_all_readonly(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_all_readonly",
+                          TRUE, TRUE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+
 /* The test table.  */
 
 static int max_threads = 3;
@@ -1184,6 +1266,14 @@ static struct svn_test_descriptor_t test
                    "test svn_io_open_uniquely_named()"),
     SVN_TEST_PASS2(test_apr_trunc_workaround,
                    "test workaround for APR in svn_io_file_trunc"),
+    SVN_TEST_PASS2(test_rmtree_all_writable,
+                   "test svn_io_remove_dir2() with writable tree"),
+    SVN_TEST_PASS2(test_rmtree_file_readonly,
+                   "test svn_io_remove_dir2() with read-only file"),
+    SVN_TEST_PASS2(test_rmtree_dir_readonly,
+                   "test svn_io_remove_dir2() with read-only directory"),
+    SVN_TEST_PASS2(test_rmtree_all_readonly,
+                   "test svn_io_remove_dir2() with read-only tree"),
     SVN_TEST_NULL
   };
 



Mime
View raw message