Re: [PATCH] tools/libbpf: add WERROR option

[Sam James <sam@gentoo.org>]

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 7/5/25 12:43 PM, Sam James wrote:
>> Check the 'WERROR' variable and suppress adding '-Werror' if WERROR=0.
>> This mirrors what tools/perf and other directories in tools do to
>> handle
>> -Werror rather than adding it unconditionally.
>
> Could you also add to the commit desc why you need it? Are there particular
> warnings you specifically need to suppress when building under gentoo?

Sure. In this case, it was https://bugs.gentoo.org/959293 where I think
it's fixed by
https://github.com/libbpf/libbpf/commit/715808d3e2d8c54f3001ce3d7fcda0844f765969
(and the corresponding commit in the kernel tree proper). Backporting
that was a bit too big for our tastes.

The real issue is just that -Werror when we have users who might be
testing with in-development compilers or with alternative options
results in a build failure when you didn't expect one.

>
>> Signed-off-by: Sam James <sam@gentoo.org>
>> ---
>>   tools/lib/bpf/Makefile | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index 168140f8e646..9563d37265da 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -77,10 +77,15 @@ else
>>     CFLAGS := -g -O2
>>   endif
>>   +# Treat warnings as errors unless directed not to
>> +ifneq ($(WERROR),0)
>> +  CFLAGS += -Werror
>> +endif
>
> Should we also add sth similar to tools/bpf/bpftool/Makefile and by default
> enforce with -Werror with the option to disable?

Yes, that sounds good to me, though I was nervous of stumbling onto a
philosophical debate about -Werror and wasn't sure what y'all preferred
:)

I can send v2 with an updated commit message and this change. I'll wait
a bit for further comments based on my two replies here.

>
>>   # Append required CFLAGS
>>   override CFLAGS += -std=gnu89
>>   override CFLAGS += $(EXTRA_WARNINGS) -Wno-switch-enum
>> -override CFLAGS += -Werror -Wall
>> +override CFLAGS += -Wall
>>   override CFLAGS += $(INCLUDES)
>>   override CFLAGS += -fvisibility=hidden
>>   override CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64

sam