On 2/21/22 11:54 AM, Daniel Shahaf wrote:
> Ruediger Pluem wrote on Mon, Feb 21, 2022 at 08:49:08 +0100:
>> On 2/19/22 5:07 PM, Daniel Shahaf wrote:
>>> I can't reproduce this. Using unpatched trunk:
>>
>> I checked again and I was able to reproduce it with 1.10.
>> With 1.13 though it shows the behavior you describe for trunk below.
> ⋮
>>> How do other patch tools behave in this case?
>>
>> I tried GNU patch 2.7.6 and it kept the permissions of the file to be
>> patched. No matter what umask was set.
>> git 2.27.0 on the other side behaves like svn trunk: The file permissons are
>> affected by the umask.
>
> And «svn update» creates the new file with umask permissions too. E.g.,
> «chmod 0644 README; umask 077; svn up -r PREV README; stat README» gives
> 0600 (with current trunk).
I can confirm this behavior even with 1.10.
>
> I'm not sure what behaviour I would expect. On the one hand, umask only
> kicks in because of an implementation detail of atomic renames; there's
> no business logic reason to use the "default permissions for new files"
> value when changing an existing file. On the other hand, there's an
> argument that «svn patch» and «svn up» should be consistent with each
> other, and changing the latter is more likely to break people's
> proverbial spacebar heating.
These are good points. Another possible option which is not covered by the
proposed
patch would be to offer options to svn patch and svn up that the permissions
of existing files should be kept (possibly also or instead an option for
.subversion/config). This would ensure that the behavior
of svn up and svn patch is consistent by default and remains the same as
currently but offers people who need to keep the
permissions an option.
If this looks like a way forward I would at least have a look into it. No
promise yet that I can provide a patch though :-)
>
> Even if we don't consense on what the behaviour should be, we could at
> least add a test that expects the current behaviour, in order to ensure
> we don't change it unintentionally.
+1
Regards
Rüdiger
