Re: [PATCH 2/2] diff: --quote-path-with-sp

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 16, 2021, 09:06 UTC
Message-ID: <87ee9ohnqd.fsf@evledraar.gmail.com>
In-Reply-To: <20210915223316.1653443-3-gitster@pobox.com>

On Wed, Sep 15 2021, Junio C Hamano wrote:

[CC-ing Andreas Gruenbacher who's currently active in GNU patch development, and Paul Eggert at his current address, in case he's curious about this blast from the past.

Both: The full context for this proposed change in Git is at
https://lore.kernel.org/git/20210915223316.1653443-1-gitster@pobox.com/]

Show 6 quoted lines

> Long time ago, we had a discussion with GNU patch/diff maintainer
> and agreed that pathnames with certain "difficult" bytes needs to be
> quoted to ensure the resulting patch is machine parseable in an
> unambiguous way [*1*].  Recently, we saw a report that found that
> GNU patch is unhappy with our diff output for a path with SP in it
> [*2*].

It would also be good to mention 4f6fbcdcf96 (Functions to quote and unquote pathnames in C-style., 2005-10-14) here, which is what came out of the [*1*] discussion you're citing, and that also link to the later:

    https://lore.kernel.org/git/7vll0wvb2a.fsf@assigned-by-dhcp.cox.net/

Where you quote a message of Paul Eggert's that didn't make it into the archive, but which AFAICT accurately summarizes the behavior in 4f6fbcdcf96.

Whereas the [*1*] you linked to is still the early proposal of handling UTF-8 specially (not quoting it), which doesn't appear to be what either GNU patch or Git went for in the end (both fully quote some fairly vanilla (also in latin1) UTF-8 when I tested it).

It's still not clear to me if what was agreed upon was accurately implemented by Git at the time, but that GNU patch had a bug vis-a-vis the desired discussed behavior, if the bug is Git's, or both etc. Does a fix still need to be made in GNU patch?

There's also a mention of busybox's interaction with this behavior in https://lore.kernel.org/git/YUK7Bl9uzNE1YErg@tilde.club/; has anyone (you or Gwyneth) sent them an FYI about this in case they'd like to adjust the behavior of their patch tool?

Show 19 quoted lines

> Teach "git diff" and friends the "--quote-path-with-sp" option, that
> encloses a pathname with SP in it inside a pair of double-quotes,
> even though there is otherwise no byte in the pathname that need to
> be encoded in the octal.
>
> As an earlier parts of t/t3902 (outside the patch context) shows,
> output from "ls-files", "ls-tree", and "diff --name-only" all follow
> the same rule to decide paths with what bytes in them need quoting
> and how they are quoted.
>
> This experimental option deliberately refrains from touching these
> output and affects ONLY the paths that appear in the patch header,
> i.e. "diff --git", "--- a/path" and "+++ b/path" lines, that GNU
> patch may care.  This is to minimize potential damage this change
> may cause to tools and scripts the users have been relying on.
>
>  *1* https://lore.kernel.org/git/87ek6s0w34.fsf@penguin.cs.ucla.edu/
>  *2* https://lore.kernel.org/git/YR9Iaj%2FFqAyCMade@tilde.club/
> [...patch omitted...]