9front - general discussion about 9front
 help / color / mirror / Atom feed
From: Michael Forney <mforney@mforney.org>
To: 9front@9front.org
Subject: [9front] patch(1) filename detection
Date: Wed, 28 Sep 2022 22:49:37 -0700	[thread overview]
Message-ID: <6335318d.170a0220.77fb0.8d3c@mx.google.com> (raw)

I recently encountered an issue using patch and git.

Say I remove two files in git and generate a patch:

diff 0890cdaa2d2a6ad756ba98130e078b3cd64bee7d 5b6279eb53ece69d2a014adf566f22adb61d94e4
--- a/a
+++ /dev/null
@@ -1,1 +1,0 @@
-foo
--- a/b
+++ /dev/null
@@ -1,1 +1,0 @@
-bar

When I try to apply this, we process the first hunk. curfile is nil,
so we set it to /dev/null, then read the contents of a into f, and
apply the hunk.  Then, we get to the second hunk. It also has newpath
of /dev/null, so we *do not* read the contents of b. However, now we
are searching for the contents of b in a, which fails:

	stdin:8: unable to find hunk offset in b

Historically, I believe patch implementations worked by using the
first file that existed of the --- name and the +++ name, falling back
to prompting the user for the name of the file to patch. The new
9front patch works a bit differently and patches the --- name into the
+++ name, with a bit of logic to remove files with /dev/null as the
+++ name.

I don't think this is ideal behavior, because it is common to see
patches like

--- a.orig	Wed Sep 28 21:53:55 2022
+++ a		Wed Sep 28 21:53:58 2022
@@ -1 +1 @@
-foo
+bar

for example in BSD ports systems. Given this file, patch(1) will
complain

	patch: open a.orig: 'a.orig' file not found

rather than patching the file a. Similarly, given a patch like

--- a
+++ a.new
@@ -1,1 +1,1 @@
-foo
+bar

patch(1) will write the patched file to a.new and leave a alone
rather than patching a.

I propose we implement the standard behavior in patch(1) (maybe
without the prompting part) to interoperate better with patches
created by other tools. This will fix the issue I encountered since
there is no separate "old path" and "new path", just the file selected
for patching.

We can still retain the logic to remove the file if the +++ name is
/dev/null. However, longer term it might be worth supporting the
git-diff extended header lines:

	deleted file mode <mode>
	new file mode <mode>
	old mode <mode>
	new mode <mode>

There are also headers for renames and copies, but I don't think git
or patch should bother with that, and should just present them as
deleted and added files.

Thoughts?

             reply	other threads:[~2022-09-29  5:49 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29  5:49 Michael Forney [this message]
2022-09-29  6:47 ` unobe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6335318d.170a0220.77fb0.8d3c@mx.google.com \
    --to=mforney@mforney.org \
    --cc=9front@9front.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).