Stefan Sperling <stsp_at_elego.de> 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*
Received on 2013-11-11 15:40:03 CET