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
