2023-01-18 04:13:58

by Thomas Weißschuh

Subject: [PATCH 1/3] selftests/bpf: align kbuild messages to standard

The common layout for kbuild messages is as follows:

- 2 spaces
- 7 or more characters for the action
- 1 space
- name of the file being built/generated

The custom message formatting included an additional space in the action
part, which leads to misalignments with the rest of kbuild.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/bpf/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c22c43bbee19..5190c19295d4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -98,7 +98,7 @@ Q =
msg =
else
Q = @
-msg = @printf ' %-8s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
+msg = @printf ' %-7s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
MAKEFLAGS += --no-print-directory
submake_extras := feature_display=0
endif

--
2.39.1

2023-01-18 05:16:53

by Yonghong Song

Subject: Re: [PATCH 1/3] selftests/bpf: align kbuild messages to standard

On 1/17/23 7:52 PM, Thomas Weißschuh wrote:
> The common layout for kbuild messages is as follows:
>
> - 2 spaces
> - 7 or more characters for the action
> - 1 space
> - name of the file being built/generated
>
> The custom message formatting included an additional space in the action
> part, which leads to misalignments with the rest of kbuild.

Could you give an example to show the output before/after the patch, and
how it leads to mis-alignment and why it is a problem?

>
> Signed-off-by: Thomas Weißschuh <[email protected]>
> ---
> tools/testing/selftests/bpf/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index c22c43bbee19..5190c19295d4 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -98,7 +98,7 @@ Q =
> msg =
> else
> Q = @
> -msg = @printf ' %-8s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
> +msg = @printf ' %-7s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
> MAKEFLAGS += --no-print-directory
> submake_extras := feature_display=0
> endif
>

2023-01-18 05:41:19

by Thomas Weißschuh

Subject: Re: [PATCH 1/3] selftests/bpf: align kbuild messages to standard

On Tue, Jan 17, 2023 at 09:02:20PM -0800, Yonghong Song wrote:
>
>
> On 1/17/23 7:52 PM, Thomas Wei?schuh wrote:
> > The common layout for kbuild messages is as follows:
> >
> > - 2 spaces
> > - 7 or more characters for the action
> > - 1 space
> > - name of the file being built/generated
> >
> > The custom message formatting included an additional space in the action
> > part, which leads to misalignments with the rest of kbuild.
>
> Could you give an example to show the output before/after the patch, and
> how it leads to mis-alignment and why it is a problem?

Before:

LD .../linux/tools/bpf/resolve_btfids/resolve_btfids-in.o
LINK resolve_btfids
CHK kernel/kheaders_data.tar.xz

After:

LD .../linux/tools/bpf/resolve_btfids/resolve_btfids-in.o
LINK resolve_btfids
CHK kernel/kheaders_data.tar.xz

The line starting with "LINK" has the filename "resolve_btfids" one
space character more to the right than the other lines.

It's slightly confusing when scanning the build logs.

> > Signed-off-by: Thomas Wei?schuh <[email protected]>
> > ---
> > tools/testing/selftests/bpf/Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index c22c43bbee19..5190c19295d4 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -98,7 +98,7 @@ Q =
> > msg =
> > else
> > Q = @
> > -msg = @printf ' %-8s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
> > +msg = @printf ' %-7s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
> > MAKEFLAGS += --no-print-directory
> > submake_extras := feature_display=0
> > endif
> >

2023-01-18 07:47:22

by Yonghong Song

Subject: Re: [PATCH 1/3] selftests/bpf: align kbuild messages to standard

On 1/17/23 9:15 PM, Thomas Weißschuh wrote:
> On Tue, Jan 17, 2023 at 09:02:20PM -0800, Yonghong Song wrote:
>>
>>
>> On 1/17/23 7:52 PM, Thomas Weißschuh wrote:
>>> The common layout for kbuild messages is as follows:
>>>
>>> - 2 spaces
>>> - 7 or more characters for the action
>>> - 1 space
>>> - name of the file being built/generated
>>>
>>> The custom message formatting included an additional space in the action
>>> part, which leads to misalignments with the rest of kbuild.
>>
>> Could you give an example to show the output before/after the patch, and
>> how it leads to mis-alignment and why it is a problem?
>
> Before:
>
> LD .../linux/tools/bpf/resolve_btfids/resolve_btfids-in.o
> LINK resolve_btfids
> CHK kernel/kheaders_data.tar.xz
>
> After:
>
> LD .../linux/tools/bpf/resolve_btfids/resolve_btfids-in.o
> LINK resolve_btfids
> CHK kernel/kheaders_data.tar.xz
>
> The line starting with "LINK" has the filename "resolve_btfids" one
> space character more to the right than the other lines.

Thanks! I would be great if you can put the details about
(1) what are the command line to reproduce the issue, and
(2) what the output differences,
to the commit message in all three patches.

>
> It's slightly confusing when scanning the build logs.
>
>>> Signed-off-by: Thomas Weißschuh <[email protected]>
>>> ---
>>> tools/testing/selftests/bpf/Makefile | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>>> index c22c43bbee19..5190c19295d4 100644
>>> --- a/tools/testing/selftests/bpf/Makefile
>>> +++ b/tools/testing/selftests/bpf/Makefile
>>> @@ -98,7 +98,7 @@ Q =
>>> msg =
>>> else
>>> Q = @
>>> -msg = @printf ' %-8s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
>>> +msg = @printf ' %-7s%s %s%s\n' "$(1)" "$(if $(2), [$(2)])" "$(notdir $(3))" "$(if $(4), $(4))";
>>> MAKEFLAGS += --no-print-directory
>>> submake_extras := feature_display=0
>>> endif
>>>