Re: Fix issue SVN-4622 (Was: Re: "svn revert -R ." outputs spurious Reverted messages)

[Vincent Lefevre]

On 2023-12-26 16:30:21 +0100, Daniel Sahlberg wrote:
> TLDR; svn status is verifying read-only status for each file (based on
> svn:needs-lock).

I never use the svn:needs-lock property. So I don't see why svn
tries to modify the read-only status in this case.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Fix issue SVN-4622 (Was: Re: "svn revert -R ." outputs spurious Reverted messages)

[Daniel Sahlberg]

Hi,

Prompted by Vincent Lefevre's e-mail (below, see thread at [1]), I took at
look at SVN-4622, or at least the second half of it.

TLDR; svn status is verifying read-only status for each file (based on
svn:needs-lock). Under Windows there is a separate read-only flag so this
is easily done and fixed. Under *nix the code will check for the normal
permission bits, first on user (if current user is owner), then on group
(if current user's group is the owner group) and finally on world. A file
with 664 permissions not owned by the current user/group will be seen as
read-only and the code will try to fix this (failing with Permissions
denied but still reporting Reverted).

The permissions code is in svn_io__is_finfo_read_only().
The code trying to "fix" the permissions is in revert_wc_data().

One way to solve this would be to add a separate argument to
svn_io__is_finfo_read_only() signalling if the file is not owned by the
current user (directly or via a group). This could then be picked up by
revert_wc_data() to output a separate message "Incorrect
permissions/ownership on XX" instead of the "Reverted".

There are some additional uses of svn_io__is_finfo_read_onl() but they can
easily fixed. As far as I understand, this function is a private function
so it doesn't require a changed API.

What do you think?

Kind regards,
Daniel

[1] https://lists.apache.org/thread/p1ky889bxwy8okqly7h1lgckxfpldnxs


Den sön 24 dec. 2023 kl 01:35 skrev Vincent Lefevre :

> On 2023-12-23 22:44:51 +0100, Daniel Sahlberg wrote:
> > Thank you for your report and also for finding the root cause. I can
> > confirm this is the same on my machine. I'd like to take a closer look
> but
> > I'm not quite sure when I will find the time.
> >
> > This has been reported before:
> > https://issues.apache.org/jira/browse/SVN-4622
>
> Indeed. Not for the original bug report, but what is said in the
> comments.
>
> I'm wondering why svn wants to change the permissions, at least if
> there is nothing to revert for the concerned file. I would see this
> as a (separate, but related) bug. FYI, in my working copies, I set
> some files as read-only in order to make sure I won't modify them
> by mistake (even though there would be no data loss with a revert,
> except the timestamp). So I think that this is a bad behavior if
> "svn revert" changes the write permission.
>
> --
> Vincent Lefèvre  - Web: 
> 100% accessible validated (X)HTML - Blog: 
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
>