linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
@ 2023-09-11 23:44 Leonardo Bras
  2023-09-12  4:24 ` Randy Dunlap
  2023-09-12  7:53 ` Masahiro Yamada
  0 siblings, 2 replies; 7+ messages in thread
From: Leonardo Bras @ 2023-09-11 23:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Mauro Carvalho Chehab, Randy Dunlap
  Cc: Leonardo Bras, linux-kernel, linux-kbuild

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 <leobras@redhat.com>
---

Please provide feedback on what else to add / remove / reorder here!

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

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

diff --git a/scripts/git.orderFile b/scripts/git.orderFile
new file mode 100644
index 000000000000..819f0a957fe3
--- /dev/null
+++ b/scripts/git.orderFile
@@ -0,0 +1,32 @@
+/* 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
+
+# build system
+Kbuild*
+Kconfig*
+Makefile*
+*.mak
+
+# semantic patches
+*.cocci
+
+# headers
+*.h
+
+# code
+*.c
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
  2023-09-11 23:44 [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile Leonardo Bras
@ 2023-09-12  4:24 ` Randy Dunlap
  2023-09-12 19:29   ` Leonardo Bras
  2023-09-12  7:53 ` Masahiro Yamada
  1 sibling, 1 reply; 7+ messages in thread
From: Randy Dunlap @ 2023-09-12  4:24 UTC (permalink / raw)
  To: Leonardo Bras, Masahiro Yamada, Nathan Chancellor,
	Nick Desaulniers, Nicolas Schier, Mauro Carvalho Chehab
  Cc: linux-kernel, linux-kbuild



On 9/11/23 16:44, Leonardo Bras 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 <leobras@redhat.com>

LGTM. Thanks.

Acked-by: Randy Dunlap <rdunlap@infradead.org>

> ---
> 
> Please provide feedback on what else to add / remove / reorder here!
> 
> Changes since RFCv1:
> - Added Kconfig* (thanks Randy Dunlap!)
> - Changed Kbuild to Kbuild* (improve matching)
> 
>  scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 scripts/git.orderFile
> 
> diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> new file mode 100644
> index 000000000000..819f0a957fe3
> --- /dev/null
> +++ b/scripts/git.orderFile
> @@ -0,0 +1,32 @@
> +/* 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
> +
> +# build system
> +Kbuild*
> +Kconfig*
> +Makefile*
> +*.mak
> +
> +# semantic patches
> +*.cocci
> +
> +# headers
> +*.h
> +
> +# code
> +*.c

-- 
~Randy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
  2023-09-11 23:44 [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile Leonardo Bras
  2023-09-12  4:24 ` Randy Dunlap
@ 2023-09-12  7:53 ` Masahiro Yamada
  2023-09-12 19:51   ` Leonardo Bras
  1 sibling, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2023-09-12  7:53 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Mauro Carvalho Chehab, Randy Dunlap, linux-kernel, linux-kbuild

On Tue, Sep 12, 2023 at 8:45 AM Leonardo Bras <leobras@redhat.com> 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 <leobras@redhat.com>
> ---
>
> Please provide feedback on what else to add / remove / reorder here!
>
> Changes since RFCv1:
> - Added Kconfig* (thanks Randy Dunlap!)
> - Changed Kbuild to Kbuild* (improve matching)
>
>  scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 scripts/git.orderFile
>
> diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> new file mode 100644
> index 000000000000..819f0a957fe3
> --- /dev/null
> +++ b/scripts/git.orderFile
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +


Please use "# SPDX-License-Identifier: GPL-2.0".

/* ... */ is not a valid comment style for the orderfile.




> +# 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


These comments are bogus.


I guess this comment header was copied from QEMU,
but you changed the file path
from scripts/git.orderfile to scripts/git.orderFile.


You need to adjust the comment lines to


    git diff -O scripts/git.orderFile ...

    git config diff.orderFile scripts/git.orderFile



Or, you need to get the file path back to scripts/git.orderfile








> +#
> +
> +MAINTAINERS
> +
> +# Documentation
> +Documentation/*
> +*.rst
> +
> +# build system
> +Kbuild*
> +Kconfig*
> +Makefile*


Kbuild* and Makefile* are interchangeable.
(both are for GNU Make)

Kconfig* are different types.


Better to arrange the order to

Kconfig*
Kbuild*
Makefile*





> +*.mak

QEMU consistently uses only *.mak.

I just realized the kernel tree uses both *.mak and *.mk

masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mak'
./tools/scripts/utilities.mak
masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mk'
./tools/testing/selftests/lib.mk
./tools/testing/selftests/ptp/testptp.mk





BTW, I quickly tested this, but
it did not work as I expected.





masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only d34599b^..d34599b
MAINTAINERS
drivers/Kconfig
drivers/Makefile
drivers/cache/Kconfig
drivers/cache/Makefile
drivers/cache/ax45mp_cache.c

masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
scripts/git.orderFile d34599b^..d34599b
MAINTAINERS
drivers/cache/ax45mp_cache.c
drivers/Kconfig
drivers/Makefile
drivers/cache/Kconfig
drivers/cache/Makefile

masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
scripts/git.orderFile d34599b..d34599b^
MAINTAINERS
drivers/cache/ax45mp_cache.c
drivers/Kconfig
drivers/Makefile
drivers/cache/Kconfig
drivers/cache/Makefile





My expectation was the following:

MAINTAINERS
drivers/Kconfig
drivers/cache/Kconfig
drivers/Makefile
drivers/cache/Makefile
drivers/cache/ax45mp_cache.c


It did not work like that.
Am I missing something?





> +
> +# semantic patches
> +*.cocci
> +
> +# headers
> +*.h
> +
> +# code
> +*.c
> --
> 2.42.0
>





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
  2023-09-12  4:24 ` Randy Dunlap
@ 2023-09-12 19:29   ` Leonardo Bras
  0 siblings, 0 replies; 7+ messages in thread
From: Leonardo Bras @ 2023-09-12 19:29 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Mauro Carvalho Chehab, linux-kernel,
	linux-kbuild

On Mon, Sep 11, 2023 at 09:24:04PM -0700, Randy Dunlap wrote:
> 
> 
> On 9/11/23 16:44, Leonardo Bras 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 <leobras@redhat.com>
> 
> LGTM. Thanks.
> 
> Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks Randy!

> 
> > ---
> > 
> > Please provide feedback on what else to add / remove / reorder here!
> > 
> > Changes since RFCv1:
> > - Added Kconfig* (thanks Randy Dunlap!)
> > - Changed Kbuild to Kbuild* (improve matching)
> > 
> >  scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 scripts/git.orderFile
> > 
> > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > new file mode 100644
> > index 000000000000..819f0a957fe3
> > --- /dev/null
> > +++ b/scripts/git.orderFile
> > @@ -0,0 +1,32 @@
> > +/* 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
> > +
> > +# build system
> > +Kbuild*
> > +Kconfig*
> > +Makefile*
> > +*.mak
> > +
> > +# semantic patches
> > +*.cocci
> > +
> > +# headers
> > +*.h
> > +
> > +# code
> > +*.c
> 
> -- 
> ~Randy
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
  2023-09-12  7:53 ` Masahiro Yamada
@ 2023-09-12 19:51   ` Leonardo Bras
  2023-09-13  7:39     ` Leonardo Bras
  0 siblings, 1 reply; 7+ messages in thread
From: Leonardo Bras @ 2023-09-12 19:51 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Mauro Carvalho Chehab, Randy Dunlap, linux-kernel, linux-kbuild

On Tue, Sep 12, 2023 at 04:53:11PM +0900, Masahiro Yamada wrote:
> On Tue, Sep 12, 2023 at 8:45 AM Leonardo Bras <leobras@redhat.com> 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 <leobras@redhat.com>
> > ---
> >
> > Please provide feedback on what else to add / remove / reorder here!
> >
> > Changes since RFCv1:
> > - Added Kconfig* (thanks Randy Dunlap!)
> > - Changed Kbuild to Kbuild* (improve matching)
> >
> >  scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 scripts/git.orderFile
> >
> > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > new file mode 100644
> > index 000000000000..819f0a957fe3
> > --- /dev/null
> > +++ b/scripts/git.orderFile
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> 
> 
> Please use "# SPDX-License-Identifier: GPL-2.0".
> 
> /* ... */ is not a valid comment style for the orderfile.

Oh, you are right.
My bad, it was a last minute change.

> 
> 
> 
> 
> > +# 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
> 
> 
> These comments are bogus.
> 
> 
> I guess this comment header was copied from QEMU,

Yes, I tried to adapt it from QEMU to kernel needs.


> but you changed the file path
> from scripts/git.orderfile to scripts/git.orderFile.
> 
> 
> You need to adjust the comment lines to
> 
> 
>     git diff -O scripts/git.orderFile ...
> 
>     git config diff.orderFile scripts/git.orderFile
> 
>

Adjusted, thanks!
 
> 
> Or, you need to get the file path back to scripts/git.orderfile
> 
> 
> 
> 
> 
> 
> 
> 
> > +#
> > +
> > +MAINTAINERS
> > +
> > +# Documentation
> > +Documentation/*
> > +*.rst
> > +
> > +# build system
> > +Kbuild*
> > +Kconfig*
> > +Makefile*
> 
> 
> Kbuild* and Makefile* are interchangeable.
> (both are for GNU Make)
> 
> Kconfig* are different types.
> 
> 
> Better to arrange the order to
> 
> Kconfig*
> Kbuild*
> Makefile*
> 
> 

Oh, that makes sense.
Done!

> 
> 
> 
> > +*.mak
> 
> QEMU consistently uses only *.mak.
> 
> I just realized the kernel tree uses both *.mak and *.mk
> 
> masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mak'
> ./tools/scripts/utilities.mak
> masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mk'
> ./tools/testing/selftests/lib.mk
> ./tools/testing/selftests/ptp/testptp.mk

Sure, I will add '*.mk' after '*.mak', getting:

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


> 
> 
> 
> 
> 
> BTW, I quickly tested this, but
> it did not work as I expected.
> 
> 
> 
> 
> 
> masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only d34599b^..d34599b
> MAINTAINERS
> drivers/Kconfig
> drivers/Makefile
> drivers/cache/Kconfig
> drivers/cache/Makefile
> drivers/cache/ax45mp_cache.c
> 
> masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> scripts/git.orderFile d34599b^..d34599b
> MAINTAINERS
> drivers/cache/ax45mp_cache.c
> drivers/Kconfig
> drivers/Makefile
> drivers/cache/Kconfig
> drivers/cache/Makefile
> 
> masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> scripts/git.orderFile d34599b..d34599b^
> MAINTAINERS
> drivers/cache/ax45mp_cache.c
> drivers/Kconfig
> drivers/Makefile
> drivers/cache/Kconfig
> drivers/cache/Makefile
> 
> 
> 
> 
> 
> My expectation was the following:
> 
> MAINTAINERS
> drivers/Kconfig
> drivers/cache/Kconfig
> drivers/Makefile
> drivers/cache/Makefile
> drivers/cache/ax45mp_cache.c
> 
> 
> It did not work like that.
> Am I missing something?

I can reproduce this same behavior for this commit list, and this is odd.

When I added a line-end at the .c extension, it works as expected:

*.c$

I think this makes sense. 
Just to make sure, I will add an line-end at every pattern with extension:

*.h$
*.c$
*.mk$

and so on.
Does that work for you?


I will send a v3 soon.
Thanks!
Leo

> 
> 
> 
> 
> 
> > +
> > +# semantic patches
> > +*.cocci
> > +
> > +# headers
> > +*.h
> > +
> > +# code
> > +*.c
> > --
> > 2.42.0
> >
> 
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
  2023-09-12 19:51   ` Leonardo Bras
@ 2023-09-13  7:39     ` Leonardo Bras
  2023-09-13  7:45       ` Leonardo Bras
  0 siblings, 1 reply; 7+ messages in thread
From: Leonardo Bras @ 2023-09-13  7:39 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Mauro Carvalho Chehab, Randy Dunlap, linux-kernel, linux-kbuild

On Tue, Sep 12, 2023 at 04:51:47PM -0300, Leonardo Bras wrote:
> On Tue, Sep 12, 2023 at 04:53:11PM +0900, Masahiro Yamada wrote:
> > On Tue, Sep 12, 2023 at 8:45 AM Leonardo Bras <leobras@redhat.com> 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 <leobras@redhat.com>
> > > ---
> > >
> > > Please provide feedback on what else to add / remove / reorder here!
> > >
> > > Changes since RFCv1:
> > > - Added Kconfig* (thanks Randy Dunlap!)
> > > - Changed Kbuild to Kbuild* (improve matching)
> > >
> > >  scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >  create mode 100644 scripts/git.orderFile
> > >
> > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > > new file mode 100644
> > > index 000000000000..819f0a957fe3
> > > --- /dev/null
> > > +++ b/scripts/git.orderFile
> > > @@ -0,0 +1,32 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > 
> > 
> > Please use "# SPDX-License-Identifier: GPL-2.0".
> > 
> > /* ... */ is not a valid comment style for the orderfile.
> 
> Oh, you are right.
> My bad, it was a last minute change.
> 
> > 
> > 
> > 
> > 
> > > +# 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
> > 
> > 
> > These comments are bogus.
> > 
> > 
> > I guess this comment header was copied from QEMU,
> 
> Yes, I tried to adapt it from QEMU to kernel needs.
> 
> 
> > but you changed the file path
> > from scripts/git.orderfile to scripts/git.orderFile.
> > 
> > 
> > You need to adjust the comment lines to
> > 
> > 
> >     git diff -O scripts/git.orderFile ...
> > 
> >     git config diff.orderFile scripts/git.orderFile
> > 
> >
> 
> Adjusted, thanks!
>  
> > 
> > Or, you need to get the file path back to scripts/git.orderfile
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > > +#
> > > +
> > > +MAINTAINERS
> > > +
> > > +# Documentation
> > > +Documentation/*
> > > +*.rst
> > > +
> > > +# build system
> > > +Kbuild*
> > > +Kconfig*
> > > +Makefile*
> > 
> > 
> > Kbuild* and Makefile* are interchangeable.
> > (both are for GNU Make)
> > 
> > Kconfig* are different types.
> > 
> > 
> > Better to arrange the order to
> > 
> > Kconfig*
> > Kbuild*
> > Makefile*
> > 
> > 
> 
> Oh, that makes sense.
> Done!
> 
> > 
> > 
> > 
> > > +*.mak
> > 
> > QEMU consistently uses only *.mak.
> > 
> > I just realized the kernel tree uses both *.mak and *.mk
> > 
> > masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mak'
> > ./tools/scripts/utilities.mak
> > masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mk'
> > ./tools/testing/selftests/lib.mk
> > ./tools/testing/selftests/ptp/testptp.mk
> 
> Sure, I will add '*.mk' after '*.mak', getting:
> 
> # build system
> Kconfig*
> Kbuild*
> Makefile*
> *.mak
> *.mk
> 
> 
> > 
> > 
> > 
> > 
> > 
> > BTW, I quickly tested this, but
> > it did not work as I expected.
> > 
> > 
> > 
> > 
> > 
> > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only d34599b^..d34599b
> > MAINTAINERS
> > drivers/Kconfig
> > drivers/Makefile
> > drivers/cache/Kconfig
> > drivers/cache/Makefile
> > drivers/cache/ax45mp_cache.c
> > 
> > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> > scripts/git.orderFile d34599b^..d34599b
> > MAINTAINERS
> > drivers/cache/ax45mp_cache.c
> > drivers/Kconfig
> > drivers/Makefile
> > drivers/cache/Kconfig
> > drivers/cache/Makefile
> > 
> > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> > scripts/git.orderFile d34599b..d34599b^
> > MAINTAINERS
> > drivers/cache/ax45mp_cache.c
> > drivers/Kconfig
> > drivers/Makefile
> > drivers/cache/Kconfig
> > drivers/cache/Makefile
> > 
> > 
> > 
> > 
> > 
> > My expectation was the following:
> > 
> > MAINTAINERS
> > drivers/Kconfig
> > drivers/cache/Kconfig
> > drivers/Makefile
> > drivers/cache/Makefile
> > drivers/cache/ax45mp_cache.c
> > 
> > 
> > It did not work like that.
> > Am I missing something?
> 
> I can reproduce this same behavior for this commit list, and this is odd.
> 
> When I added a line-end at the .c extension, it works as expected:
> 
> *.c$
> 
> I think this makes sense. 
> Just to make sure, I will add an line-end at every pattern with extension:
> 
> *.h$
> *.c$
> *.mk$

Oh, nevermind. This breaks the matching, and results are crazy.
I will revert it on a v4.

The real solver is:
*/Kconfig*
*/Kbuild*
*/Makefile*

The thing is that if I add just "Kconfig*" it only matches a Kconfig* in
the root dir.


> 
> and so on.
> Does that work for you?
> 
> 
> I will send a v3 soon.
> Thanks!
> Leo
> 
> > 
> > 
> > 
> > 
> > 
> > > +
> > > +# semantic patches
> > > +*.cocci
> > > +
> > > +# headers
> > > +*.h

I was talking on a previous thread, and it would probably be interesting
to add "*types.h" before *.h.

I need to think about a way to filter them out when matching "*.h", or it
won't work because of:

Git doc:
"The output order is determined by the order of glob patterns in <orderfile>.
All files with pathnames that match the first pattern are output first, all
files with pathnames that match the second pattern (but not the first) are
output next, and so on."

i.e. the file will be put in the category of the last pattern it matches,
and it makes harder to get "*types.h" before "*.h".

Trying to think on some solution.

> > > +
> > > +# code
> > > +*.c
> > > --
> > > 2.42.0
> > >
> > 
> > 
> > 
> > 
> > 
> > -- 
> > Best Regards
> > Masahiro Yamada
> > 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile
  2023-09-13  7:39     ` Leonardo Bras
@ 2023-09-13  7:45       ` Leonardo Bras
  0 siblings, 0 replies; 7+ messages in thread
From: Leonardo Bras @ 2023-09-13  7:45 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Mauro Carvalho Chehab, Randy Dunlap, linux-kernel, linux-kbuild

On Wed, Sep 13, 2023 at 04:39:08AM -0300, Leonardo Bras wrote:
> On Tue, Sep 12, 2023 at 04:51:47PM -0300, Leonardo Bras wrote:
> > On Tue, Sep 12, 2023 at 04:53:11PM +0900, Masahiro Yamada wrote:
> > > On Tue, Sep 12, 2023 at 8:45 AM Leonardo Bras <leobras@redhat.com> 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 <leobras@redhat.com>
> > > > ---
> > > >
> > > > Please provide feedback on what else to add / remove / reorder here!
> > > >
> > > > Changes since RFCv1:
> > > > - Added Kconfig* (thanks Randy Dunlap!)
> > > > - Changed Kbuild to Kbuild* (improve matching)
> > > >
> > > >  scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
> > > >  1 file changed, 32 insertions(+)
> > > >  create mode 100644 scripts/git.orderFile
> > > >
> > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > > > new file mode 100644
> > > > index 000000000000..819f0a957fe3
> > > > --- /dev/null
> > > > +++ b/scripts/git.orderFile
> > > > @@ -0,0 +1,32 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +
> > > 
> > > 
> > > Please use "# SPDX-License-Identifier: GPL-2.0".
> > > 
> > > /* ... */ is not a valid comment style for the orderfile.
> > 
> > Oh, you are right.
> > My bad, it was a last minute change.
> > 
> > > 
> > > 
> > > 
> > > 
> > > > +# 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
> > > 
> > > 
> > > These comments are bogus.
> > > 
> > > 
> > > I guess this comment header was copied from QEMU,
> > 
> > Yes, I tried to adapt it from QEMU to kernel needs.
> > 
> > 
> > > but you changed the file path
> > > from scripts/git.orderfile to scripts/git.orderFile.
> > > 
> > > 
> > > You need to adjust the comment lines to
> > > 
> > > 
> > >     git diff -O scripts/git.orderFile ...
> > > 
> > >     git config diff.orderFile scripts/git.orderFile
> > > 
> > >
> > 
> > Adjusted, thanks!
> >  
> > > 
> > > Or, you need to get the file path back to scripts/git.orderfile
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > > +#
> > > > +
> > > > +MAINTAINERS
> > > > +
> > > > +# Documentation
> > > > +Documentation/*
> > > > +*.rst
> > > > +
> > > > +# build system
> > > > +Kbuild*
> > > > +Kconfig*
> > > > +Makefile*
> > > 
> > > 
> > > Kbuild* and Makefile* are interchangeable.
> > > (both are for GNU Make)
> > > 
> > > Kconfig* are different types.
> > > 
> > > 
> > > Better to arrange the order to
> > > 
> > > Kconfig*
> > > Kbuild*
> > > Makefile*
> > > 
> > > 
> > 
> > Oh, that makes sense.
> > Done!
> > 
> > > 
> > > 
> > > 
> > > > +*.mak
> > > 
> > > QEMU consistently uses only *.mak.
> > > 
> > > I just realized the kernel tree uses both *.mak and *.mk
> > > 
> > > masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mak'
> > > ./tools/scripts/utilities.mak
> > > masahiro@zoe:~/ref/linux(master)$ find .  -name '*.mk'
> > > ./tools/testing/selftests/lib.mk
> > > ./tools/testing/selftests/ptp/testptp.mk
> > 
> > Sure, I will add '*.mk' after '*.mak', getting:
> > 
> > # build system
> > Kconfig*
> > Kbuild*
> > Makefile*
> > *.mak
> > *.mk
> > 
> > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > BTW, I quickly tested this, but
> > > it did not work as I expected.
> > > 
> > > 
> > > 
> > > 
> > > 
> > > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only d34599b^..d34599b
> > > MAINTAINERS
> > > drivers/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Kconfig
> > > drivers/cache/Makefile
> > > drivers/cache/ax45mp_cache.c
> > > 
> > > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> > > scripts/git.orderFile d34599b^..d34599b
> > > MAINTAINERS
> > > drivers/cache/ax45mp_cache.c
> > > drivers/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Kconfig
> > > drivers/cache/Makefile
> > > 
> > > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> > > scripts/git.orderFile d34599b..d34599b^
> > > MAINTAINERS
> > > drivers/cache/ax45mp_cache.c
> > > drivers/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Kconfig
> > > drivers/cache/Makefile
> > > 
> > > 
> > > 
> > > 
> > > 
> > > My expectation was the following:
> > > 
> > > MAINTAINERS
> > > drivers/Kconfig
> > > drivers/cache/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Makefile
> > > drivers/cache/ax45mp_cache.c
> > > 
> > > 
> > > It did not work like that.
> > > Am I missing something?
> > 
> > I can reproduce this same behavior for this commit list, and this is odd.
> > 
> > When I added a line-end at the .c extension, it works as expected:
> > 
> > *.c$
> > 
> > I think this makes sense. 
> > Just to make sure, I will add an line-end at every pattern with extension:
> > 
> > *.h$
> > *.c$
> > *.mk$
> 
> Oh, nevermind. This breaks the matching, and results are crazy.
> I will revert it on a v4.
> 
> The real solver is:
> */Kconfig*
> */Kbuild*
> */Makefile*

actually, to match root dir:

*Kconfig*
*Kbuild*
*Makefile*

> 
> The thing is that if I add just "Kconfig*" it only matches a Kconfig* in
> the root dir.
> 
> 
> > 
> > and so on.
> > Does that work for you?
> > 
> > 
> > I will send a v3 soon.
> > Thanks!
> > Leo
> > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > > +
> > > > +# semantic patches
> > > > +*.cocci
> > > > +
> > > > +# headers
> > > > +*.h
> 
> I was talking on a previous thread, and it would probably be interesting
> to add "*types.h" before *.h.
> 
> I need to think about a way to filter them out when matching "*.h", or it
> won't work because of:
> 
> Git doc:
> "The output order is determined by the order of glob patterns in <orderfile>.
> All files with pathnames that match the first pattern are output first, all
> files with pathnames that match the second pattern (but not the first) are
> output next, and so on."
> 
> i.e. the file will be put in the category of the last pattern it matches,
> and it makes harder to get "*types.h" before "*.h".
> 
> Trying to think on some solution.

Arg, nevermind. The matching failure caused a lot of confusion in my mind.

It works fine if we do:

*types.h
*.h
*.c

Will send a v4 soon.

> 
> > > > +
> > > > +# code
> > > > +*.c
> > > > --
> > > > 2.42.0
> > > >
> > > 
> > > 
> > > 
> > > 
> > > 
> > > -- 
> > > Best Regards
> > > Masahiro Yamada
> > > 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-09-13  7:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-11 23:44 [RFC PATCH v2 1/1] scripts: Introduce a default git.orderFile Leonardo Bras
2023-09-12  4:24 ` Randy Dunlap
2023-09-12 19:29   ` Leonardo Bras
2023-09-12  7:53 ` Masahiro Yamada
2023-09-12 19:51   ` Leonardo Bras
2023-09-13  7:39     ` Leonardo Bras
2023-09-13  7:45       ` Leonardo Bras

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).