On Mon, Oct 28, 2013 at 07:11:13AM +0100, Attila Nagy wrote:
> Hi,
>
> On 10/24/13 22:05, Branko Čibej wrote:
> >>
> >>As Thorsten has pointed out, this is a different case. BTW, I have a
> >>similar solution, like contrib/asvn, but the current operation of svn
> >>makes it impossible/very hard to make it work, because it screws up
> >>real file permissions on each commits.
> >Yes, that's a valid point. Subversion will possibly write the file
> >and/or change permissions on commit in two cases, I think:
> >
> > * The file has the svn:needs-lock property set, and has to be made
> > read-only after commit; or,
> > * The file contains keywords, which have to be updated (e.g., author
> > and revision change after a commit).
> I'm not aware of these.
> Subversion doesn't rewrite the file, because it has the same inode before
> and after the commit. And according to a trace, it does a direct chmod on
> it.
> So if neither of these are standing, is this a bug?
I believe it's the stupid code replaced below, which I wrote in r880420.
Because of it we end up setting perms based on umask upon every commit,
and end up expanding restrictive file permissions.
This patch should fix the problem.
Note that Subversion users cannot and couldn't ever rely on per-file
permission flags to keep files in a working copy hidden from other
users. This is because Subversion often resets permission bits when
it re-creates working files from temporary files. There is no way to
set bits of those files to something other than the current umask.
If you have secret files in your working copy, the only way to keep
them secret is to set restrictive permissions on the top-level directory
of the working copy, or set a restrictive umask. (It might be nice to
have a user-configurable default umask for the svn process, but that's
future work.)
[[[
* subversion/libsvn_subr/io.c
(io_set_file_perms): Set the user read/write bits to make a file read/write,
instead of merging in the default bits from the current umask.
Merging bits from the current umask is a bad idea if the file's
permissions are more restrictive than the umask implies.
]]]
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1536349)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -1591,14 +1591,9 @@ io_set_file_perms(const char *path,
{
if (enable_write) /* Make read-write. */
{
- apr_file_t *fd;
-
- /* Get the perms for the original file so we'll have any other bits
- * that were already set (like the execute bits, for example). */
- SVN_ERR(svn_io_file_open(&fd, path, APR_READ,
- APR_OS_DEFAULT, pool));
- SVN_ERR(merge_default_file_perms(fd, &perms_to_set, pool));
- SVN_ERR(svn_io_file_close(fd, pool));
+ /* Tweak the owner bits only. The group/other bits aren't safe to
+ * touch because we may end up setting them in undesired ways. */
+ perms_to_set |= (APR_UREAD|APR_UWRITE);
}
else
{
Received on 2013-10-30 09:57:21 CET