On Sun, Jan 7, 2024 at 2:46 AM Vincent Lefevre <vincent-...@vinc17.net> wrote: > On 2024-01-05 11:29:16 +0100, Daniel Sahlberg wrote: > > Den fre 5 jan. 2024 kl 10:51 skrev Johan Corveleyn <jcor...@gmail.com>: > > > > > On Fri, Jan 5, 2024 at 8:46 AM Daniel Sahlberg > > > <daniel.l.sahlb...@gmail.com> wrote: > > > ... > > > > Since the file doesn't have svn:needs-lock it should be RW [and the > > > Reverted message comes from Subversion trying to restore the W flag ...] > > > > > > Should it? Intuitively I'd say: since the file doesn't have > > > svn:needs-lock Subversion shouldn't be looking at R or RW. Why should > > > we make a file RW? Can't the user make a file readonly just locally, > > > and expect Subversion not to care? > > > > > > Or is "making a file readonly" a committable local change? Will it > > > show up on 'svn st' and can it be committed as some change that can be > > > transferred to another working copy? > > > > > > I understand that svn:needs-lock adds extra handling of the readonly > > > status of files, but without that property? > > > > All good questions, and I probably agree with you: if svn:needs-lock isn't > > set then Subversion could just ignore the R/RW status. > > I also agree. I never use svn:needs-lock, and I want to be able to > set some files in my working copy to read-only, in order to make sure > that I won't modify them by mistake. > > > But any change here would change previous behaviour so it would need > > a solid consensus. > > In any case, the current behavior of Subversion is inconsistent. > "svn revert" is not documented as the command to "fix" the permissions. > > > If the check is removed for files that doesn't svn:needs-lock, then we > > might have to add code to restore RW status if svn:needs-lock is removed. > > > > Making a file readonly is currently not a committable change, didn't check > > 'svn st' but it will be reverted by 'svn revert' and it will not be > > transferred to another WC. It can only be committed indirectly via > > svn:needs-lock. > > > > Any discussion regarding svn:needs-lock probably also have to consider > > svn:executable, since it is handled similarly (except on WIN32 and OS2, > > where the concept of executable doesn't exists). > > > > I havn't completely made up my mind, but I think I favour keeping the > > current behaviour: R/RW status in indicated by the svn:needs-lock property > > and you shouldn't change R/RW manually within a WC. > > Then this should be documented. > > But "svn st" should detect and report incorrect permissions, and > "svn up" should fix them, just like what happens when a file has > been removed with just "rm" instead of "svn delete".
Interesting discussion. I agree it should at least be documented, and perhaps be made a bit more clear from the output of 'revert' (but not sure how far we can go without breaking compat). Changing the current behavior is probably a more risky move, given the maturity of SVN and backwards compatibility etc. BTW, I did some more digging because the problem looked familiar, and apparently there was a discussion on users@ in 2016 [1] in which I participated :). It was reported with a Windows client. I ended up filing an issue which contains some interesting background information by Bert [2]. A quote that stood out to me just now when I was rereading it: "For WC-NG we explicitly made 'svn revert' always revert your working copy match how it would be after a clean checkout. (I think philipm worked on this at the time). And he did quite a bit more work to ensure that we always provide a notification when we change the working copy. (Older versions could change your working copy, but not notify you about this) ... I think it might have been better to use a specific notify for 'fixing read-only-ness' during revert, but I'm not sure if this is really worth all the backwards compatibility breakage to apply this three versions after this behavior was introduced." BTW, I know about a similar issue of 'spurious revert notifications', with mismatching timestamps (lastmod-time different from svn metadata, a condition that is normally fixed by 'svn cleanup'): if you "touch" a file in your WC so it has a different timestamp from the metadata, it will be notified when running 'svn revert' (and I believe the metadata is adjusted in this case). So that's another source of spurious revert notifications. I thought I had discussed this on one of our mailinglists, but I can't find it. The fact that revert fixes timestamps is mentioned in passing in issue SVN-4162. Perhaps both types of "adjustments" by revert should get a specific notification (if any -- for the timestamp fixup case I can live with simply not notifying, it's an adjustment in metadata, not on-disk file after all). [1] https://svn.haxx.se/users/archive-2016-03/0004.shtml [2] https://issues.apache.org/jira/browse/SVN-4637 (Revert affects unchanged files with changed permissions) [3] https://issues.apache.org/jira/browse/SVN-4162 (make svn status fix timestamp mismatches in meta-data) -- Johan
