Re: Fwd: [PATCH] Fix for `svn patch` changing permissions of patched files

Cathy Fitzpatrick wrote:

> Hi again Philip,
>
> Sorry, I was a bit too hasty in saying this was machine dependent.
> Your test case does respect the umask on all of the machines I am
> trying it on, but
[...]
> [Cathy_at_localhost subversion]$ svn revert INSTALL
> Reverted 'INSTALL'
> [Cathy_at_localhost subversion]$ ls -l INSTALL
> -rw-rw-r--. 1 Cathy Cathy 63057 Nov 22 03:29 INSTALL
> [Cathy_at_localhost subversion]$ umask 070
> [Cathy_at_localhost subversion]$ svn patch patch
> U        INSTALL
> [Cathy_at_localhost subversion]$ ls -ls INSTALL
> 68 -rw-------. 1 Cathy Cathy 63062 Nov 22 03:29 INSTALL

Hi Cathy. I can confirm Philip's recipe behaves as he showed and yours behaves as you showed, also on my system (Ubuntu 12.04, svn trunk).

With your patch and your recipe above I get this result:

$ svn revert INSTALL && ls -l INSTALL && (umask 070 && svn patch patch) && ls -l INSTALL
Reverted 'INSTALL'
-rw-rw-r-- 1 julianfoad julianfoad 63057 Nov 22 10:49 INSTALL
U         INSTALL
-rw----rw- 1 julianfoad julianfoad 63062 Nov 22 10:49 INSTALL

Is that what you get?

> This is just a guess to be verified later: it may be that the bug only
> occurs if a file has an EOL property specified.

That result is the same as in Philip's recipe (without keyword/eol-style). So that seems to me like a good fix.

I was a bit surprised at first because it *adds* world-writable permission, but maybe that's just fine given we specified a umask that allows this. At first I was expecting that it should never add permissions, but only take away any umasked-permissions and preserve the rest. But that's probably a question for another day.

One other thing. In your patch you use "svn_dirent_dirname(wcroot_abspath,)" for the temp-file directory. This puts the temp file *outside* the WC. I haven't checked what we do in other places but using 'wcroot_abspath' iteself would be less likely to fail, and probably even better is the directory where the to-be-patched filed itself is.

- Julian

[Philip's recipe results:]
>>> $ ls -l wc/f
>>> -rw-r--r-- 1 pm pm 2 Nov 22 10:03 wc/f
>>> $ (umask 070 && svn patch p wc)
>>> U        wc/f
>>> $ ls -l wc/f
>>> -rw----rw- 1 pm pm 5 Nov 22 10:00 wc/f
>>>
>>> I get that behaviour with 1.7, 1.8 and trunk.
Received on 2013-11-22 12:12:37 CET