Stefan Sperling wrote on Thu, Feb 10, 2022 at 09:29:12 +0100: > On Thu, Feb 10, 2022 at 12:10:08AM -0600, Karl Fogel wrote: > > On 09 Feb 2022, Ruediger Pluem wrote: > > > When rebuilding my own Subversion build I stumbled across the following > > > patch that I add to my build: > > > > > > Index: subversion/libsvn_client/patch.c > > > =================================================================== > > > --- subversion/libsvn_client/patch.c (revision 1897905) > > > +++ subversion/libsvn_client/patch.c (working copy) > > > @@ -3246,6 +3246,15 @@ install_patched_target(patch_target_t *target, > > > con > > > target->content->eol_style == > > > svn_subst_eol_style_native); > > > > > > + /* Make sure the patched file has the same permissions the > > > + * original file, but only if it does not get added. > > > + */ > > > + if (!target->added) > > > + { > > > + SVN_ERR(svn_io_copy_perms( > > > + target->local_abspath, target->patched_path, > > > pool)); > > > + } > > > + > > > SVN_ERR(svn_subst_copy_and_translate4( > > > target->patched_path, > > > target->move_target_abspath > > > > > > It ensures that files that get modified (not added) during svn patch > > > keep their permissions. Can this be added to trunk? > > > > I'm curious: what is the case that causes the patched target file to have > > different permissions than it had before the patch? (Or am I > > misunderstanding what you're saying?) > > Possibly related to the fact that the patched target is prepared inside > a temporary file, which has 0600 permissions by default since it gets > created by mkstemp(3) (see https://svn.apache.org/r880338). > > This temp file is then renamed on top of the file in the working copy and > file mode bits will be copied 1:1. We have had similar reports before, where > files ended up being readable only by the user who was using the working > copy, and where this interfered with some other use of such files.
I can't reproduce this. Using unpatched trunk: % echo >> iota % svn diff iota > diff % svn revert -q -R ./ % zstat -or +mode iota 0100644 (-rw-r--r--) % svn patch diff U iota % zstat -or +mode iota 0100644 (-rw-r--r--) % svn st -q M iota % What I do see, however, is that the patched file's permissions are affected by the umask. I.e., if I change the umask before calling «svn patch», then iota's permissions are the permissions of a new file under the updated umask. How do other patch tools behave in this case? Cheers, Daniel > > If you feel like writing a regression test for this case, then I could > > probably answer my question by looking at the test's code (but I realize you > > might not have time to do that). > > I agree that having a test case to cover this behaviour would be good. > A quick glance over subversion/tests/cmdline/patch.py suggests that > this behaviour is not covered by our tests yet? That would be unfortunate. > > Ruediger, you may not be aware that our /subversion subtree of the ASF > SVN repository is open for all ASF committers. You should already have > write access. So once we are done discussing this change, you will be > able to add this change to trunk yourself. > > Cheers, > Stefan