[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1452780 - /subversion/trunk/subversion/libsvn_subr/subst.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Tue, 5 Mar 2013 14:07:11 +0100

> -----Original Message-----
> From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> Sent: dinsdag 5 maart 2013 13:57
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1452780 -
> /subversion/trunk/subversion/libsvn_subr/subst.c
>
> Author: stsp
> Date: Tue Mar 5 12:57:00 2013
> New Revision: 1452780
>
> URL: http://svn.apache.org/r1452780
> Log:
> Address issue #4331, "working copy permissions change on commit for files
> with keywords" by preventing the accidental removal of existing permission
> bits set on files which undergo translation.
>
> This is not a perfect answer to the proposition given in the issue that
> "Committing a file should not change its permissions in the WC."
> Committed files may still get a new owner/group/other write bit set if
> the umask allows this and the file's properties indicate that the file must
> be set read-write (see svn_wc__sync_flags_with_props() which might tweak
> permission bits of the translated file). But I think this is acceptable.
> I'm not sure if never changing the permission bits of committed files is
> a realistic requirement. Changing this would require additional effort
> and might break other scenarios.
>
> * subversion/libsvn_subr/subst.c
> (svn_subst_copy_and_translate4): Copy file permissions from the source
> file
> to the temporary file which stores the translated result. The temporary
> file might have stricter permissions than the source, due to the way
> our APIs for creating temporary files work. Removing permission bits from
> files during translation is an unwanted side effect and should be avoided.
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/subst.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/subst.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/su
> bst.c?rev=1452780&r1=1452779&r2=1452780&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_subr/subst.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/subst.c Tue Mar 5 12:57:00
> 2013
> @@ -1743,7 +1743,12 @@ svn_subst_copy_and_translate4(const char
> }
>
> /* Now that dst_tmp contains the translated data, do the atomic rename.
> */
> - return svn_error_trace(svn_io_file_rename(dst_tmp, dst, pool));
> + SVN_ERR(svn_io_file_rename(dst_tmp, dst, pool));
> +
> + /* Preserve the source file's permission bits. */
> + SVN_ERR(svn_io_copy_perms(src, dst, pool));
> +
> + return SVN_NO_ERROR;
> }

There is a possible race condition here, which might be resolved by copying the permissions before moving.
(But most likely you just get a different race condition by switching the order)

        Bert
Received on 2013-03-05 14:08:05 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.