Thanks Jiri ! A few comments inline.
2019-12-02 14:18 UTC+0100 ~ Jiri Olsa <jolsa@xxxxxxxxxx>
> Adding new test to test_bpftool_build.sh script to
> test the dynamic linkage of libbpf for bpftool:
>
> $ ./test_bpftool_build.sh
> [SNIP]
>
> ... with dynamic libbpf
>
> $PWD: /home/jolsa/kernel/linux-perf/tools/bpf/bpftool
> command: make -s -C ../../build/feature clean >/dev/null
> command: make -s -C ../../lib/bpf clean >/dev/null
> command: make -s -C ../../lib/bpf prefix=/tmp/tmp.fG8O2Ps8ER install_lib install_headers >/dev/null
> Parsed description of 117 helper function(s)
> command: make -s clean >/dev/null
> command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/tmp.fG8O2Ps8ER >/dev/null
> binary: /home/jolsa/kernel/linux-perf/tools/bpf/bpftool/bpftool
> binary: linked with libbpf
>
> The test installs libbpf into temp directory
> and links bpftool dynamically with it.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> .../selftests/bpf/test_bpftool_build.sh | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
> index ac349a5cea7e..e4a6a0520f8e 100755
> --- a/tools/testing/selftests/bpf/test_bpftool_build.sh
> +++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
> @@ -85,6 +85,55 @@ make_with_tmpdir() {
> echo
> }
>
> +# Assumes current directory is tools/bpf/bpftool
> +make_with_dynamic_libbpf() {
> + TMPDIR=$(mktemp -d)
> + echo -e "\$PWD: $PWD"
> +
> + # It might be needed to clean build tree first because features
> + # framework does not detect the change properly
> + echo -e "command: make -s -C ../../build/feature clean >/dev/null"
(So far I did not echo the "make clean" commands, I printed only the
ones used to build bpftool. But that's your call.)
> + make $J -s -C ../../build/feature clean >/dev/null
> + if [ $? -ne 0 ] ; then
> + ERROR=1
> + fi
> + echo -e "command: make -s -C ../../lib/bpf clean >/dev/null"
> + make $J -s -C ../../lib/bpf clean >/dev/null
> + if [ $? -ne 0 ] ; then
> + ERROR=1
> + fi
> +
> + # Now install libbpf into TMPDIR
> + echo -e "command: make -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null"
> + make $J -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null
> + if [ $? -ne 0 ] ; then
> + ERROR=1
> + fi
> +
> + # And final bpftool build (with clean first) with libbpf dynamic link
> + echo -e "command: make -s clean >/dev/null"
> + if [ $? -ne 0 ] ; then
> + ERROR=1
> + fi
I do not believe you need to "make clean" here, this should have been
done by the previous test in that dir earlier in the script (cd
tools/bpf/bpftool; make_and_clean)
> + echo -e "command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null"
> + make $J -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null
> + if [ $? -ne 0 ] ; then
> + ERROR=1
> + fi
> +
> + check .
> + ldd bpftool | grep -q libbpf.so
> + if [ $? -ne 0 ] ; then
(Or "if ldd bpftool | grep -q libbpf.so ; then")
> + printf "FAILURE: Did not find libbpf linked\n"
Please also set $(ERROR) here.
(Also, stick to echo rather than mixing with printf? I can't remember
why I used one over echo in the check() function, that was probably not
on purpose.)
> + else
> + echo "binary: linked with libbpf"
> + fi
> + make -s -C ../../lib/bpf clean
> + make -s clean
> + rm -rf -- $TMPDIR
We probably want to clean features too? We tried to check that libbpf
was available, but with a very specific $(LIBBPF_DIR), which was
temporary and no longer exist. So better to reset features to a clean state?
> + echo
> +}
> +
> echo "Trying to build bpftool"
> echo -e "... through kbuild\n"
>
> @@ -145,3 +194,7 @@ make_and_clean
> make_with_tmpdir OUTPUT
>
> make_with_tmpdir O
> +
> +echo -e "... with dynamic libbpf\n"
> +
> +make_with_dynamic_libbpf
>
Thanks,
Quentin
