diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

[Linus Torvalds]

On Fri, Feb 24, 2023 at 8:55 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Ok, it's not quilt's fault, it's GNU diff's fault from what I can tell.
> quilt relies on diff to generate the patch, and I can't figure out how
> to get diff to notice file permissions at all.  Am I just missing
> an option to 'diff' somewhere that I can't find in the manual?

No, I think you're right.

GNU patch was updated long ago to understand and apply the extended
git patch data.

But as far as I can tell, GNU diffutils have never actually grown the
ability to generate those extensions, even though at least the mode
bit one should be fairly simple (the file rename/copy ones are rather
more complicated, but those are just a "make diffs more legible and
compact" convenience thing, unlike the executable bit thing that
allows for scripts to remain executable).

> Anyway, quilt can handle replacing what it uses for 'diff', so I'll just
> replace it with 'git diff' and that seems to solve the problem for me!

That does sound like the right solution.

I don't think the diffutils people really care, but let's cc Paul
Eggert and Jim Meyering anyway, just in case. Because looking at the
diffutils git tree, it's still actively maintained even if the patch
load seems quite low.

Maybe there's some patch floating around that would allow diffutils to
also add those mode change lines. A quick google didn't find anything,
but Paul and Jim would probably know (or can just say "yeah, not even
interested").

                Linus

Re: diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

[Andrew Morton]

On Fri, 24 Feb 2023 11:16:52 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> But as far as I can tell, GNU diffutils have never actually grown the
> ability to generate those extensions, even though at least the mode
> bit one should be fairly simple (the file rename/copy ones are rather
> more complicated, but those are just a "make diffs more legible and
> compact" convenience thing, unlike the executable bit thing that
> allows for scripts to remain executable).

yeah, irritating.

Can we use git instead of diff?  I tried once, but it didn't work -
perhaps because it didn't like doing stuff outside a git repo.

However, trying it now...

hp2:/home/akpm> mkdir foo
hp2:/home/akpm> cd foo
hp2:/home/akpm/foo> date > a
hp2:/home/akpm/foo> cp a b
hp2:/home/akpm/foo> chmod +x a
hp2:/home/akpm/foo> git diff a b
diff --git a/a b/b              
old mode 100755
new mode 100644

Re: diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

[Linus Torvalds]

On Fri, Feb 24, 2023 at 11:32 AM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> Can we use git instead of diff?  I tried once, but it didn't work -
> perhaps because it didn't like doing stuff outside a git repo.

Just using "git diff" does work *but* I would strongly suggest using
"--no-index" as in

    git diff --no-index -- path1 path2

because without the "--no-index" you will find that "git diff" will
use heuristics to decide what it is you want to do.

So if you do just

    git diff path1 path2

and both of those paths are *inside* a git directory, then git thinks
that "oh, you want to see the diff of those two paths against the
current git index", and does something *very* different from showing
the diff between those two paths.

And I suspect that is the exact reason you *thought* it didn't work,
but now that you tried it in a new test-directory, it did work for
you.

With the "--no-index", the ambiguity of "do you want a diff against
git state, or the files against each other" goes away

Just to give another example of this:

 (a) when I'm in my kernel tree, I can do

      $ git diff .config /etc/kernel-config

and it will show me the diff between the two files, because while my
".config" file is inside the repository, "/etc/kernel-config" is
clearly not, so I get that "diff between two files" behavior.

 (b) but then if I do a conceptually similar

    $ git diff .config arch/x86/configs/x86_64_defconfig

then git will see that both paths *are* inside the repository, and
think I'm doing a diff vs the git index state, and since I have no
changes wrt any checked in state in any paths that match, it will show
no diff at all.

So if I actually want to see the file diff between those two paths, I have to do

    $ git diff --no-index .config arch/x86/configs/x86_64_defconfig

to clarify what it is that I want.

Also note that "git diff" is *not* a replacement for the 'diff' binary
from diffutils in general.

Doing a 'git diff' will *only* generate the extended git unified
diffs. There's no support for any other diff format, and while there
is overlap in the command line switches, there's a lot of differences
too.

So "git diff" is very much a "you can use it as a replacement for
plain 'diff', but only in very specific circumstances" thing.

                 Linus

Re: diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

On 2023-02-24 11:16, Linus Torvalds wrote:
>   GNU diffutils have never actually grown the
> ability to generate those extensions

Thanks for pointing this out. I added this to our list of things to do, 
by installing the attached patch to the GNU diffutils TODO file. If this 
patch's wording isn't right, please let me know, as I haven't read this 
whole email thread, just the three emails sent directly to me.

[-- Attachment #2: 0001-maint-add-diff-git-TODO.patch --]
[-- Type: text/x-patch, Size: 894 bytes --]

From 3fa7218567bdcf958f1342c13c848c9e2f2d38c5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 24 Feb 2023 12:13:12 -0800
Subject: [PATCH] maint: add 'diff --git' TODO

* TODO: Suggest better compatibility with 'git diff -p'.
From a suggestion by Linus Torvalds
<https://lkml.org/lkml/2023/2/24/797>.
---
 TODO | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/TODO b/TODO
index 5132de8..25fc22b 100644
--- a/TODO
+++ b/TODO
@@ -1,3 +1,9 @@
+Add --git option to generate output compatible with 'git diff -p'.
+This would behave like 'diff -p', except that it would also generate
+the extended headers 'old mode', 'new mode', 'deleted file mode', and
+'new file mode', and it would quote file names with unusual characters.
+GNU patch already parses this format.
+
 Add --include option (opposite of --exclude).
 
 Look into sdiff improvement here:
-- 
2.37.2

Re: diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

[Linus Torvalds]

On Fri, Feb 24, 2023 at 12:20 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> Thanks for pointing this out. I added this to our list of things to do,
> by installing the attached patch to the GNU diffutils TODO file. If this
> patch's wording isn't right, please let me know, as I haven't read this
> whole email thread, just the three emails sent directly to me.

Looks good to me.

The whole thread isn't actually any more interesting, it was mainly a
lot of "how did the bits get lost" confusion because it wasn't clear
whether it was some local script or quilt or whatever that lost sight
of the executable bit.

It starts with a report about how the lost bit results in a build error at

   https://lore.kernel.org/lkml/adc1b0b7-f837-fbb4-332b-4ce18fc55709@roeck-us.net/

and there's discussion about how this has happened before and how our
kernel Makefiles generally should try to not execute scripts directly
exactly because our ancient "tarballs and patches" model never
supported the executable bit etc.

So none of it is very relevant, except in the sense that it would be
convenient if diffutils did support the executable bit changes and we
wouldn't have these kinds of things happening every once in a blue
moon.

Again - there are multiple different workarounds, ranging from "we
shouldn't do that then in our makefiles" to "you can use 'git diff'
instead in the quilt scripts".

So not a huge deal - but it would just be nice if diffutils just
supported the extended format.

           Linus

Re: diffutils file mode (was Re: [PATCH 5.15 00/37] 5.15.96-rc2 review)

[Slade Watkins]

On Fri, Feb 24, 2023 at 3:20 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> On 2023-02-24 11:16, Linus Torvalds wrote:
> >   GNU diffutils have never actually grown the
> > ability to generate those extensions
>
> Thanks for pointing this out. I added this to our list of things to do,
> by installing the attached patch to the GNU diffutils TODO file. If this
> patch's wording isn't right, please let me know, as I haven't read this
> whole email thread, just the three emails sent directly to me.

For what it's worth, this looks good to me.

Thank you,
-- Slade