On Wed, 18 Mar 2026 18:08:47 +0100
Mattias Andrée <m_AT_maandree.se> wrote:
> On Wed, 18 Mar 2026 12:44:28 +0100
> "Roberto E. Vargas Caballero" <k0ga_AT_shike2.net> wrote:
>
> > On Tue, Mar 10, 2026 at 08:28:11PM +0100, Mattias Andrée wrote:
> > > + * Petty POSIX violations:
> > > + * - Rejected hunks are saved without timestamps.
>
> I don't recall, but it shouldn't matter. But I will look into it.
>
> > > + * - Questions are prompted to /dev/tty rather than to /dev/stdout.
>
> POSIX specifies that the input shall be read from the controlling terminal,
> not from standard input, when prompting for filenames. It does not give
> any guidance for when it asks about reversing or unreversing patches, except
> that the STDIN section only refers to INPUT FILES which just says input files
> shall be text files — therefore reading from /dev/tty is legal and seems
> more correct.
>
> Regarding the output of the prompt, the POSIX says this should be done to
> standard output, but I though it would be better to print the prompt to the
> controlling terminals as the output may be a file. It didn't really seem
> reasonable to prompt to standard output but read from the controlling terminal.
>
> > > + * - -N skips applied hunks rather than applied patches.
>
> I want to recall that it was just easier to implement. But I do also think
> it is more useful.
>
> >
> > What is the reasoning about these deviations? I see that getting questions
> > from /dev/tty breaks well stablished use cases (like for example using yes).
> >
> > > +static int
> > > +file_exists(const char *filename)
> > > +{
> > > + char *adjusted;
> > > + int ret;
> > > +
> > > + if (strstart(filename, "/dev/"))
> > > + return 0;
> >
> > Why this deviation? I think patch should deal with any file in a transparent
> > way. This breaks the least surprise rule.
>
> I'll investigate, but I think its just used for when it read the filename
> from the patch file, in which case I think it is correct.
I believe this behaviour is correct. It is used when reading filenames from
the patch and when determining whether to back up a file. You do not want
to use filename like /dev/fd/3 when they appear in the file because diff
was run with a `<( )`, or even /dev/stdin. Likewise, you do not want to
attempt to back up such files.
>
> >
> > In general I think the implemenation looks nice, and I think after solving
> > these small question it should be added to the repository. Would be nice if
> > you can add some tests for this new tool (I suppose you had to test it while
> > you developed, so it should be just a question of adapting the tests).
>
> I've used these test cases https://git.maandree.se/base-util-tests/tree/patch-test,
> it caught bugs in GNU patch but not in this patch. I've also been daily driving
> this for 10 years now without any encountering any problem.
>
> There are test cases for a few other utilities https://git.maandree.se/base-util-tests/
> as well that could also be ported over.
>
> >
> > Regards,
> >
>
>
Reading the specification again, I noticed that it said that if the edited lines
cannot be found, patch(1) shall try removing the first and last line of context,
and if that doesn't help, the two first and two last lines of context, and
optionally more still if it cannot fine the line. Apparently, I missed that part
when I implemented this.
I think this is a really bad idea, but it should probably be implemented anyways.
I think it's enough (and best) to do the absolutely minimum here and do nothing
else than ignore up to two lines of context of either end and print a short
message to the user, just like in the case of line number fuzzing.
Received on Thu Mar 19 2026 - 17:56:05 CET