subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From phi...@apache.org
Subject svn commit: r1081892 - in /subversion/trunk/subversion: include/private/svn_io_private.h libsvn_subr/io.c libsvn_wc/adm_ops.c libsvn_wc/questions.c libsvn_wc/wc.h tests/cmdline/revert_tests.py
Date Tue, 15 Mar 2011 18:33:53 GMT
Author: philip
Date: Tue Mar 15 18:33:52 2011
New Revision: 1081892

URL: http://svn.apache.org/viewvc?rev=1081892&view=rev
Log:
Make revert change file permissions on the basis of the current magic
properties and the current permissions, rather than on any change in
the magic permissions.  This means file permissions follow the same
rules as file content, and that permissions-only reverts occur.

* subversion/include/private/svn_private_io.h: New.

* subversion/libsvn_subr/io.c
  (): Include svn_private_io.h.
  (svn_io__is_finfo_read_only, svn_io__is_executable): New.
  (svn_io_is_file_executable): Call svn_io__is_executable.

* subversion/libsvn_wc/wc.h
  (svn_wc__internal_file_modified_p): New.

* subversion/libsvn_wc/questions.c
  (): Include svn_private_io.h.
  (svn_wc__internal_file_modified_p): New.
  (svn_wc__internal_text_modified_p): Call svn_wc__internal_file_modified_p.

* subversion/libsvn_wc/adm_ops.c
  (new_revert_internal): Use svn_wc__internal_file_modified_p to
   determine whether to reset contents or permissions.

* subversion/tests/cmdline/revert_tests.py
  (revert_permissions_only): New, marked XFail but passes with new revert.
  (test_list): Add revert_permissions_only.

Added:
    subversion/trunk/subversion/include/private/svn_io_private.h
Modified:
    subversion/trunk/subversion/libsvn_subr/io.c
    subversion/trunk/subversion/libsvn_wc/adm_ops.c
    subversion/trunk/subversion/libsvn_wc/questions.c
    subversion/trunk/subversion/libsvn_wc/wc.h
    subversion/trunk/subversion/tests/cmdline/revert_tests.py

Added: subversion/trunk/subversion/include/private/svn_io_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_io_private.h?rev=1081892&view=auto
==============================================================================
--- subversion/trunk/subversion/include/private/svn_io_private.h (added)
+++ subversion/trunk/subversion/include/private/svn_io_private.h Tue Mar 15 18:33:52 2011
@@ -0,0 +1,59 @@
+/**
+ * @copyright
+ * ====================================================================
+ *    Licensed to the Apache Software Foundation (ASF) under one
+ *    or more contributor license agreements.  See the NOTICE file
+ *    distributed with this work for additional information
+ *    regarding copyright ownership.  The ASF licenses this file
+ *    to you under the Apache License, Version 2.0 (the
+ *    "License"); you may not use this file except in compliance
+ *    with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing,
+ *    software distributed under the License is distributed on an
+ *    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *    KIND, either express or implied.  See the License for the
+ *    specific language governing permissions and limitations
+ *    under the License.
+ * ====================================================================
+ * @endcopyright
+ *
+ * @file svn_io_private.h
+ * @brief Private IO API
+ */
+
+#ifndef SVN_IO_PRIVATE_H
+#define SVN_IO_PRIVATE_H
+
+#include <apr.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+
+/** Set @a *executable TRUE if @a file_info is executable for the
+ * user, FALSE otherwise.
+ */
+svn_error_t *
+svn_io__is_finfo_executable(svn_boolean_t *executable,
+                            apr_finfo_t *file_info,
+                            apr_pool_t *pool);
+
+/** Set @a *read_only TRUE if @a file_info is read-only for the user,
+ * FALSE otherwise.
+ */
+svn_error_t *
+svn_io__is_finfo_read_only(svn_boolean_t *read_only,
+                           apr_finfo_t *file_info,
+                           apr_pool_t *pool);
+
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+
+#endif /* SVN_IO_PRIVATE_H */

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1081892&r1=1081891&r2=1081892&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Tue Mar 15 18:33:52 2011
@@ -64,6 +64,7 @@
 #include "svn_ctype.h"
 
 #include "private/svn_atomic.h"
+#include "private/svn_io_private.h"
 
 #define SVN_SLEEP_ENV_VAR "SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_SLEEP_FOR_TIMESTAMPS"
 
@@ -1761,36 +1762,84 @@ svn_io_set_file_executable(const char *p
 
 
 svn_error_t *
-svn_io_is_file_executable(svn_boolean_t *executable,
-                          const char *path,
-                          apr_pool_t *pool)
+svn_io__is_finfo_read_only(svn_boolean_t *read_only,
+                           apr_finfo_t *file_info,
+                           apr_pool_t *pool)
+{
+#if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
+  apr_status_t apr_err;
+  apr_uid_t uid;
+  apr_gid_t gid;
+
+  *read_only = FALSE;
+
+  apr_err = apr_uid_current(&uid, &gid, pool);
+
+  if (apr_err)
+    return svn_error_wrap_apr(apr_err, _("Error getting UID of process"));
+
+  /* Check write bit for current user. */
+  if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
+    *read_only = !(file_info->protection & APR_UWRITE);
+
+  else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
+    *read_only = !(file_info->protection & APR_GWRITE);
+  
+  else
+    *read_only = !(file_info->protection & APR_WWRITE);
+
+#else  /* WIN32 || __OS2__ || !APR_HAS_USER */
+  *read_only = !(file_info->protection & APR_UWRITE);
+#endif
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_io__is_finfo_executable(svn_boolean_t *executable,
+                            apr_finfo_t *file_info,
+                            apr_pool_t *pool)
 {
 #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
-  apr_finfo_t file_info;
   apr_status_t apr_err;
   apr_uid_t uid;
   apr_gid_t gid;
 
   *executable = FALSE;
 
-  /* Get file and user info. */
-  SVN_ERR(svn_io_stat(&file_info, path,
-                      (APR_FINFO_PROT | APR_FINFO_OWNER),
-                      pool));
   apr_err = apr_uid_current(&uid, &gid, pool);
 
   if (apr_err)
     return svn_error_wrap_apr(apr_err, _("Error getting UID of process"));
 
   /* Check executable bit for current user. */
-  if (apr_uid_compare(uid, file_info.user) == APR_SUCCESS)
-    *executable = (file_info.protection & APR_UEXECUTE);
+  if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
+    *executable = (file_info->protection & APR_UEXECUTE);
 
-  else if (apr_gid_compare(gid, file_info.group) == APR_SUCCESS)
-    *executable = (file_info.protection & APR_GEXECUTE);
+  else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
+    *executable = (file_info->protection & APR_GEXECUTE);
 
   else
-    *executable = (file_info.protection & APR_WEXECUTE);
+    *executable = (file_info->protection & APR_WEXECUTE);
+
+#else  /* WIN32 || __OS2__ || !APR_HAS_USER */
+  *executable = FALSE;
+#endif
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_io_is_file_executable(svn_boolean_t *executable,
+                          const char *path,
+                          apr_pool_t *pool)
+{
+#if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
+  apr_finfo_t file_info;
+
+  SVN_ERR(svn_io_stat(&file_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
+                      pool));
+  SVN_ERR(svn_io__is_finfo_executable(executable, &file_info, pool));
 
 #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
   *executable = FALSE;

Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1081892&r1=1081891&r2=1081892&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar 15 18:33:52 2011
@@ -1398,9 +1398,11 @@ new_revert_internal(svn_wc__db_t *db,
         }
       else if (on_disk == svn_node_file)
         {
-          svn_boolean_t modified;
+          svn_boolean_t modified, executable, read_only;
 
-          SVN_ERR(svn_wc__internal_text_modified_p(&modified, db, local_abspath,
+          SVN_ERR(svn_wc__internal_file_modified_p(&modified, &executable,
+                                                   &read_only,
+                                                   db, local_abspath,
                                                    FALSE, FALSE, scratch_pool));
           if (modified)
             {
@@ -1409,27 +1411,38 @@ new_revert_internal(svn_wc__db_t *db,
             }
           else
             {
-              /* ### Need to reset read-only, executable.  Old revert
-                     used the change in the magic properties to do
-                     this, but we don't have the old values.
-
-                 ### Perhaps use temporary SQLite table to store old
-                     properies, conflicts and local_relpaths?  Might
-                     also help with notification for recursive case.
-
-                 ### Need to set notification if changing
-                     attributes. */
               apr_hash_t *props;
 
               SVN_ERR(svn_wc__db_read_pristine_props(&props, db, local_abspath,
                                                      scratch_pool,
                                                      scratch_pool));
-              if (apr_hash_get(props, SVN_PROP_NEEDS_LOCK, APR_HASH_KEY_STRING))
-                SVN_ERR(svn_io_set_file_read_only(local_abspath, FALSE,
-                                                  scratch_pool));
-              if (apr_hash_get(props, SVN_PROP_EXECUTABLE, APR_HASH_KEY_STRING))
-                SVN_ERR(svn_io_set_file_executable(local_abspath, TRUE, FALSE,
-                                                   scratch_pool));
+              if (apr_hash_get(props, SVN_PROP_NEEDS_LOCK, APR_HASH_KEY_STRING)
+                  && !read_only)
+                {
+                  SVN_ERR(svn_io_set_file_read_only(local_abspath,
+                                                    FALSE, scratch_pool));
+                  notify_required = TRUE;
+                }
+              else if (read_only)
+                {
+                  SVN_ERR(svn_io_set_file_read_write(local_abspath,
+                                                     FALSE, scratch_pool));
+                  notify_required = TRUE;
+                }
+
+              if (apr_hash_get(props, SVN_PROP_EXECUTABLE, APR_HASH_KEY_STRING)
+                  && !executable)
+                {
+                  SVN_ERR(svn_io_set_file_executable(local_abspath, TRUE,
+                                                     FALSE, scratch_pool));
+                  notify_required = TRUE;
+                }
+              else if (executable)
+                {
+                  SVN_ERR(svn_io_set_file_executable(local_abspath, FALSE,
+                                                     FALSE, scratch_pool));
+                  notify_required = TRUE;
+                }
             }
         }
     }

Modified: subversion/trunk/subversion/libsvn_wc/questions.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/questions.c?rev=1081892&r1=1081891&r2=1081892&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/questions.c (original)
+++ subversion/trunk/subversion/libsvn_wc/questions.c Tue Mar 15 18:33:52 2011
@@ -47,6 +47,7 @@
 
 #include "svn_private_config.h"
 #include "private/svn_wc_private.h"
+#include "private/svn_io_private.h"
 
 
 
@@ -249,14 +250,36 @@ svn_wc__internal_text_modified_p(svn_boo
                                  svn_boolean_t compare_textbases,
                                  apr_pool_t *scratch_pool)
 {
+  SVN_ERR(svn_wc__internal_file_modified_p(modified_p, NULL, NULL,
+                                           db, local_abspath,
+                                           force_comparison, compare_textbases,
+                                           scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_wc__internal_file_modified_p(svn_boolean_t *modified_p,
+                                 svn_boolean_t *executable_p,
+                                 svn_boolean_t *read_only_p,
+                                 svn_wc__db_t *db,
+                                 const char *local_abspath,
+                                 svn_boolean_t force_comparison,
+                                 svn_boolean_t compare_textbases,
+                                 apr_pool_t *scratch_pool)
+{
   svn_stream_t *pristine_stream;
   svn_error_t *err;
   apr_finfo_t finfo;
+  apr_int32_t wanted
+    = APR_FINFO_SIZE | APR_FINFO_MTIME | APR_FINFO_TYPE | APR_FINFO_LINK;
+
+  if (executable_p)
+    wanted |= APR_FINFO_PROT | APR_FINFO_OWNER;
+  if (read_only_p)
+    wanted |= APR_FINFO_PROT;
 
   /* No matter which way you look at it, the file needs to exist. */
-  err = svn_io_stat(&finfo, local_abspath,
-                    APR_FINFO_SIZE | APR_FINFO_MTIME | APR_FINFO_TYPE
-                    | APR_FINFO_LINK, scratch_pool);
+  err = svn_io_stat(&finfo, local_abspath, wanted, scratch_pool);
   if ((err && APR_STATUS_IS_ENOENT(err->apr_err))
       || (!err && !(finfo.filetype == APR_REG ||
                     finfo.filetype == APR_LNK)))
@@ -270,6 +293,11 @@ svn_wc__internal_text_modified_p(svn_boo
   else if (err)
     return err;
 
+  if (executable_p)
+    SVN_ERR(svn_io__is_finfo_executable(executable_p, &finfo, scratch_pool));
+  if (read_only_p)
+    SVN_ERR(svn_io__is_finfo_read_only(read_only_p, &finfo, scratch_pool));
+
   if (! force_comparison)
     {
       svn_filesize_t translated_size;

Modified: subversion/trunk/subversion/libsvn_wc/wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc.h?rev=1081892&r1=1081891&r2=1081892&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc.h Tue Mar 15 18:33:52 2011
@@ -395,6 +395,22 @@ svn_wc__internal_text_modified_p(svn_boo
                                  apr_pool_t *scratch_pool);
 
 
+/* Like svn_wc__internal_text_modified_p but also sets *EXECUTABLE_P
+ * and *READ_ONLY_P based on the files current permissions.
+ *
+ * EXECUTABLE_P and READ_ONLY_P can be NULL.
+ */
+svn_error_t *
+svn_wc__internal_file_modified_p(svn_boolean_t *modified_p,
+                                 svn_boolean_t *executable_p,
+                                 svn_boolean_t *read_only_p,
+                                 svn_wc__db_t *db,
+                                 const char *local_abspath,
+                                 svn_boolean_t force_comparison,
+                                 svn_boolean_t compare_textbases,
+                                 apr_pool_t *scratch_pool);
+
+
 
 /* Merge the difference between LEFT_ABSPATH and RIGHT_ABSPATH into
    TARGET_ABSPATH, return the appropriate work queue operations in

Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1081892&r1=1081891&r2=1081892&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Tue Mar 15 18:33:52 2011
@@ -25,7 +25,7 @@
 ######################################################################
 
 # General modules
-import re, os
+import re, os, stat
 
 # Our testing module
 import svntest
@@ -1087,6 +1087,97 @@ def revert_non_recusive_after_delete(sbo
   expected_status.tweak('A/B/E', status='  ')
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
 
+@XFail()
+def revert_permissions_only(sbox):
+  "permission-only reverts"
+
+  sbox.build()
+  wc_dir = sbox.wc_dir
+
+  # Helpers pinched/adapted from lock_tests.py.  Put them somewhere common?
+  def check_writability(path, writable):
+    bits = stat.S_IWGRP | stat.S_IWOTH | stat.S_IWRITE
+    mode = os.stat(path)[0]
+    if bool(mode & bits) != writable:
+      raise svntest.Failure("path '%s' is unexpectedly %s (mode %o)"
+                            % (path, ["writable", "read-only"][writable], mode))
+
+  def is_writable(path):
+    "Raise if PATH is not writable."
+    check_writability(path, True)
+
+  def is_readonly(path):
+    "Raise if PATH is not readonly."
+    check_writability(path, False)
+
+  def check_executability(path, executable):
+    bits = stat.S_IXGRP | stat.S_IXOTH | stat.S_IEXEC
+    mode = os.stat(path)[0]
+    if bool(mode & bits) != executable:
+      raise svntest.Failure("path '%s' is unexpectedly %s (mode %o)"
+                            % (path,
+                               ["executable", "non-executable"][executable],
+                               mode))
+
+  def is_executable(path):
+    "Raise if PATH is not executable."
+    check_executability(path, True)
+
+  def is_non_executable(path):
+    "Raise if PATH is executable."
+    check_executability(path, False)
+
+
+  os.chmod(sbox.ospath('A/B/E/alpha'), 0444);  # read-only
+  is_readonly(sbox.ospath('A/B/E/alpha'))
+  expected_output = ["Reverted '%s'\n" % sbox.ospath('A/B/E/alpha')]
+  svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                     'revert', sbox.ospath('A/B/E/alpha'))
+  is_writable(sbox.ospath('A/B/E/alpha'))
+
+  if svntest.main.is_posix_os:
+    os.chmod(sbox.ospath('A/B/E/beta'), 0777);   # executable
+    is_executable(sbox.ospath('A/B/E/beta'))
+    expected_output = ["Reverted '%s'\n" % sbox.ospath('A/B/E/beta')]
+    svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                       'revert', sbox.ospath('A/B/E/beta'))
+    is_non_executable(sbox.ospath('A/B/E/beta'))
+
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'propset', 'svn:needs-lock', '1',
+                                     sbox.ospath('A/B/E/alpha'))
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'propset', 'svn:executable', '1',
+                                     sbox.ospath('A/B/E/beta'))
+
+  expected_output = svntest.wc.State(wc_dir, {
+    'A/B/E/alpha': Item(verb='Sending'),
+    'A/B/E/beta':  Item(verb='Sending'),
+    })
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+  expected_status.tweak('A/B/E/alpha', wc_rev='2')
+  expected_status.tweak('A/B/E/beta',  wc_rev='2')
+  svntest.actions.run_and_verify_commit(wc_dir,
+                                        expected_output,
+                                        expected_status,
+                                        None, wc_dir)
+
+  os.chmod(sbox.ospath('A/B/E/alpha'), 0666);  # not read-only
+  is_writable(sbox.ospath('A/B/E/alpha'))
+  expected_output = ["Reverted '%s'\n" % sbox.ospath('A/B/E/alpha')]
+  svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                     'revert', sbox.ospath('A/B/E/alpha'))
+  is_readonly(sbox.ospath('A/B/E/alpha'))
+
+  if svntest.main.is_posix_os:
+    os.chmod(sbox.ospath('A/B/E/beta'), 0666);   # not executable
+    is_non_executable(sbox.ospath('A/B/E/beta'))
+    expected_output = ["Reverted '%s'\n" % sbox.ospath('A/B/E/beta')]
+    svntest.actions.run_and_verify_svn(None, expected_output, [],
+                                       'revert', sbox.ospath('A/B/E/beta'))
+    is_executable(sbox.ospath('A/B/E/beta'))
+
+
 ########################################################################
 # Run the tests
 
@@ -1116,6 +1207,7 @@ test_list = [ None,
               revert_added_tree,
               revert_child_of_copy,
               revert_non_recusive_after_delete,
+              revert_permissions_only,
              ]
 
 if __name__ == '__main__':



Mime
View raw message