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)
>

Re: "svn revert -R ." outputs spurious Reverted messages

[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)

Re: "svn revert -R ." outputs spurious Reverted messages

[Daniel Sahlberg]

Den lör 23 dec. 2023 kl 04:10 skrev Vincent Lefevre :

> On 2023-12-23 03:49:04 +0100, Vincent Lefevre wrote:
> > In one of my working copies:
> >
> > qaa% svn st
> > qaa% svn revert -R .
> > Reverted 'etc/apache2/conf-available/javascript-common.conf'
> > Reverted 'etc/apache2/mods-available/dnssd.load'
> > Reverted 'etc/apache2/mods-available/dnssd.conf'
> > qaa% svn revert -R .
> > Reverted 'etc/apache2/mods-available/dnssd.load'
> > Reverted 'etc/apache2/mods-available/dnssd.conf'
> > Reverted 'etc/apache2/conf-available/javascript-common.conf'
> [...]
>
> I've eventually found the cause: these files has owner root.
> Perhaps a mistake I did when I copied the files (but I am
> wondering how I could have done that). I've now fixed the
> ownership and the messages have disappeared.
>
> However, I don't see why I get such Reverted messages in the case
> of an incorrect owner.
>
> --
> Vincent Lefèvre  - Web: 
> 100% accessible validated (X)HTML - Blog: 
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
>

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

Kind regards,
Daniel Sahlberg

Re: "svn revert -R ." outputs spurious Reverted messages

[Vincent Lefevre]

On 2023-12-23 03:49:04 +0100, Vincent Lefevre wrote:
> In one of my working copies:
> 
> qaa% svn st 
> qaa% svn revert -R .
> Reverted 'etc/apache2/conf-available/javascript-common.conf'
> Reverted 'etc/apache2/mods-available/dnssd.load'
> Reverted 'etc/apache2/mods-available/dnssd.conf'
> qaa% svn revert -R .
> Reverted 'etc/apache2/mods-available/dnssd.load'
> Reverted 'etc/apache2/mods-available/dnssd.conf'
> Reverted 'etc/apache2/conf-available/javascript-common.conf'
[...]

I've eventually found the cause: these files has owner root.
Perhaps a mistake I did when I copied the files (but I am
wondering how I could have done that). I've now fixed the
ownership and the messages have disappeared.

However, I don't see why I get such Reverted messages in the case
of an incorrect owner.

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

"svn revert -R ." outputs spurious Reverted messages

[Vincent Lefevre]

In one of my working copies:

qaa% svn st 
qaa% svn revert -R .
Reverted 'etc/apache2/conf-available/javascript-common.conf'
Reverted 'etc/apache2/mods-available/dnssd.load'
Reverted 'etc/apache2/mods-available/dnssd.conf'
qaa% svn revert -R .
Reverted 'etc/apache2/mods-available/dnssd.load'
Reverted 'etc/apache2/mods-available/dnssd.conf'
Reverted 'etc/apache2/conf-available/javascript-common.conf'
qaa% svn revert -R .
Reverted 'etc/apache2/mods-available/dnssd.conf'
Reverted 'etc/apache2/mods-available/dnssd.load'
Reverted 'etc/apache2/conf-available/javascript-common.conf'
qaa% svn revert -R .
Reverted 'etc/apache2/conf-available/javascript-common.conf'
Reverted 'etc/apache2/mods-available/dnssd.load'
Reverted 'etc/apache2/mods-available/dnssd.conf'
qaa% svn revert -R .
Reverted 'etc/apache2/mods-available/dnssd.load'
Reverted 'etc/apache2/mods-available/dnssd.conf'
Reverted 'etc/apache2/conf-available/javascript-common.conf'

This occurs with the subversion 1.14.2-5+b1 Debian package under
Debian/unstable on one machine, but not on another one.

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