Re: svn commit: r1915214 - /subversion/trunk/subversion/libsvn_subr/io.c
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
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
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
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
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