2020-09-04 20:27:30

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH] tools feature: Add missing -lzstd to the fast path feature detection

We were failing that due to GTK2+ and then for the ZSTD test, which made
test-all.c, the fast path feature detection file to fail and thus
trigger building all of the feature tests, slowing down the test.

Eventually the ZSTD test would be built and would succeed, since it had
the needed -lzstd, avoiding:

$ cat /tmp/build/perf/feature/test-all.make.output
/usr/bin/ld: /tmp/ccRRJQ4u.o: in function `main_test_libzstd':
/home/acme/git/perf/tools/build/feature/test-libzstd.c:8: undefined reference to `ZSTD_createCStream'
/usr/bin/ld: /home/acme/git/perf/tools/build/feature/test-libzstd.c:9: undefined reference to `ZSTD_freeCStream'
collect2: error: ld returned 1 exit status
$

Fix it by adding -lzstd to the test-all target.

Now I need an entry to 'perf test' to make sure that
/tmp/build/perf/feature/test-all.make.output is empty...

Fixes: 3b1c5d9659718263 ("tools build: Implement libzstd feature check, LIBZSTD_DIR and NO_LIBZSTD defines")
Cc: Adrian Hunter <[email protected]>
Cc: Alexey Budankov <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

---

diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 977067e34dff064d..ec815ffc7777a02b 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -91,7 +91,7 @@ __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(
###############################

$(OUTPUT)test-all.bin:
- $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -I/usr/include/slang -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma
+ $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -I/usr/include/slang -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma -lzstd

$(OUTPUT)test-hello.bin:
$(BUILD)


2020-09-05 03:29:15

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] tools feature: Add missing -lzstd to the fast path feature detection

On Sat, Sep 5, 2020 at 5:26 AM Arnaldo Carvalho de Melo <[email protected]> wrote:
>
> We were failing that due to GTK2+ and then for the ZSTD test, which made
> test-all.c, the fast path feature detection file to fail and thus
> trigger building all of the feature tests, slowing down the test.
>
> Eventually the ZSTD test would be built and would succeed, since it had
> the needed -lzstd, avoiding:
>
> $ cat /tmp/build/perf/feature/test-all.make.output
> /usr/bin/ld: /tmp/ccRRJQ4u.o: in function `main_test_libzstd':
> /home/acme/git/perf/tools/build/feature/test-libzstd.c:8: undefined reference to `ZSTD_createCStream'
> /usr/bin/ld: /home/acme/git/perf/tools/build/feature/test-libzstd.c:9: undefined reference to `ZSTD_freeCStream'
> collect2: error: ld returned 1 exit status
> $
>
> Fix it by adding -lzstd to the test-all target.
>
> Now I need an entry to 'perf test' to make sure that
> /tmp/build/perf/feature/test-all.make.output is empty...
>
> Fixes: 3b1c5d9659718263 ("tools build: Implement libzstd feature check, LIBZSTD_DIR and NO_LIBZSTD defines")
> Cc: Adrian Hunter <[email protected]>
> Cc: Alexey Budankov <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

Acked-by: Namhyung Kim <[email protected]>

Thanks
Namhyung

>
> ---
>
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 977067e34dff064d..ec815ffc7777a02b 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -91,7 +91,7 @@ __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(
> ###############################
>
> $(OUTPUT)test-all.bin:
> - $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -I/usr/include/slang -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma
> + $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -I/usr/include/slang -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma -lzstd
>
> $(OUTPUT)test-hello.bin:
> $(BUILD)

2020-09-05 10:45:25

by Alexey Budankov

[permalink] [raw]
Subject: Re: [PATCH] tools feature: Add missing -lzstd to the fast path feature detection


On 04.09.2020 23:26, Arnaldo Carvalho de Melo wrote:
> We were failing that due to GTK2+ and then for the ZSTD test, which made
> test-all.c, the fast path feature detection file to fail and thus
> trigger building all of the feature tests, slowing down the test.
>
> Eventually the ZSTD test would be built and would succeed, since it had
> the needed -lzstd, avoiding:
>
> $ cat /tmp/build/perf/feature/test-all.make.output
> /usr/bin/ld: /tmp/ccRRJQ4u.o: in function `main_test_libzstd':
> /home/acme/git/perf/tools/build/feature/test-libzstd.c:8: undefined reference to `ZSTD_createCStream'
> /usr/bin/ld: /home/acme/git/perf/tools/build/feature/test-libzstd.c:9: undefined reference to `ZSTD_freeCStream'
> collect2: error: ld returned 1 exit status
> $
>
> Fix it by adding -lzstd to the test-all target.
>
> Now I need an entry to 'perf test' to make sure that
> /tmp/build/perf/feature/test-all.make.output is empty...
>
> Fixes: 3b1c5d9659718263 ("tools build: Implement libzstd feature check, LIBZSTD_DIR and NO_LIBZSTD defines")
> Cc: Adrian Hunter <[email protected]>
> Cc: Alexey Budankov <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ---
>
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 977067e34dff064d..ec815ffc7777a02b 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -91,7 +91,7 @@ __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(
> ###############################
>
> $(OUTPUT)test-all.bin:
> - $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -I/usr/include/slang -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma
> + $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -I/usr/include/slang -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma -lzstd
>
> $(OUTPUT)test-hello.bin:
> $(BUILD)
>

Reviewed-by: Alexei Budankov <[email protected]>

Thanks!
Alexei