Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check zlib feature error #3

Closed
aguaiyoung opened this issue Jan 26, 2022 · 3 comments
Closed

Check zlib feature error #3

aguaiyoung opened this issue Jan 26, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@aguaiyoung
Copy link

aguaiyoung commented Jan 26, 2022

Makefile.feature may check requirement lib like as zlib,but if clang's version 11.0.0 it's not work well.
some code need escape character like \n you need add like that:

-        $(shell echo $(ZLIB_PROBE) | \
+        $(shell echo -e $(ZLIB_PROBE) | \
       $(CC) $(CFLAGS) -Wall -Werror $(LDFLAGS) -x c - -lz -S -o - >/dev/null 2>&1 \
     && echo 1)

or it output error like that:

<stdin>:1:18: error: extra tokens at end of #include directive [-Werror,-Wextra-tokens]
#include <zlib.h>\n int main(void) {  z_stream zs;      inflateInit(&zs);       return 0; }
                 ^
                 //
1 error generated.

and other echo need echo -e replace

Makefile.include need add EXTRA_WARNINGS in clang version is 11.0.0

      +        -Wno-unused-command-line-argument \
qmonnet added a commit to qmonnet/bpftool that referenced this issue Jan 26, 2022
When detecting the features on the machine, clang does not need the
flags for telling the linker what libraries to use. As a consequence, it
prints a warning to let users know that the flag is unused, and this
turns into an error due to the use of -Wall and -Werror:

  clang: error: -: 'linker' input unused [-Werror,-Wunused-command-line-argument]

To address it, we can temporarily add -Wno-unused-command-line-argument
to the $(CFLAGS) when we check the features with clang.

Fixes: libbpf#3

Reported-by: @aguaiyoung
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet added a commit to qmonnet/bpftool that referenced this issue Jan 26, 2022
Using "echo" in Makefiles, especially when line breaks should be
printed, is not portable. In the case of features detection, the C
snippets contained in Makefile variables must have a line break after
the #include directives, and printing them with "echo" may produce
different outputs depending on the environment.

Instead of echo, we can use printf, which should be more portable.

Fixes: libbpf#3

Reported-by: @aguaiyoung
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
Copy link
Member

qmonnet commented Jan 26, 2022

Hi, thanks a lot for the report!

Both issues are bugs in the feature detection. It looks that clang does not need the flags telling to the linker what libraries to use, and this results in warnings that break detection due to the use of -Werror. Adding Wno-unused-command-line-argument seems to fix it indeed.

I don't directly experience the issue with the echo on my setup, it “works fine for me”. But echo is not known to be portable, and I'm ready to believe that the current code breaks on other environments. I observed that the $(shell echo) invocation calls /bin/sh (dash for me) which is supposed to use /usr/bin/echo, but it seems that in some cases (including the default case for me, where the current code works), some built-in echo is used instead. I think it's some built-in from make, but didn't manage to confirm it. Anyway. Adding -e is not a solution though: it breaks on my setup. I think we'll switch to using printf instead of echo, hopefully it will be more stable.

If you have a moment, could you please check whether the changes in #5 fix the build on your setup?

@qmonnet qmonnet added the bug Something isn't working label Jan 26, 2022
qmonnet added a commit to qmonnet/bpftool that referenced this issue Jan 26, 2022
When detecting the features on the machine, clang does not need the
flags for telling the linker what libraries to use. As a consequence, it
prints a warning to let users know that the flag is unused, and this
turns into an error due to the use of -Wall and -Werror:

  clang: error: -: 'linker' input unused [-Werror,-Wunused-command-line-argument]

To address it, we can temporarily add -Wno-unused-command-line-argument
to the $(CFLAGS) when we check the features with clang.

Fixes: libbpf#3
Fixes: 28b7ccc ("mirror: Reimplement feature detection and reallocarray()")

Reported-by: @aguaiyoung
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet added a commit to qmonnet/bpftool that referenced this issue Jan 26, 2022
Using "echo" in Makefiles, especially when line breaks should be
printed, is not portable. In the case of features detection, the C
snippets contained in Makefile variables must have a line break after
the #include directives, and printing them with "echo" may produce
different outputs depending on the environment.

Instead of echo, we can use printf, which should be more portable.

Fixes: libbpf#3
Fixes: 28b7ccc ("mirror: Reimplement feature detection and reallocarray()")

Reported-by: @aguaiyoung
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@aguaiyoung
Copy link
Author

Hi, thanks a lot for the report!

Both issues are bugs in the feature detection. It looks that clang does not need the flags telling to the linker what libraries to use, and this results in warnings that break detection due to the use of -Werror. Adding Wno-unused-command-line-argument seems to fix it indeed.

I don't directly experience the issue with the echo on my setup, it “works fine for me”. But echo is not known to be portable, and I'm ready to believe that the current code breaks on other environments. I observed that the $(shell echo) invocation calls /bin/sh (dash for me) which is supposed to use /usr/bin/echo, but it seems that in some cases (including the default case for me, where the current code works), some built-in echo is used instead. I think it's some built-in from make, but didn't manage to confirm it. Anyway. Adding -e is not a solution though: it breaks on my setup. I think we'll switch to using printf instead of echo, hopefully it will be more stable.

If you have a moment, could you please check whether the changes in #5 fix the build on your setup?

check in my env, it's ok.
thank your replay.
... libbfd: [ on ]
... disassembler-four-args: [ OFF ]
... zlib: [ on ]
... libcap: [ OFF ]
... clang-bpf-co-re: [ on ]

qmonnet added a commit that referenced this issue Jan 27, 2022
Using "echo" in Makefiles, especially when line breaks should be
printed, is not portable. In the case of features detection, the C
snippets contained in Makefile variables must have a line break after
the #include directives, and printing them with "echo" may produce
different outputs depending on the environment.

Instead of echo, we can use printf, which should be more portable.

Fixes: #3
Fixes: 28b7ccc ("mirror: Reimplement feature detection and reallocarray()")

Reported-by: @aguaiyoung
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet
Copy link
Member

qmonnet commented Jan 27, 2022

And thank you for testing :) Merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants