Tools: Testbench: Avoid need linux kernel headers in build #7244

singalsu · 2023-03-10T11:12:26Z

This patch adds a simplified linux/types.h into testbench headers to enable include of ALSA asoc.h to build without other difficult kernel headers content.

The include path for asoc.h is changed in fuzzer, tplg_parser, and testbench to be without prefix alsa/sound/uapi to allow the Makefile headers path to specify only the uapi directory.

This change avoids build fail with xcc toolchain that can't use the gcc toolchain headers from the system where the ALSA headers are located.

marc-hb

Note this was successfully tested as part of larger

Tools: Testbench: xt-testbench #6513

lgirdwood · 2023-03-10T22:31:01Z

tools/fuzzer/CMakeLists.txt

@@ -40,6 +40,7 @@ target_include_directories(sof-fuzzer PRIVATE
 	"${SOF_ROOT_SOURCE_DIRECTORY}/src/arch/host/include"
 	"${SOF_ROOT_SOURCE_DIRECTORY}/src/platform/library/include"
 	"${SOF_ROOT_SOURCE_DIRECTORY}"
+        "/usr/include/alsa/sound/uapi"


This needs to be a CMake macro (with this as the default) as the ALSA lib prefix can be different.

marc-hb

This change avoids build fail with xcc toolchain that can't use the gcc toolchain headers from the system where the ALSA headers are located.

On second thought this is way too much of a hack.

Instead ask CMake to copy /usr/include/alsa/sound/uapi/asoc.h into something like build_dir/alsa/sound/uapi/asoc.h

Something like this UNTESTED:

# configuration time
configure_file ( /usr/include/alsa/sound/uapi/asoc.h ${CMAKE_CURRENT_BINARY_DIR}/alsa/sound/uapi/asoc.h

# build  time
target_include_directories(sometarget ${CMAKE_CURRENT_BINARY_DIR})

https://www.google.com/search?q=cmake+generated+header

PS: I ran pkg-config --cflags-only-I alsa and made sure it's empty on Ubuntu, Fedora and Arch Linux. In other words the location is standard.

lgirdwood · 2023-03-13T21:52:55Z

PS: I ran pkg-config --cflags-only-I alsa and made sure it's empty on Ubuntu, Fedora and Arch Linux. In other words the location is standard.

Thanks for checking @marc-hb - @singalsu I would make sure the comment about running pkg-config --cflags-only-I alsa is next to where you define the location. That way easy to check and update.

singalsu · 2023-03-16T15:08:00Z

@lgirdwood @marc-hb I tried to add pkg-config to build. Is this how you wanted it to be?

marc-hb · 2023-03-16T16:07:49Z

No this is very far from what I suggested, sorry for the confusion.

- I mentioned pkg-config --cflags-only-I alsa only to demonstrate it was always empty hence NOT needed. To test your latest CMake code invoking pkg-config you would need to find a really weird Linux distribution where it is not empty (hence required), I really doubt you found any such distro? So your latest CMake code is not tested and most likely does not work :-) Also, if we ever wanted to use pkg-config (I think we do not) then it should not be invoked from CMake directly, there's a CMake API for that: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html

- There are 3 different asoc.h files in /usr/include/ + maybe others installed manually so I think we should NOT #include <asoc.h> because no one wants to debug the order of -I include flags in very long and usually hidden command lines generated by 2 or 3 layers of build system indirections - multiplied by the number of supported toolchains. Top up that mess with whatever "creative" modifications developers have done to their system; like installing custom ALSA versions in random locations. So I think we should keep #include <alsa/sound/asoc.h>. It is what a non-empty pkg-config would require anyway. To make #include <alsa/sound/asoc.h> work with XCC, simply copy the hardcoded /usr/include/alsa/sound/asoc.h to some ${CMAKE_BINARY_DIR}/alsa/sound/ directory and add ${CMAKE_BINARY_DIR} to target_include_directories( [ SYSTEM ] ). See above.

marc-hb · 2023-03-16T16:50:45Z

simply copy the hardcoded /usr/include/alsa/sound/asoc.h to some ${CMAKE_BINARY_DIR}/alsa/sound/ directory and add the latter to target_include_directories()

This is actually the example given in https://cmake.org/cmake/help/latest/command/configure_file.html !

Two differences:

zero substitution needed, pure copy
You may need to create the ${CMAKE_BINARY_DIR}/alsa/sound/ directory yourself. This should do it: https://cmake.org/cmake/help/latest/command/file.html#make-directory

singalsu · 2023-03-16T16:57:20Z

I tried to manually set the path that pkg-config to /foo and checked that the build tried to find headers from /foo/alsa/sound/uapi

No this is very far from what I suggested, sorry for the confusion.

I mentioned pkg-config --cflags-only-I alsa only to demonstrate it was always empty hence NOT needed. To test your latest CMake code invoking pkg-config you would need to find a really weird Linux distribution where it is not empty (hence required), I really doubt you found any such distro?

I could test it only by setting other_alsa_include_path to /foo and check that code build failed with -I/foo/alsa/sound/uapi seen in the command line. But not a real test, agree.

So your latest CMake code is not tested and most likely does not work :-) Also, if we ever wanted to use pkg-config (I think we do not) then it should not be invoked from CMake directly, there's a CMake API for that: https://cmake.org/cmake/help/latest/module/FindPkgConfig.html

OK, I'll learn about that

There are 3 different asoc.h files in /usr/include/ + maybe others installed manually so I think we should NOT #include <asoc.h> because no one wants to debug the order of -I include flags in very long and usually hidden command lines generated by 2 or 3 layers of build system indirections - multiplied by the number of supported toolchains. Top up that mess with whatever "creative" modifications developers have done to their system; like installing custom ALSA versions in random locations. So I think we should keep #include <alsa/sound/asoc.h>. It is what a non-empty pkg-config would require anyway. To make #include <alsa/sound/asoc.h> work with XCC, simply copy the hardcoded /usr/include/alsa/sound/asoc.h to some ${CMAKE_BINARY_DIR}/alsa/sound/ directory and add the ${CMAKE_BINARY_DIR} to target_include_directories(). See above and below.

Yep, will try :)

marc-hb · 2023-03-16T17:35:29Z

copy the hardcoded /usr/include/alsa/sound/asoc.h to some ${CMAKE_BINARY_DIR}/alsa/sound/ directory and add ${CMAKE_BINARY_DIR} to target_include_directories( ). See above and below.

I just realized you may need the SYSTEM keyword in target_include_directories( ... SYSTEM ... ) because we have #include <alsa/sound/asoc.h> with < >.

Unless we switch to #include "alsa/sound/asoc.h"? Hopefully not, this would affect the regular host toolchain.

I'm not sure. However I'm sure that we would have EVEN MORE problems like this if we switch to #include "asoc.h" :-)

singalsu · 2023-03-17T11:22:27Z

I just realized you may need the SYSTEM keyword in target_include_directories( ... SYSTEM ... ) because we have #include <alsa/sound/asoc.h> with < >.

I tried to replace PRIVATE with SYSTEM. The cmake errors with that. Changing to SYSTEM PRIVATE works. But I have no idea what I'm doing. Is this better?

tools/testbench/CMakeLists.txt

This patch adds simplified linux/types.h into testbench headers to enable include of ALSA asoc.h to build without other difficult kernel headers content. The CMakeLists.txt files are updated to make a build time copy of asoc.h to subdirectory include of the build directories. The new directory is added to headers path. This change avoids build fail with xcc toolchain that can't use the gcc toolchain headers from the system where the ALSA headers are located. Suggested-by: Marc Herbert <[email protected]> Signed-off-by: Seppo Ingalsuo <[email protected]>

marc-hb · 2023-03-20T15:29:58Z

Only suspend/resume and PM failures in MTL https://sof-ci.01.org/sofpr/PR7244/build4801/devicetest

Everything else green including all cavs2.5 https://sof-ci.01.org/sofpr/PR7244/build4802/devicetest

singalsu marked this pull request as ready for review March 10, 2023 11:40

singalsu requested review from marc-hb, aborisovich, lgirdwood, plbossart, mmaka1, lbetlej and dbaluta as code owners March 10, 2023 11:40

marc-hb approved these changes Mar 10, 2023

View reviewed changes

lgirdwood reviewed Mar 10, 2023

View reviewed changes

marc-hb requested changes Mar 11, 2023

View reviewed changes

singalsu mentioned this pull request Mar 13, 2023

Tools: Testbench: xt-testbench #6513

Merged

singalsu force-pushed the testbench_avoid_linux_kernel_headers branch from 88b596f to 9fb2197 Compare March 16, 2023 14:43

singalsu requested review from lgirdwood and marc-hb March 16, 2023 14:45

singalsu force-pushed the testbench_avoid_linux_kernel_headers branch from 9fb2197 to a14774e Compare March 17, 2023 11:09

singalsu force-pushed the testbench_avoid_linux_kernel_headers branch from a14774e to f204d33 Compare March 17, 2023 11:22

marc-hb approved these changes Mar 17, 2023

View reviewed changes

lgirdwood approved these changes Mar 17, 2023

View reviewed changes

marc-hb reviewed Mar 17, 2023

View reviewed changes

tools/testbench/CMakeLists.txt Outdated Show resolved Hide resolved

singalsu force-pushed the testbench_avoid_linux_kernel_headers branch from f204d33 to ef99cc8 Compare March 17, 2023 17:45

marc-hb requested a review from ranj063 March 17, 2023 20:06

marc-hb requested review from ujfalusi, paulstelian97, aiChaoSONG, andyross, iuliana-prodan, kv2019i and lyakh March 17, 2023 20:06

kv2019i approved these changes Mar 20, 2023

View reviewed changes

kv2019i merged commit e0d8078 into thesofproject:main Mar 20, 2023

singalsu deleted the testbench_avoid_linux_kernel_headers branch May 9, 2023 12:59

marc-hb mentioned this pull request Aug 31, 2023

Preparation for introducing the SOF ALSA plugin #8132

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tools: Testbench: Avoid need linux kernel headers in build #7244

Tools: Testbench: Avoid need linux kernel headers in build #7244

singalsu commented Mar 10, 2023

marc-hb left a comment •

edited

Loading

lgirdwood Mar 10, 2023

marc-hb left a comment •

edited

Loading

lgirdwood commented Mar 13, 2023

singalsu commented Mar 16, 2023

marc-hb commented Mar 16, 2023 •

edited

Loading

marc-hb commented Mar 16, 2023

singalsu commented Mar 16, 2023 •

edited by marc-hb

Loading

marc-hb commented Mar 16, 2023

singalsu commented Mar 17, 2023

marc-hb commented Mar 20, 2023 •

edited

Loading

Tools: Testbench: Avoid need linux kernel headers in build #7244

Tools: Testbench: Avoid need linux kernel headers in build #7244

Conversation

singalsu commented Mar 10, 2023

marc-hb left a comment • edited Loading

Choose a reason for hiding this comment

lgirdwood Mar 10, 2023

Choose a reason for hiding this comment

marc-hb left a comment • edited Loading

Choose a reason for hiding this comment

lgirdwood commented Mar 13, 2023

singalsu commented Mar 16, 2023

marc-hb commented Mar 16, 2023 • edited Loading

marc-hb commented Mar 16, 2023

singalsu commented Mar 16, 2023 • edited by marc-hb Loading

marc-hb commented Mar 16, 2023

singalsu commented Mar 17, 2023

marc-hb commented Mar 20, 2023 • edited Loading

marc-hb left a comment •

edited

Loading

marc-hb left a comment •

edited

Loading

marc-hb commented Mar 16, 2023 •

edited

Loading

singalsu commented Mar 16, 2023 •

edited by marc-hb

Loading

marc-hb commented Mar 20, 2023 •

edited

Loading