2023-12-08 18:19:39

by Leonardo Bras

[permalink] [raw]
Subject: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

When reviewing patches, it looks much nicer to have some changes shown
before others, which allow better understanding of the patch before the
the .c files reviewing.

Introduce a default git.orderFile, in order to help developers getting the
best ordering easier.

Signed-off-by: Leonardo Bras <[email protected]>
Acked-by: Randy Dunlap <[email protected]>

---
Changes since RFCv4:
- Added scripts/* into "build system" section
- Added "git-specific" section with this script and .gitignore
- Thanks for this feedback Nicolas!

Changes since RFCv3:
- Added "*types.h" matching so type headers appear before regular headers
- Removed line ends ($) in patterns: they previously provided a
false-positive
- Fixed build patterns to allow matching Kconfig, Kbuild & Makefile
in any subdirectory

Changes since RFCv2:
- Fixed licence comment to from /**/ to #
- Fixed filename in how-to comment
- Fix build order: Kconfig -> Kbuild -> Makefile
- Add *.mk extension
- Add line-ends ($) to make sure and get the correct extensions
- Thanks Masahiro Yamada for above suggestions!
- 1 Ack, thanks Randy!

Changes since RFCv1:
- Added Kconfig* (thanks Randy Dunlap!)
- Changed Kbuild to Kbuild* (improve matching)


scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 scripts/git.orderFile

diff --git a/scripts/git.orderFile b/scripts/git.orderFile
new file mode 100644
index 0000000000000..31649ff53d22c
--- /dev/null
+++ b/scripts/git.orderFile
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# order file for git, to produce patches which are easier to review
+# by diffing the important stuff like header changes first.
+#
+# one-off usage:
+# git diff -O scripts/git.orderFile ...
+#
+# add to git config:
+# git config diff.orderFile scripts/git.orderFile
+#
+
+MAINTAINERS
+
+# Documentation
+Documentation/*
+*.rst
+
+# git-specific
+.gitignore
+scripts/git.orderFile
+
+# build system
+*Kconfig*
+*Kbuild*
+*Makefile*
+*.mak
+*.mk
+scripts/*
+
+# semantic patches
+*.cocci
+
+# headers
+*types.h
+*.h
+
+# code
+*.c
--
2.43.0


2023-12-09 19:14:44

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <[email protected]> wrote:
>
> When reviewing patches, it looks much nicer to have some changes shown
> before others, which allow better understanding of the patch before the
> the .c files reviewing.
>
> Introduce a default git.orderFile, in order to help developers getting the
> best ordering easier.
>
> Signed-off-by: Leonardo Bras <[email protected]>
> Acked-by: Randy Dunlap <[email protected]>
>
> ---
> Changes since RFCv4:
> - Added scripts/* into "build system" section
> - Added "git-specific" section with this script and .gitignore
> - Thanks for this feedback Nicolas!
>
> Changes since RFCv3:
> - Added "*types.h" matching so type headers appear before regular headers
> - Removed line ends ($) in patterns: they previously provided a
> false-positive
> - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile
> in any subdirectory
>
> Changes since RFCv2:
> - Fixed licence comment to from /**/ to #
> - Fixed filename in how-to comment
> - Fix build order: Kconfig -> Kbuild -> Makefile
> - Add *.mk extension
> - Add line-ends ($) to make sure and get the correct extensions
> - Thanks Masahiro Yamada for above suggestions!
> - 1 Ack, thanks Randy!
>
> Changes since RFCv1:
> - Added Kconfig* (thanks Randy Dunlap!)
> - Changed Kbuild to Kbuild* (improve matching)
>
>
> scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
> create mode 100644 scripts/git.orderFile
>
> diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> new file mode 100644
> index 0000000000000..31649ff53d22c
> --- /dev/null
> +++ b/scripts/git.orderFile
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# order file for git, to produce patches which are easier to review
> +# by diffing the important stuff like header changes first.
> +#
> +# one-off usage:
> +# git diff -O scripts/git.orderFile ...
> +#
> +# add to git config:
> +# git config diff.orderFile scripts/git.orderFile
> +#
> +
> +MAINTAINERS
> +
> +# Documentation
> +Documentation/*
> +*.rst
> +
> +# git-specific
> +.gitignore
> +scripts/git.orderFile



I think scripts/git.orderFile should be part of
"scripts/*" below.






> +
> +# build system
> +*Kconfig*
> +*Kbuild*
> +*Makefile*

I do not like this because "foo-Makefile-bar"
is not a Makefile, but would match "*Makefile*".


If you do not use wildcard at all, 'Makefile'
will match to the root-dir and sub-directories.


Kconfig
*/Kconfig*
Kbuild
Makefile
*.mak
*.mk
scripts/*


may satisfy your needs mostly.




















> +*.mak
> +*.mk
> +scripts/*
> +
> +# semantic patches
> +*.cocci
> +
> +# headers
> +*types.h
> +*.h
> +
> +# code
> +*.c
> --
> 2.43.0
>


--
Best Regards
Masahiro Yamada

2023-12-11 13:14:25

by lsoaresp

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

From: Leonardo Bras <[email protected]>

On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote:
> On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <[email protected]> wrote:
> >
> > When reviewing patches, it looks much nicer to have some changes shown
> > before others, which allow better understanding of the patch before the
> > the .c files reviewing.
> >
> > Introduce a default git.orderFile, in order to help developers getting the
> > best ordering easier.
> >
> > Signed-off-by: Leonardo Bras <[email protected]>
> > Acked-by: Randy Dunlap <[email protected]>
> >
> > ---
> > Changes since RFCv4:
> > - Added scripts/* into "build system" section
> > - Added "git-specific" section with this script and .gitignore
> > - Thanks for this feedback Nicolas!
> >
> > Changes since RFCv3:
> > - Added "*types.h" matching so type headers appear before regular headers
> > - Removed line ends ($) in patterns: they previously provided a
> > false-positive
> > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile
> > in any subdirectory
> >
> > Changes since RFCv2:
> > - Fixed licence comment to from /**/ to #
> > - Fixed filename in how-to comment
> > - Fix build order: Kconfig -> Kbuild -> Makefile
> > - Add *.mk extension
> > - Add line-ends ($) to make sure and get the correct extensions
> > - Thanks Masahiro Yamada for above suggestions!
> > - 1 Ack, thanks Randy!
> >
> > Changes since RFCv1:
> > - Added Kconfig* (thanks Randy Dunlap!)
> > - Changed Kbuild to Kbuild* (improve matching)
> >
> >
> > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> > create mode 100644 scripts/git.orderFile
> >
> > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > new file mode 100644
> > index 0000000000000..31649ff53d22c
> > --- /dev/null
> > +++ b/scripts/git.orderFile
> > @@ -0,0 +1,39 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# order file for git, to produce patches which are easier to review
> > +# by diffing the important stuff like header changes first.
> > +#
> > +# one-off usage:
> > +# git diff -O scripts/git.orderFile ...
> > +#
> > +# add to git config:
> > +# git config diff.orderFile scripts/git.orderFile
> > +#
> > +
> > +MAINTAINERS
> > +
> > +# Documentation
> > +Documentation/*
> > +*.rst
> > +
> > +# git-specific
> > +.gitignore
> > +scripts/git.orderFile
>

Hello Masahiro, thanks for the feedback!

>
>
> I think scripts/git.orderFile should be part of
> "scripts/*" below.
>
>
>
>
>
>
> > +
> > +# build system
> > +*Kconfig*
> > +*Kbuild*
> > +*Makefile*
>
> I do not like this because "foo-Makefile-bar"
> is not a Makefile, but would match "*Makefile*".

That makes sense.

>
>
> If you do not use wildcard at all, 'Makefile'
> will match to the root-dir and sub-directories.

I tried a quick test here changing an mm/*.c file and mm/Makefile, and the
above will print the .c file changes first in any situation here, so it
won't have the desired behavior.

But if we want to achieve the above we can do so with a slight change in
the suggestion:

>
>
> Kconfig
> */Kconfig*
> Kbuild
> Makefile
*/Makefile
> *.mak
> *.mk
> scripts/*
>
>
> may satisfy your needs mostly.
>

I have tried the following in the Kernel root:

$ find . |grep Makefile |grep -v Makefile$
./arch/arm/mach-s3c/Makefile.s3c64xx
./arch/mips/Makefile.postlink
./arch/powerpc/Makefile.postlink
./arch/um/Makefile-os-Linux
./arch/um/Makefile-skas
./arch/um/scripts/Makefile.rules
./arch/x86/Makefile_32.cpu
./arch/x86/Makefile.um
./arch/x86/Makefile.postlink
./arch/riscv/Makefile.postlink
./drivers/firmware/efi/libstub/Makefile.zboot
./drivers/usb/serial/Makefile-keyspan_pda_fw
[...]

$ find . |grep Kbuild |grep -v Kbuild$
./arch/mips/Kbuild.platforms
./scripts/Kbuild.include

Which leads to an honest question:
Don't we want to show changes on those files before C files, for example?

If so, we need something like:

# build system
Kconfig*
*/Kconfig*
Kbuild*
*/Kbuild*
Makefile*
*/Makefile*
*.mak
*.mk
scripts/*

It would get rid of "foo-Makefile-bar" case but still match
"Makefile-bar" case, which seems to be used around.

Is that ok?

Thanks!
Leo

2023-12-11 13:18:30

by Leonardo Bras

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Mon, Dec 11, 2023 at 10:14 AM <[email protected]> wrote:
>
> From: Leonardo Bras <[email protected]>

Sorry about this, there seems to be a bug in my send-email script.
I will get it fixed.

[...]

2023-12-11 18:06:23

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Mon, Dec 11, 2023 at 10:14 PM <[email protected]> wrote:
>
> From: Leonardo Bras <[email protected]>
>
> On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote:
> > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <[email protected]> wrote:
> > >
> > > When reviewing patches, it looks much nicer to have some changes shown
> > > before others, which allow better understanding of the patch before the
> > > the .c files reviewing.
> > >
> > > Introduce a default git.orderFile, in order to help developers getting the
> > > best ordering easier.
> > >
> > > Signed-off-by: Leonardo Bras <[email protected]>
> > > Acked-by: Randy Dunlap <[email protected]>
> > >
> > > ---
> > > Changes since RFCv4:
> > > - Added scripts/* into "build system" section
> > > - Added "git-specific" section with this script and .gitignore
> > > - Thanks for this feedback Nicolas!
> > >
> > > Changes since RFCv3:
> > > - Added "*types.h" matching so type headers appear before regular headers
> > > - Removed line ends ($) in patterns: they previously provided a
> > > false-positive
> > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile
> > > in any subdirectory
> > >
> > > Changes since RFCv2:
> > > - Fixed licence comment to from /**/ to #
> > > - Fixed filename in how-to comment
> > > - Fix build order: Kconfig -> Kbuild -> Makefile
> > > - Add *.mk extension
> > > - Add line-ends ($) to make sure and get the correct extensions
> > > - Thanks Masahiro Yamada for above suggestions!
> > > - 1 Ack, thanks Randy!
> > >
> > > Changes since RFCv1:
> > > - Added Kconfig* (thanks Randy Dunlap!)
> > > - Changed Kbuild to Kbuild* (improve matching)
> > >
> > >
> > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 39 insertions(+)
> > > create mode 100644 scripts/git.orderFile
> > >
> > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > > new file mode 100644
> > > index 0000000000000..31649ff53d22c
> > > --- /dev/null
> > > +++ b/scripts/git.orderFile
> > > @@ -0,0 +1,39 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +# order file for git, to produce patches which are easier to review
> > > +# by diffing the important stuff like header changes first.
> > > +#
> > > +# one-off usage:
> > > +# git diff -O scripts/git.orderFile ...
> > > +#
> > > +# add to git config:
> > > +# git config diff.orderFile scripts/git.orderFile
> > > +#
> > > +
> > > +MAINTAINERS
> > > +
> > > +# Documentation
> > > +Documentation/*
> > > +*.rst
> > > +
> > > +# git-specific
> > > +.gitignore
> > > +scripts/git.orderFile
> >
>
> Hello Masahiro, thanks for the feedback!
>
> >
> >
> > I think scripts/git.orderFile should be part of
> > "scripts/*" below.
> >
> >
> >
> >
> >
> >
> > > +
> > > +# build system
> > > +*Kconfig*
> > > +*Kbuild*
> > > +*Makefile*
> >
> > I do not like this because "foo-Makefile-bar"
> > is not a Makefile, but would match "*Makefile*".
>
> That makes sense.
>
> >
> >
> > If you do not use wildcard at all, 'Makefile'
> > will match to the root-dir and sub-directories.
>
> I tried a quick test here changing an mm/*.c file and mm/Makefile, and the
> above will print the .c file changes first in any situation here, so it
> won't have the desired behavior.



Hmm, you are right.


OK, your suggestion below looks good.


Thanks.






>
> But if we want to achieve the above we can do so with a slight change in
> the suggestion:
>
> >
> >
> > Kconfig
> > */Kconfig*
> > Kbuild
> > Makefile
> */Makefile
> > *.mak
> > *.mk
> > scripts/*
> >
> >
> > may satisfy your needs mostly.
> >
>
> I have tried the following in the Kernel root:
>
> $ find . |grep Makefile |grep -v Makefile$
> ./arch/arm/mach-s3c/Makefile.s3c64xx
> ./arch/mips/Makefile.postlink
> ./arch/powerpc/Makefile.postlink
> ./arch/um/Makefile-os-Linux
> ./arch/um/Makefile-skas
> ./arch/um/scripts/Makefile.rules
> ./arch/x86/Makefile_32.cpu
> ./arch/x86/Makefile.um
> ./arch/x86/Makefile.postlink
> ./arch/riscv/Makefile.postlink
> ./drivers/firmware/efi/libstub/Makefile.zboot
> ./drivers/usb/serial/Makefile-keyspan_pda_fw
> [...]
>
> $ find . |grep Kbuild |grep -v Kbuild$
> ./arch/mips/Kbuild.platforms
> ./scripts/Kbuild.include
>
> Which leads to an honest question:
> Don't we want to show changes on those files before C files, for example?
>
> If so, we need something like:
>
> # build system
> Kconfig*
> */Kconfig*
> Kbuild*
> */Kbuild*
> Makefile*
> */Makefile*
> *.mak
> *.mk
> scripts/*
>
> It would get rid of "foo-Makefile-bar" case but still match
> "Makefile-bar" case, which seems to be used around.
>
> Is that ok?
>
> Thanks!
> Leo
>
>


--
Best Regards
Masahiro Yamada

2023-12-11 22:04:39

by lsoaresp

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

From: Leonardo Bras <[email protected]>

On Tue, Dec 12, 2023 at 03:05:38AM +0900, Masahiro Yamada wrote:
> On Mon, Dec 11, 2023 at 10:14 PM <[email protected]> wrote:
> >
> > From: Leonardo Bras <[email protected]>
> >
> > On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote:
> > > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <[email protected]> wrote:
> > > >
> > > > When reviewing patches, it looks much nicer to have some changes shown
> > > > before others, which allow better understanding of the patch before the
> > > > the .c files reviewing.
> > > >
> > > > Introduce a default git.orderFile, in order to help developers getting the
> > > > best ordering easier.
> > > >
> > > > Signed-off-by: Leonardo Bras <[email protected]>
> > > > Acked-by: Randy Dunlap <[email protected]>
> > > >
> > > > ---
> > > > Changes since RFCv4:
> > > > - Added scripts/* into "build system" section
> > > > - Added "git-specific" section with this script and .gitignore
> > > > - Thanks for this feedback Nicolas!
> > > >
> > > > Changes since RFCv3:
> > > > - Added "*types.h" matching so type headers appear before regular headers
> > > > - Removed line ends ($) in patterns: they previously provided a
> > > > false-positive
> > > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile
> > > > in any subdirectory
> > > >
> > > > Changes since RFCv2:
> > > > - Fixed licence comment to from /**/ to #
> > > > - Fixed filename in how-to comment
> > > > - Fix build order: Kconfig -> Kbuild -> Makefile
> > > > - Add *.mk extension
> > > > - Add line-ends ($) to make sure and get the correct extensions
> > > > - Thanks Masahiro Yamada for above suggestions!
> > > > - 1 Ack, thanks Randy!
> > > >
> > > > Changes since RFCv1:
> > > > - Added Kconfig* (thanks Randy Dunlap!)
> > > > - Changed Kbuild to Kbuild* (improve matching)
> > > >
> > > >
> > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 39 insertions(+)
> > > > create mode 100644 scripts/git.orderFile
> > > >
> > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > > > new file mode 100644
> > > > index 0000000000000..31649ff53d22c
> > > > --- /dev/null
> > > > +++ b/scripts/git.orderFile
> > > > @@ -0,0 +1,39 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +# order file for git, to produce patches which are easier to review
> > > > +# by diffing the important stuff like header changes first.
> > > > +#
> > > > +# one-off usage:
> > > > +# git diff -O scripts/git.orderFile ...
> > > > +#
> > > > +# add to git config:
> > > > +# git config diff.orderFile scripts/git.orderFile
> > > > +#
> > > > +
> > > > +MAINTAINERS
> > > > +
> > > > +# Documentation
> > > > +Documentation/*
> > > > +*.rst
> > > > +
> > > > +# git-specific
> > > > +.gitignore
> > > > +scripts/git.orderFile
> > >
> >
> > Hello Masahiro, thanks for the feedback!
> >
> > >
> > >
> > > I think scripts/git.orderFile should be part of
> > > "scripts/*" below.
> > >
> > >
> > >
> > >
> > >
> > >
> > > > +
> > > > +# build system
> > > > +*Kconfig*
> > > > +*Kbuild*
> > > > +*Makefile*
> > >
> > > I do not like this because "foo-Makefile-bar"
> > > is not a Makefile, but would match "*Makefile*".
> >
> > That makes sense.
> >
> > >
> > >
> > > If you do not use wildcard at all, 'Makefile'
> > > will match to the root-dir and sub-directories.
> >
> > I tried a quick test here changing an mm/*.c file and mm/Makefile, and the
> > above will print the .c file changes first in any situation here, so it
> > won't have the desired behavior.
>
>
>
> Hmm, you are right.
>
>
> OK, your suggestion below looks good.
>
>
> Thanks.

Thank you for this feedback!

I will send a v6 shortly.

Thank you!
Leo

>
>
>
>
>
>
> >
> > But if we want to achieve the above we can do so with a slight change in
> > the suggestion:
> >
> > >
> >>
> > > Kconfig
> > > */Kconfig*
> > > Kbuild
> > > Makefile
> > */Makefile
> > > *.mak
> > > *.mk
> > > scripts/*
> > >
> > >
> > > may satisfy your needs mostly.
> > >
> >
> > I have tried the following in the Kernel root:
> >
> > $ find . |grep Makefile |grep -v Makefile$
> > ./arch/arm/mach-s3c/Makefile.s3c64xx
> > ./arch/mips/Makefile.postlink
> > ./arch/powerpc/Makefile.postlink
> > ./arch/um/Makefile-os-Linux
> > ./arch/um/Makefile-skas
> > ./arch/um/scripts/Makefile.rules
> > ./arch/x86/Makefile_32.cpu
> > ./arch/x86/Makefile.um
> > ./arch/x86/Makefile.postlink
> > ./arch/riscv/Makefile.postlink
> > ./drivers/firmware/efi/libstub/Makefile.zboot
> > ./drivers/usb/serial/Makefile-keyspan_pda_fw
> > [...]
> >
> > $ find . |grep Kbuild |grep -v Kbuild$
> > ./arch/mips/Kbuild.platforms
> > ./scripts/Kbuild.include
> >
> > Which leads to an honest question:
> > Don't we want to show changes on those files before C files, for example?
> >
> > If so, we need something like:
> >
> > # build system
> > Kconfig*
> > */Kconfig*
> > Kbuild*
> > */Kbuild*
> > Makefile*
> > */Makefile*
> > *.mak
> > *.mk
> > scripts/*
> >
> > It would get rid of "foo-Makefile-bar" case but still match
> > "Makefile-bar" case, which seems to be used around.
> >
> > Is that ok?
> >
> > Thanks!
> > Leo
> >
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>

2023-12-11 22:41:37

by Leonardo Bras

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

From: Leonardo Bras <[email protected]>

On Mon, Dec 11, 2023 at 07:03:50PM -0300, [email protected] wrote:
> From: Leonardo Bras <[email protected]>
>
> On Tue, Dec 12, 2023 at 03:05:38AM +0900, Masahiro Yamada wrote:
> > On Mon, Dec 11, 2023 at 10:14 PM <[email protected]> wrote:
> > >
> > > From: Leonardo Bras <[email protected]>
> > >
> > > On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote:
> > > > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <[email protected]> wrote:
> > > > >
> > > > > When reviewing patches, it looks much nicer to have some changes shown
> > > > > before others, which allow better understanding of the patch before the
> > > > > the .c files reviewing.
> > > > >
> > > > > Introduce a default git.orderFile, in order to help developers getting the
> > > > > best ordering easier.
> > > > >
> > > > > Signed-off-by: Leonardo Bras <[email protected]>
> > > > > Acked-by: Randy Dunlap <[email protected]>
> > > > >
> > > > > ---
> > > > > Changes since RFCv4:
> > > > > - Added scripts/* into "build system" section
> > > > > - Added "git-specific" section with this script and .gitignore
> > > > > - Thanks for this feedback Nicolas!
> > > > >
> > > > > Changes since RFCv3:
> > > > > - Added "*types.h" matching so type headers appear before regular headers
> > > > > - Removed line ends ($) in patterns: they previously provided a
> > > > > false-positive
> > > > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile
> > > > > in any subdirectory
> > > > >
> > > > > Changes since RFCv2:
> > > > > - Fixed licence comment to from /**/ to #
> > > > > - Fixed filename in how-to comment
> > > > > - Fix build order: Kconfig -> Kbuild -> Makefile
> > > > > - Add *.mk extension
> > > > > - Add line-ends ($) to make sure and get the correct extensions
> > > > > - Thanks Masahiro Yamada for above suggestions!
> > > > > - 1 Ack, thanks Randy!
> > > > >
> > > > > Changes since RFCv1:
> > > > > - Added Kconfig* (thanks Randy Dunlap!)
> > > > > - Changed Kbuild to Kbuild* (improve matching)
> > > > >
> > > > >
> > > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 39 insertions(+)
> > > > > create mode 100644 scripts/git.orderFile
> > > > >
> > > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > > > > new file mode 100644
> > > > > index 0000000000000..31649ff53d22c
> > > > > --- /dev/null
> > > > > +++ b/scripts/git.orderFile
> > > > > @@ -0,0 +1,39 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +
> > > > > +# order file for git, to produce patches which are easier to review
> > > > > +# by diffing the important stuff like header changes first.
> > > > > +#
> > > > > +# one-off usage:
> > > > > +# git diff -O scripts/git.orderFile ...
> > > > > +#
> > > > > +# add to git config:
> > > > > +# git config diff.orderFile scripts/git.orderFile
> > > > > +#
> > > > > +
> > > > > +MAINTAINERS
> > > > > +
> > > > > +# Documentation
> > > > > +Documentation/*
> > > > > +*.rst
> > > > > +
> > > > > +# git-specific
> > > > > +.gitignore
> > > > > +scripts/git.orderFile
> > > >
> > >
> > > Hello Masahiro, thanks for the feedback!
> > >
> > > >
> > > >
> > > > I think scripts/git.orderFile should be part of
> > > > "scripts/*" below.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > +
> > > > > +# build system
> > > > > +*Kconfig*
> > > > > +*Kbuild*
> > > > > +*Makefile*
> > > >
> > > > I do not like this because "foo-Makefile-bar"
> > > > is not a Makefile, but would match "*Makefile*".
> > >
> > > That makes sense.
> > >
> > > >
> > > >
> > > > If you do not use wildcard at all, 'Makefile'
> > > > will match to the root-dir and sub-directories.
> > >
> > > I tried a quick test here changing an mm/*.c file and mm/Makefile, and the
> > > above will print the .c file changes first in any situation here, so it
> > > won't have the desired behavior.
> >
> >
> >
> > Hmm, you are right.
> >
> >
> > OK, your suggestion below looks good.
> >
> >
> > Thanks.
>
> Thank you for this feedback!
>
> I will send a v6 shortly.
>
> Thank you!
> Leo
>
> >
> >
> >
> >
> >
> >
> > >
> > > But if we want to achieve the above we can do so with a slight change in
> > > the suggestion:
> > >
> > > >
> > >>
> > > > Kconfig
> > > > */Kconfig*
> > > > Kbuild
> > > > Makefile
> > > */Makefile
> > > > *.mak
> > > > *.mk
> > > > scripts/*
> > > >
> > > >
> > > > may satisfy your needs mostly.
> > > >
> > >
> > > I have tried the following in the Kernel root:
> > >
> > > $ find . |grep Makefile |grep -v Makefile$
> > > ./arch/arm/mach-s3c/Makefile.s3c64xx
> > > ./arch/mips/Makefile.postlink
> > > ./arch/powerpc/Makefile.postlink
> > > ./arch/um/Makefile-os-Linux
> > > ./arch/um/Makefile-skas
> > > ./arch/um/scripts/Makefile.rules
> > > ./arch/x86/Makefile_32.cpu
> > > ./arch/x86/Makefile.um
> > > ./arch/x86/Makefile.postlink
> > > ./arch/riscv/Makefile.postlink
> > > ./drivers/firmware/efi/libstub/Makefile.zboot
> > > ./drivers/usb/serial/Makefile-keyspan_pda_fw
> > > [...]
> > >
> > > $ find . |grep Kbuild |grep -v Kbuild$
> > > ./arch/mips/Kbuild.platforms
> > > ./scripts/Kbuild.include
> > >
> > > Which leads to an honest question:
> > > Don't we want to show changes on those files before C files, for example?
> > >
> > > If so, we need something like:
> > >
> > > # build system
> > > Kconfig*
> > > */Kconfig*
> > > Kbuild*
> > > */Kbuild*
> > > Makefile*
> > > */Makefile*
> > > *.mak
> > > *.mk
> > > scripts/*
> > >
> > > It would get rid of "foo-Makefile-bar" case but still match
> > > "Makefile-bar" case, which seems to be used around.
> > >
> > > Is that ok?
> > >
> > > Thanks!
> > > Leo
> > >
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >


RFCv6 patch at:
https://lore.kernel.org/all/[email protected]/

2023-12-12 07:32:15

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

Urrg, hell no. Alphabetic order is the only one allowing sensible
searching. If you have a different preference use your local .gitconfig
instead of enforcing completely random preference on others.

2023-12-12 08:10:12

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Tue, Dec 12, 2023 at 4:32 PM Christoph Hellwig <[email protected]> wrote:
>
> Urrg, hell no. Alphabetic order is the only one allowing sensible
> searching. If you have a different preference use your local .gitconfig
> instead of enforcing completely random preference on others.



Unlike .gitignore, this feature is opt-in rather than enforced.

To use this, you need to run

'git config diff.orderFile scripts/git.orderFile'

or

'git diff -C scripts/git.orderFile'



Indeed, the file order is subjective, leaving
us a question "do we need it in upstream"?

At least, it is harmless for people who have no interest.


--
Best Regards
Masahiro Yamada

2023-12-12 13:09:00

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote:
> Unlike .gitignore, this feature is opt-in rather than enforced.
>
> To use this, you need to run
>
> 'git config diff.orderFile scripts/git.orderFile'
>
> or
>
> 'git diff -C scripts/git.orderFile'

Oh, ok. That greatly reduces my concern.

>
> Indeed, the file order is subjective, leaving
> us a question "do we need it in upstream"?
>
> At least, it is harmless for people who have no interest.

.. but this is still a good question. I'm not really sure there is
much of a need for it, but as long as it doesn't harm everyone else
I'm at least neutral on it.

2023-12-12 17:10:36

by Leonardo Bras

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Tue, Dec 12, 2023 at 05:08:34AM -0800, Christoph Hellwig wrote:
> On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote:
> > Unlike .gitignore, this feature is opt-in rather than enforced.
> >
> > To use this, you need to run
> >
> > 'git config diff.orderFile scripts/git.orderFile'
> >
> > or
> >
> > 'git diff -C scripts/git.orderFile'
>
> Oh, ok. That greatly reduces my concern.

Yes, it's an opt-in, so no user should be directly impacted.

>
> >
> > Indeed, the file order is subjective, leaving
> > us a question "do we need it in upstream"?

The main idea is patch generation.
This file's order is supposed to be the best order for reading a raw patch
and understanding the code changes.

> >
> > At least, it is harmless for people who have no interest.
>
> .. but this is still a good question. I'm not really sure there is
> much of a need for it, but as long as it doesn't harm everyone else
> I'm at least neutral on it.

diff.orderfile was introduced in git to help order the git diff, and thus
the patch generation, in a way that it's easier to understand what the
commit / patch intends on doing.

Take this example introducing a feature foo, you should see:
- Documentation on foo, if introduced
- How is foo enabled in build system, if needed
- The types / stucts / fields introduced by foo, if any
- The interface for using foo, if any
- The actual foo implementation.

Of course the actual order is open to discussion, and I encourage everyone
to suggest any other items or order.

Thanks!
Leo

2023-12-15 17:03:21

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Wed, Dec 13, 2023 at 2:10 AM Leonardo Bras <[email protected]> wrote:
>
> On Tue, Dec 12, 2023 at 05:08:34AM -0800, Christoph Hellwig wrote:
> > On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote:
> > > Unlike .gitignore, this feature is opt-in rather than enforced.
> > >
> > > To use this, you need to run
> > >
> > > 'git config diff.orderFile scripts/git.orderFile'
> > >
> > > or
> > >
> > > 'git diff -C scripts/git.orderFile'
> >
> > Oh, ok. That greatly reduces my concern.
>
> Yes, it's an opt-in, so no user should be directly impacted.


Applied to linux-kbuild.
Thanks.








> >
> > >
> > > Indeed, the file order is subjective, leaving
> > > us a question "do we need it in upstream"?
>
> The main idea is patch generation.
> This file's order is supposed to be the best order for reading a raw patch
> and understanding the code changes.
>
> > >
> > > At least, it is harmless for people who have no interest.
> >
> > .. but this is still a good question. I'm not really sure there is
> > much of a need for it, but as long as it doesn't harm everyone else
> > I'm at least neutral on it.
>
> diff.orderfile was introduced in git to help order the git diff, and thus
> the patch generation, in a way that it's easier to understand what the
> commit / patch intends on doing.
>
> Take this example introducing a feature foo, you should see:
> - Documentation on foo, if introduced
> - How is foo enabled in build system, if needed
> - The types / stucts / fields introduced by foo, if any
> - The interface for using foo, if any
> - The actual foo implementation.
>
> Of course the actual order is open to discussion, and I encourage everyone
> to suggest any other items or order.
>
> Thanks!
> Leo
>
>


--
Best Regards
Masahiro Yamada

2023-12-15 18:30:35

by Leonardo Bras

[permalink] [raw]
Subject: Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile

On Fri, Dec 15, 2023 at 2:03 PM Masahiro Yamada <[email protected]> wrote:
>
> On Wed, Dec 13, 2023 at 2:10 AM Leonardo Bras <[email protected]> wrote:
> >
> > On Tue, Dec 12, 2023 at 05:08:34AM -0800, Christoph Hellwig wrote:
> > > On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote:
> > > > Unlike .gitignore, this feature is opt-in rather than enforced.
> > > >
> > > > To use this, you need to run
> > > >
> > > > 'git config diff.orderFile scripts/git.orderFile'
> > > >
> > > > or
> > > >
> > > > 'git diff -C scripts/git.orderFile'
> > >
> > > Oh, ok. That greatly reduces my concern.
> >
> > Yes, it's an opt-in, so no user should be directly impacted.
>
>
> Applied to linux-kbuild.
> Thanks.

Thank you!



>
>
>
>
>
>
> > >
> > > >
> > > > Indeed, the file order is subjective, leaving
> > > > us a question "do we need it in upstream"?
> >
> > The main idea is patch generation.
> > This file's order is supposed to be the best order for reading a raw patch
> > and understanding the code changes.
> >
> > > >
> > > > At least, it is harmless for people who have no interest.
> > >
> > > .. but this is still a good question. I'm not really sure there is
> > > much of a need for it, but as long as it doesn't harm everyone else
> > > I'm at least neutral on it.
> >
> > diff.orderfile was introduced in git to help order the git diff, and thus
> > the patch generation, in a way that it's easier to understand what the
> > commit / patch intends on doing.
> >
> > Take this example introducing a feature foo, you should see:
> > - Documentation on foo, if introduced
> > - How is foo enabled in build system, if needed
> > - The types / stucts / fields introduced by foo, if any
> > - The interface for using foo, if any
> > - The actual foo implementation.
> >
> > Of course the actual order is open to discussion, and I encourage everyone
> > to suggest any other items or order.
> >
> > Thanks!
> > Leo
> >
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>