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.
> 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
