Re: svn commit: r1915214 - /subversion/trunk/subversion/libsvn_subr/io.c

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

> Author: dsahlberg
> Date: Sat Jan 13 08:56:50 2024
> New Revision: 1915214
>
> URL: http://svn.apache.org/viewvc?rev=1915214=rev
> Log:
> Replace the homegrown checks for readonly/executable with calls to
> access(2)
> to consider, for example, user's secondary groups.
>
> * subversion/libsvn_subr/io.c
>   (svn_io__is_finfo_read_only): As above
>   (svn_io__is_finfo_executable): As above but move the permission check
> code
>  from here
>   (svn_io_is_file_executable): .. to here since access() wants a path and
> we
>already have it in the arguments.
>
> Suggested by: Joe Orton
> See https://lists.apache.org/list?d...@apr.apache.org
>
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1915214=1915213=1915214=diff
>
> ==
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Sat Jan 13 08:56:50 2024
> @@ -2531,27 +2531,14 @@ svn_io__is_finfo_read_only(svn_boolean_t
> 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(, , 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);
> -
> +  *read_only = (access(file_info->fname, W_OK) != 0);
> +  /* svn_io__is_finfo_read_only can be called with a dangling
> +   * symlink. access() will check the permission on the missing
> +   * target and return -1 and errno = ENOENT. Check for ENOENT
> +   * and pretend the file is writeable, otherwise we will get
> +   * spurious Reverted messages on the symlink.
> +   */
> +  if (*read_only && errno == ENOENT) *read_only = FALSE;
>  #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
>*read_only = (file_info->protection & APR_FREADONLY);
>  #endif
> @@ -2564,33 +2551,7 @@ svn_io__is_finfo_executable(svn_boolean_
>  apr_finfo_t *file_info,
>  apr_pool_t *pool)
>  {
> -#if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
>

I'm not sure if we need to keep APR_HAS_USER here. It was required for...


> -  apr_status_t apr_err;
> -  apr_uid_t uid;
> -  apr_gid_t gid;
> -
> -  *executable = FALSE;
> -
> -  apr_err = apr_uid_current(, , pool);
>

... this function.



> -
> -  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);
> -
> -  else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
> -*executable = (file_info->protection & APR_GEXECUTE);
> -
> -  else
> -*executable = (file_info->protection & APR_WEXECUTE);
> -
> -#else  /* WIN32 || __OS2__ || !APR_HAS_USER */
> -  *executable = FALSE;
> -#endif
> -
> -  return SVN_NO_ERROR;
> +  return svn_io_is_file_executable(executable, file_info->fname, pool);
>  }
>
>  svn_error_t *
> @@ -2599,12 +2560,7 @@ svn_io_is_file_executable(svn_boolean_t
>apr_pool_t *pool)
>  {
>  #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
>

Same here...


> -  apr_finfo_t file_info;
> -
> -  SVN_ERR(svn_io_stat(_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
> -  pool));
> -  SVN_ERR(svn_io__is_finfo_executable(executable, _info, pool));
> -
> +  *executable = (access(path, X_OK) == 0);
>  #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
>*executable = FALSE;
>  #endif
>
>
>
Your thoughts?

Kind regards,
Daniel


svn commit: r1915225 - /subversion/site/staging/mailing-lists.html

2024-01-13 Thread dsahlberg
Author: dsahlberg
Date: Sat Jan 13 18:39:01 2024
New Revision: 1915225

URL: http://svn.apache.org/viewvc?rev=1915225=rev
Log:
In site/staging:

Add instructions how to download mbox files

Suggested by: danielsh

Update link to (retired) mail-search.a.o to lists.a.o

* mailing-lists.html
  (#list-list): Change link
  (#downloading): New section

Modified:
subversion/site/staging/mailing-lists.html

Modified: subversion/site/staging/mailing-lists.html
URL: 
http://svn.apache.org/viewvc/subversion/site/staging/mailing-lists.html?rev=1915225=1915224=1915225=diff
==
--- subversion/site/staging/mailing-lists.html (original)
+++ subversion/site/staging/mailing-lists.html Sat Jan 13 18:39:01 2024
@@ -380,8 +380,11 @@ delay for your post to appear (see below
 
 
   Archives:
-  Not public.
-  Only https://mail-search.apache.org/pmc/private-arch/subversion-private/;>full 
committers and https://mail-search.apache.org/pmc/private-arch/subversion-private/;>ASF 
Members have access.
+  
+  https://lists.apache.org/list.html?priv...@subversion.apache.org;
+  >lists.apache.org (searchable, not public)
+  
+  Only full committers and ASF Members have access.
 
 
   
@@ -469,6 +472,27 @@ delay for your post to appear (see below
  
 
 
+
+Downloading mbox files
+  
+
+
+If you want to download the archives as mbox files, you can do this
+   from https://lists.apache.org;>lists.apache.org (use
+   the download link in the upper right).
+
+If you want to download the mbox files regularly, you can use the
+   following url:
+   
+https://lists.apache.org/api/mbox.lua?list=dev=subversion.apache.org=2022-12
+
+   Replace the list argument with the name of the list in the table above
+   and change the year/date.
+
+ 
+
+
 
 Announcements
   

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 

svn commit: r1915214 - /subversion/trunk/subversion/libsvn_subr/io.c

2024-01-13 Thread dsahlberg
Author: dsahlberg
Date: Sat Jan 13 08:56:50 2024
New Revision: 1915214

URL: http://svn.apache.org/viewvc?rev=1915214=rev
Log:
Replace the homegrown checks for readonly/executable with calls to access(2)
to consider, for example, user's secondary groups.

* subversion/libsvn_subr/io.c
  (svn_io__is_finfo_read_only): As above
  (svn_io__is_finfo_executable): As above but move the permission check code
 from here
  (svn_io_is_file_executable): .. to here since access() wants a path and we
   already have it in the arguments.

Suggested by: Joe Orton
See https://lists.apache.org/list?d...@apr.apache.org


Modified:
subversion/trunk/subversion/libsvn_subr/io.c

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1915214=1915213=1915214=diff
==
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sat Jan 13 08:56:50 2024
@@ -2531,27 +2531,14 @@ svn_io__is_finfo_read_only(svn_boolean_t
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(, , 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);
-
+  *read_only = (access(file_info->fname, W_OK) != 0);
+  /* svn_io__is_finfo_read_only can be called with a dangling
+   * symlink. access() will check the permission on the missing
+   * target and return -1 and errno = ENOENT. Check for ENOENT
+   * and pretend the file is writeable, otherwise we will get
+   * spurious Reverted messages on the symlink.
+   */
+  if (*read_only && errno == ENOENT) *read_only = FALSE;
 #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
   *read_only = (file_info->protection & APR_FREADONLY);
 #endif
@@ -2564,33 +2551,7 @@ svn_io__is_finfo_executable(svn_boolean_
 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;
-
-  *executable = FALSE;
-
-  apr_err = apr_uid_current(, , 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);
-
-  else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
-*executable = (file_info->protection & APR_GEXECUTE);
-
-  else
-*executable = (file_info->protection & APR_WEXECUTE);
-
-#else  /* WIN32 || __OS2__ || !APR_HAS_USER */
-  *executable = FALSE;
-#endif
-
-  return SVN_NO_ERROR;
+  return svn_io_is_file_executable(executable, file_info->fname, pool);
 }
 
 svn_error_t *
@@ -2599,12 +2560,7 @@ svn_io_is_file_executable(svn_boolean_t
   apr_pool_t *pool)
 {
 #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
-  apr_finfo_t file_info;
-
-  SVN_ERR(svn_io_stat(_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
-  pool));
-  SVN_ERR(svn_io__is_finfo_executable(executable, _info, pool));
-
+  *executable = (access(path, X_OK) == 0);
 #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
   *executable = FALSE;
 #endif