You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ds...@apache.org on 2024/01/13 09:16:26 UTC
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&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,
- ¬ify_required,
+ ¬ify_required, ¬ify_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"),
Re: svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c
Posted by Daniel Sahlberg <da...@gmail.com>.
Den lör 13 jan. 2024 kl 10:16 skrev <ds...@apache.org>:
> 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.
>
...
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"),
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
Re: svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c
Posted by Daniel Sahlberg <da...@gmail.com>.
Den mån 29 jan. 2024 kl 13:26 skrev Jun Omae <ju...@gmail.com>:
> On 2024/01/13 18:16, dsahlberg@apache.org wrote:
> > 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
>
> After r1915215, make check and check-apache-javahl failed.
>
> [[[
> Summary of test results:
> 2558 tests PASSED
> 168 tests SKIPPED
> 81 tests XFAILED (17 WORK-IN-PROGRESS)
> 27 tests FAILED
> ]]]
>
That was a stupid mistake on my part. I obviously only checked part of the
testsuite. After r1915466, the testsuite passes again.
>
> [[[
> There was 1 error:
> 1)
> testDiff(org.apache.subversion.javahl.BasicTests)org.apache.subversion.javahl.ClientException:
> The operation was interrupted
> svn: Operation cancelled
> svn: Wrapped Java Exception
>
> at org.apache.subversion.javahl.SVNClient.revert(Native Method)
> at org.apache.subversion.javahl.SVNClient.revert(SVNClient.java:253)
> at
> org.apache.subversion.javahl.BasicTests.testDiff(BasicTests.java:3419)
> at
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at org.apache.subversion.javahl.RunTests.main(RunTests.java:119)
> Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 90 out of
> bounds for length 80
> ... 19 more
>
> FAILURES!!!
> Tests run: 148, Failures: 0, Errors: 1
>
> make: *** [Makefile:534: check-apache-javahl] Error 1
> ]]]
>
The java tests also pass for me now.
Kind regards,
Daniel
>
> --
> Jun Omae <ju...@gmail.com> (大前 潤)
>
>
Re: svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c
Posted by Jun Omae <ju...@gmail.com>.
On 2024/01/13 18:16, dsahlberg@apache.org wrote:
> 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
After r1915215, make check and check-apache-javahl failed.
[[[
Summary of test results:
2558 tests PASSED
168 tests SKIPPED
81 tests XFAILED (17 WORK-IN-PROGRESS)
27 tests FAILED
]]]
[[[
There was 1 error:
1) testDiff(org.apache.subversion.javahl.BasicTests)org.apache.subversion.javahl.ClientException: The operation was interrupted
svn: Operation cancelled
svn: Wrapped Java Exception
at org.apache.subversion.javahl.SVNClient.revert(Native Method)
at org.apache.subversion.javahl.SVNClient.revert(SVNClient.java:253)
at org.apache.subversion.javahl.BasicTests.testDiff(BasicTests.java:3419)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at org.apache.subversion.javahl.RunTests.main(RunTests.java:119)
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 90 out of bounds for length 80
... 19 more
FAILURES!!!
Tests run: 148, Failures: 0, Errors: 1
make: *** [Makefile:534: check-apache-javahl] Error 1
]]]
--
Jun Omae <ju...@gmail.com> (大前 潤)