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

> -----Original Message-----
> From: Philip Martin [mailto:philip.martin_at_wandisco.com]
> Sent: vrijdag 22 november 2013 14:31
> To: Branko Čibej
> Cc: dev_at_subversion.apache.org
> Subject: Re: Fwd: [PATCH] Fix for `svn patch` changing permissions of
> patched files
>
> Branko Čibej <brane_at_wandisco.com> writes:
>
> >
> > We're obviously making a lot of assumptions about which permissions are
> > correct. When we replace a file in the working copy, we should copy the
> > file's permissions, not invent new ones based on either the source
> > (which may be an arbitrary temporary file) or the umask.
> >
> > Of course, this is moderately easy on Unix (ignoring filesystems that
> > support ACLs) and quite a major pain on Windows.
>
> Yes, I think the aim of preserving the permissions when replacing files
> is good. We also want to avoid changing the permissions of files in the
> system temporary dir, those should remain 600. Running 'svn diff' on a
> working copy that has a file that requires translation reveals two
> different bugs that cause files in the system temp dir to have their
> permissions changed:
>
> - svn_subst_copy_and_translate4 uses the parent directory of a
> temporary file as the temporary directory for a call to
> svn_io_open_unique_file3. That causes svn_io_open_unique_file3 to
> set the permissions on the file according to umask.

This sounds like a left over of the .svn/tmp directories in the old per directory working copies.

It should probably use either the right .svn/tmp directory below the working copy root or the system temporary folder. But as a subst function it can't get the working copy information.

> - svn_subst_copy_and_translate4 copies the permissions from the
> working file to a file in the system temp dir.

Quite possible an artifact of converting this code, and adding permission changes in some code paths. (Originally we found that we created too readable files in global temp...) Only in 1.6/1.7 we started really using the global temp directory.

Branko: On Windows we never alter permissions. We let the inherited folder permission handle these things. A really nice case where the ACL's make our code easier, by not letting user code worry about this.
(There is also a per user temporary directory instead of one for all users)

        Bert
Received on 2013-11-22 15:21:38 CET