Re: [bug-patch] How to handle read-only trees

[Jean Delvare]

Hi Andreas,

Thanks for your fast reply.

On Fri, 10 Mar 2017 16:45:35 +0100, Andreas Grünbacher wrote:
> 2017-03-10 12:16 GMT+01:00 Jean Delvare <address@hidden>:
> > Hi all,
> >
> > Where I work, we use GNU patch to work on read-only source trees. The
> > reason is that we work with hard-linked trees, and apply different
> > patch series to the different copies. We want to make sure nobody
> > accidentally affects another tree by changing a file without first
> > breaking the hard link.
> >
> > While it works somewhat, there are still 2 issues left that I'd like to
> > solve.
> >
> > 1* Since April 2012 (commit 9a26fde226), patch complains when patched
> > files are read-only. That's a lot better than failing, as was the case
> > between May 2010 (commit cabd1add57) and April 2012 (thanks Andreas for
> > fixing that!) but still not too convenient. I know that users can pass
> > option --read-only=ignore to get rid of the warning, but this option is
> > not supported on older versions of GNU patch, and our users may have to
> > work on older systems sometimes, so they have to change their habits
> > depending on the system they are working on.
> >
> > More importantly, I believe it is conceptually wrong to complain about
> > patching a read-only file if the user also asks for the original file
> > to be backed up (which we always do.) If you keep a backup of the
> > original read-only file, then modifying a copy of that file is not
> > breaking the read-only paradigm.
> >
> > So I would like to propose that option --read-only defaults to "ignore"
> > if --backup is passed, and to "warn" if not (as is the case today.)
> > What do you think?
> 
> The fact that quilt uses patch in a very particular way doesn't
> actually create a connection between --backup and --read-only; those
> two options serve entirely different purposes.

What makes you consider that quilt uses patch in a "very particular"
way? I see quilt as calling patch exactly as a user would, just
automating things a lot to be more user-friendly and less error-prone.

There is no relation between files being read-only and using quilt.
Files being read-only is something specific to where I work, but
nothing prevents others from using a similar strategy.

Obviously options --backup and --read-only serve different purposes,
but that doesn't mean they are not (or should not) be related.

> > 2* After patching a read-only file, and even if --backup and --read-
> > only=ignore are passed, the patched file is still read-only. This means
> > that our users must manually chmod +w each patched file to which they
> > need to make further changes.
> >
> > It seems conceptually wrong to me, as option --backup created a new
> > copy of the file and patched it, the contents are already modified so
> > the read-only nature of the file is already gone in practice.
> >
> > So I would like to propose that patched files are always made writable
> > when option --backup is used. What do you think?
> 
> I can follow the argument that files should be (owner) writable after
> patch has modified them. But again I don't see the connection with
> --backup.

For me, --backup changes the notion of "original file". Without the
option, you overwrite the original file. With the option, you move the
original file, and create a new file with the original name.

Given that, patching a read-only file without using --backup is
"wrong", in that it breaks the restriction set by the file owner. But
patching a read-only file with --backup is "right" (in my eyes at
least), in that the original file is preserved. As a matter of fact, if
you patch a hard-linked file with --backup, the backed-up file keeps
the hard links while the patched file does not.

We have been working with read-only hard-linked trees for over a
decade. It was working just fine before patch was modified to complain
about this scenario.

I do believe that patch complaining about read-only files in the
absence of --backup makes a lot of sense, as the whole point of making
files read-only is to prevent them from being changed. In that sense
the change in behavior from commit cabd1add57 fixed a serious bug. But
in my opinion it introduced a regression in the --backup case
(mitigated by the commit 9a26fde226, but still.)

> A corner case with quilt is when a patch tries to modify a file but
> doesn't actually end up changing anything. In that case, patch should
> probably still create a backup and make the original file writable, if
> only to make life easier for quilt.

I'm sorry, I'm not sure I follow you here. Do yo mean for example a
no-op patch replacing a line with the exact same line? Or something
else?

Thanks,
-- 
Jean Delvare
SUSE L3 Support