Author: dsahlberg
Date: Sat Jan 13 09:16:26 2024
New Revision: 1915215

URL: http://svn.apache.org/viewvc?rev=1915215&view=rev
Log:
Manage spurious Reverted message caused by non-W access to files
owned by another user. Part of Issue #4622.

The revert notification comes from the code trying to add W permissions
but since there is already W (for another user) the code doesn't change
anything and the notification will come back next time as well.

Changing to add a separate notification type "you don't have W access
and we can't do anything about it".

The text should be tweaked further.

Discussed on dev@: 
https://lists.apache.org/thread/p1ky889bxwy8okqly7h1lgckxfpldnxs

* subversion/include/svn_wc.h
  (svn_wc_notify_action_t): Add a new notification type
* subversion/libsvn_wc/revert.c
  (revert_wc_data): Add new parameter to indicate the need for notification
                    of "no access" and use that when a file is readonly but
                    (some other user) already has W.
  (revert_restore): Handle the "no access" case with the new notification type.
* subversion/svn/notify.c
  (notify_body): Handle the new notification type
* subversion/svnbench/notify.c
  (notify): Handle the new notification type


Modified:
    subversion/trunk/subversion/include/svn_wc.h
    subversion/trunk/subversion/libsvn_wc/revert.c
    subversion/trunk/subversion/svn/notify.c
    subversion/trunk/subversion/svnbench/notify.c

Modified: subversion/trunk/subversion/include/svn_wc.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1915215&r1=1915214&r2=1915215&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Sat Jan 13 09:16:26 2024
@@ -993,6 +993,7 @@ typedef enum svn_wc_notify_action_t
   svn_wc_notify_restore,
 
   /** Reverting a modified path. */
+  /* See also svn_wc_notify_revert_noaccess */
   svn_wc_notify_revert,
 
   /** A revert operation has failed. */
@@ -1325,6 +1326,12 @@ typedef enum svn_wc_notify_action_t
    * @since New in 1.15. */
   svn_wc_notify_warning,
 
+  /** A file is readonly for the user but isn't svn:needs-lock.
+   * So we want to restore RW, but fail since the file has W bits,
+   * just not for the current user. 
+   * @since New in 1.15. */
+  svn_wc_notify_revert_noaccess,
+
 } svn_wc_notify_action_t;
 
 

Modified: subversion/trunk/subversion/libsvn_wc/revert.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/revert.c?rev=1915215&r1=1915214&r2=1915215&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/revert.c (original)
+++ subversion/trunk/subversion/libsvn_wc/revert.c Sat Jan 13 09:16:26 2024
@@ -263,6 +263,7 @@ revert_restore_handle_copied_dirs(svn_bo
 static svn_error_t *
 revert_wc_data(svn_boolean_t *run_wq,
                svn_boolean_t *notify_required,
+               svn_boolean_t *notify_noaccess,
                svn_wc__db_t *db,
                const char *local_abspath,
                svn_wc__db_status_t status,
@@ -309,6 +310,7 @@ revert_restore(svn_boolean_t *run_wq,
   svn_wc__db_status_t status;
   svn_node_kind_t kind;
   svn_boolean_t notify_required;
+  svn_boolean_t notify_noaccess;
   const apr_array_header_t *conflict_files;
   svn_filesize_t recorded_size;
   apr_time_t recorded_time;
@@ -398,7 +400,7 @@ revert_restore(svn_boolean_t *run_wq,
   if (!metadata_only)
     {
       SVN_ERR(revert_wc_data(run_wq,
-                             &notify_required,
+                             &notify_required, &notify_noaccess,
                              db, local_abspath, status, kind,
                              reverted_kind, recorded_size, recorded_time,
                              copied_here, use_commit_times,
@@ -419,12 +421,19 @@ revert_restore(svn_boolean_t *run_wq,
         }
     }
 
-  if (notify_func && notify_required)
-    notify_func(notify_baton,
-                svn_wc_create_notify(local_abspath, svn_wc_notify_revert,
-                                     scratch_pool),
-                scratch_pool);
-
+  if (notify_func)
+    {
+      if (notify_required)
+        notify_func(notify_baton,
+                    svn_wc_create_notify(local_abspath, svn_wc_notify_revert,
+                                         scratch_pool),
+                    scratch_pool);
+      else if (notify_noaccess)
+        notify_func(notify_baton,
+                    svn_wc_create_notify(local_abspath, 
svn_wc_notify_revert_noaccess,
+                                         scratch_pool),
+                    scratch_pool);
+    }
   if (depth == svn_depth_infinity && kind == svn_node_dir)
     {
       apr_pool_t *iterpool = svn_pool_create(scratch_pool);
@@ -482,6 +491,7 @@ revert_restore(svn_boolean_t *run_wq,
 static svn_error_t *
 revert_wc_data(svn_boolean_t *run_wq,
                svn_boolean_t *notify_required,
+               svn_boolean_t *notify_noaccess,
                svn_wc__db_t *db,
                const char *local_abspath,
                svn_wc__db_status_t status,
@@ -661,11 +671,23 @@ revert_wc_data(svn_boolean_t *run_wq,
                         }
                       else if (!needs_lock_prop && read_only)
                         {
-                          SVN_ERR(svn_io_set_file_read_write(local_abspath,
-                                                             FALSE,
-                                                             scratch_pool));
-                          *notify_required = TRUE;
-                        }
+                          /* If there is already W on the file, it is owned by
+                          * some other user. Then svn_io_set_file_read_write
+                          * will return without making any changes and the 
+                          * user will get a spurious "Reverted" message.
+                          * Only checking for user's W since that is the only
+                          * one set by svn_io_set_file_read_write()
+                          * Issue #4622 */
+                          if (finfo.protection |  APR_UWRITE)
+                            *notify_noaccess = TRUE;
+                          else
+                            {
+                              SVN_ERR(svn_io_set_file_read_write(local_abspath,
+                                                                 FALSE,
+                                                                 
scratch_pool));
+                              *notify_required = TRUE;
+                            }
+                       }
                     }
 
 #if !defined(WIN32) && !defined(__OS2__)

Modified: subversion/trunk/subversion/svn/notify.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/notify.c?rev=1915215&r1=1915214&r2=1915215&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/notify.c (original)
+++ subversion/trunk/subversion/svn/notify.c Sat Jan 13 09:16:26 2024
@@ -450,6 +450,11 @@ notify_body(struct notify_baton *nb,
                                  path_local));
       break;
 
+    case svn_wc_notify_revert_noaccess:
+      SVN_ERR(svn_cmdline_printf(pool, _("User doesn't have WRITE permissions 
to file '%s' and the file isn't svn:needslock. But the file is already 
writeable. Probably owned by another user."),
+                             path_local));
+      break;
+
     case svn_wc_notify_failed_revert:
       SVN_ERR(svn_cmdline_printf(pool, _("Failed to revert '%s' -- "
                                          "try updating instead.\n"),

Modified: subversion/trunk/subversion/svnbench/notify.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svnbench/notify.c?rev=1915215&r1=1915214&r2=1915215&view=diff
==============================================================================
--- subversion/trunk/subversion/svnbench/notify.c (original)
+++ subversion/trunk/subversion/svnbench/notify.c Sat Jan 13 09:16:26 2024
@@ -241,6 +241,12 @@ notify(void *baton, const svn_wc_notify_
         goto print_error;
       break;
 
+    case svn_wc_notify_revert_noaccess:
+      if ((err = svn_cmdline_printf(pool, _("User doesn't have WRITE 
permissions to file '%s' and the file isn't svn:needslock. But the file is 
already writeable. Probably owned by another user."),
+                                    path_local)))
+        goto print_error;
+      break;
+
     case svn_wc_notify_failed_revert:
       if (( err = svn_cmdline_printf(pool, _("Failed to revert '%s' -- "
                                              "try updating instead.\n"),


Reply via email to