Stefan Sperling <[email protected]> writes: > On Mon, Nov 11, 2013 at 01:40:01PM +0000, Philip Martin wrote: >> I'm looking at r1540417 nominated for backport to 1.8. > > It seems you're talking about r1537018.
Yes. >> The fix allows >> me to remove group/world read and/or read/write access and not have it >> restored by commit even when my umask allows it: >> >> $ ls -l wc/f >> -rw-r--r-- 1 pm pm 2 Nov 11 13:10 wc/f >> $ chmod go-r wc/f >> $ ls -l wc/f >> -rw------- 1 pm pm 2 Nov 11 13:10 wc/f >> $ echo xx >> wc/f >> $ svn ci -mm wc >> $ ls -l wc/f >> -rw------- 1 pm pm 2 Nov 11 13:10 wc/f >> >> There is similar code affecting the execute bit but the behaviour is a >> bit different. If I remove both read and execute then commit doesn't >> restore execute but if I just remove execute it gets restored: >> >> $ ls -l wc/f >> -rwxr-xr-x 1 pm pm 2 Nov 11 13:13 wc/f >> $ chmod go-x wc/f >> $ ls -l wc/f >> -rwxr--r-- 1 pm pm 2 Nov 11 13:13 wc/f >> $ echo xx >> wc/f >> $ svn ci -mm wc >> $ ls -l wc/f >> -rwxr-xr-x 1 pm pm 2 Nov 11 13:13 wc/f >> >> I'm not sure what behaviour we are trying to implement, is restoring >> execute a bug? > > Does wc/f have an svn:executable property in this example? Yes. > The proper way of clearing the 'x' bit would be to remove > the svn:executable property. That removes all x. The r1537018 fix allows users to remove just group/world permissions and have Subversion preserve that change. It appears the same is not possible with x. And if I try to do it by removing svn:executable and setting x just for the owner then commit removes x. > As long as that property exists I think we should restore the 'x' bit > for users who have read access to the file. If svn:executable isn't > set I would expect svn to leave the x' bit alone. > > However, because we have no svn property to control 'r' and 'w' bits > I think we shouldn't mess with them on existing files. We have some control over w via svn:needs-lock. > The umask should affect newly created files only. > It is a problem to apply the umask to existing files because users > might not want that at all (hence the r1537018 nomination). So the desired behaviour is to let users set r/w different from umask and preserve it, but not to preserve x different from umask? But we do preserve x different from umask if combined with r different from umask. I'm not claiming that behaviour would be right or wrong, just that I'm not clear what is intended. > So I think the behaviour you are showing above is fine. > > In any case, we should have a set of regression tests to ensure > that we keep the behaviour we want both documented and working. -- Philip Martin | Subversion Committer WANdisco // *Non-Stop Data*
