Re: [PATCH 1/2] tools build feature: Add some comments to explain the FEATURE_TESTS logic

[Ian Rogers]

On Wed, Dec 11, 2024 at 2:45 PM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>
> The tools/build/feature/test-all.c works in conjunction with the
> tools/build/Makefile.feature FEATURE_TESTS_BASIC and FEATURE_TESTS_EXTRA
> contents, so that if test-all.c manages to be built, we go on and
> iterate all entries in FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA setting
> then to 1.

nit: s/then/them/

> To test this:
>
>   $ rm -rf /tmp/b ; mkdir /tmp/b ; make -C tools/perf O=/tmp/b feature-dump
>   $ cat /tmp/b/feature/test-all.make.output
>   $ ldd /tmp/b/feature/test-all.bin
>         linux-vdso.so.1 (0x00007f2a47a67000)
>         libdw.so.1 => /lib64/libdw.so.1 (0x00007f2a477cf000)
>         libpython3.12.so.1.0 => /lib64/libpython3.12.so.1.0 (0x00007f2a471fe000)
>         libm.so.6 => /lib64/libm.so.6 (0x00007f2a4711a000)
>         libtraceevent.so.1 => /lib64/libtraceevent.so.1 (0x00007f2a470f2000)
>         libtracefs.so.1 => /lib64/libtracefs.so.1 (0x00007f2a470cb000)
>         libcrypto.so.3 => /lib64/libcrypto.so.3 (0x00007f2a46c1b000)
>         libz.so.1 => /lib64/libz.so.1 (0x00007f2a46bf8000)
>         libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1 (0x00007f2a46bad000)
>         libcapstone.so.5 => /lib64/libcapstone.so.5 (0x00007f2a464b8000)
>         libopencsd_c_api.so.1 => /lib64/libopencsd_c_api.so.1 (0x00007f2a464a8000)
>         libopencsd.so.1 => /lib64/libopencsd.so.1 (0x00007f2a46422000)
>         libelf.so.1 => /lib64/libelf.so.1 (0x00007f2a46406000)
>         libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f2a463f6000)
>         libslang.so.2 => /lib64/libslang.so.2 (0x00007f2a46113000)
>         libperl.so.5.38 => /lib64/libperl.so.5.38 (0x00007f2a45d74000)
>         libc.so.6 => /lib64/libc.so.6 (0x00007f2a45b83000)
>         liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f2a45b50000)
>         libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f2a45a91000)
>         libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f2a45a7b000)
>         /lib64/ld-linux-x86-64.so.2 (0x00007f2a47a69000)
>         libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007f2a45a6b000)
>         libpopt.so.0 => /lib64/libpopt.so.0 (0x00007f2a45a5b000)
>         libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f2a45a51000)
>         libgmodule-2.0.so.0 => /lib64/libgmodule-2.0.so.0 (0x00007f2a45a4a000)
>         libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x00007f2a458fa000)
>         libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f2a45696000)
>         libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f2a45668000)
>         libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f2a45630000)
>         libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x00007f2a45590000)
>   $ head /tmp/b/FEATURE-DUMP
>   feature-backtrace=1
>   feature-libdw=1
>   feature-eventfd=1
>   feature-fortify-source=1
>   feature-get_current_dir_name=1
>   feature-gettid=1
>   feature-glibc=1
>   feature-libbfd=1
>   feature-libbfd-buildid=1
>   feature-libcap=1
>   $
>
> There are inconsistencies that are being audited, as can be seen above
> with the libcap case, that is not linked with test-all.bin nor is
> present in test-all.c, so shouldn't be set as present. Further patches
> are going to address those inconsistencies, but lets document this a bit
> more to reduce the chances of this happening again.
>
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: Ian Rogers <irogers@xxxxxxxxxx>
> Cc: James Clark <james.clark@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  tools/build/Makefile.feature | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 80563154318601ac..52600e4d33af8712 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -28,6 +28,41 @@ endef
>  #   the rule that uses them - an example for that is the 'bionic'
>  #   feature check. ]
>  #
> +# These + the ones in FEATURE_TESTS_EXTRA are included in
> +# tools/build/feature/test-all.c and we try to build it all together
> +# then setting all those features to '1' meaning they are all enabled.
> +#
> +# There are things like fortify-source that will be set to 1 because test-all
> +# is built with the flags needed to test if its enabled, resulting in
> +#
> +#   $ rm -rf /tmp/b ; mkdir /tmp/b ; make -C tools/perf O=/tmp/b feature-dump
> +#   $ grep fortify-source /tmp/b/FEATURE-DUMP
> +#   feature-fortify-source=1
> +#   $
> +#
> +#   All the others should have lines in tools/build/feature/test-all.c like:
> +#
> +#    #define main main_test_disassembler_init_styled
> +#    # include "test-disassembler-init-styled.c"
> +#    #undef main
> +#
> +#    #define main main_test_libzstd
> +#    # include "test-libzstd.c"
> +#    #undef main
> +#
> +#    int main(int argc, char *argv[])
> +#    {
> +#      main_test_disassembler_four_args();
> +#      main_test_libzstd();
> +#      return 0;
> +#    }
> +#
> +#    If the sample above works, then we end up with these lines in the FEATURE-DUMP
> +#    file:
> +#
> +#    feature-disassembler-four-args=1
> +#    feature-libzstd=1
> +#
>  FEATURE_TESTS_BASIC :=                  \
>          backtrace                       \
>          libdw                           \
> --
> 2.47.0
>