Re: svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c

2024-01-13 Thread Daniel Sahlberg
Den lör 13 jan. 2024 kl 10:16 skrev :

> Author: dsahlberg
> Date: Sat Jan 13 09:16:26 2024
> New Revision: 1915215
>
> URL: http://svn.apache.org/viewvc?rev=1915215=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.
>

...

Modified: subversion/trunk/subversion/svn/notify.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/notify.c?rev=1915215=1915214=1915215=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=1915214=1915215=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"),


I would REALLY like someone to suggest a better wording for these messages.
I couldn't come up with something simple that both explains what happend
and suggests a reason.

Kind regards,
Daniel


svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c

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

URL: http://svn.apache.org/viewvc?rev=1915215=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=1915214=1915215=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=1915214=1915215=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,
- _required,
+ _required, _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