-
Notifications
You must be signed in to change notification settings - Fork 339
[SKIP SOFCI-TEST] topology: build using a local ALSA install #9870
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
Conversation
This wont pass CI yet, pending a alsa-utils fix to upstream 1st before merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, Windows CI jobs looking green, so seems topology builds are attempted at limited places. We do need to update the CI flows were topologies are built before merge.
e5c19f6
to
1cf54d0
Compare
838b80a
to
b3670cb
Compare
ac138c8
to
8f6c110
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seemed to work for me. I wonder if the alsa-utils build question "Press Return to acknowledge the previous three paragraphs." would cause issues in CI batch env?
Tested that - it passes straight thru as no console stdin is available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me too, and did not see anything wrong with the commits either.
136240c
to
8b165a3
Compare
Used to validate docker behavior in PR thesofproject#9870 Signed-off-by: Christopher Turner <[email protected]>
Used to validate docker behavior in PR thesofproject#9870 docker-run.sh: update Docker image tag to '20250410_no-alsa' Signed-off-by: Christopher Turner <[email protected]>
Used to validate docker behavior in PR thesofproject#9870 docker-run.sh: update Docker image tag to '20250410_no-alsa' Signed-off-by: Christopher Turner <[email protected]>
# Directory where repositories will be cloned/updated. | ||
if [[ -z "$SOF_WORKSPACE" ]]; then | ||
# Environment variable is empty or unset so use default | ||
BASE_DIR="$HOME/work/sof" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be
BASE_DIR="$HOME/work"
For the default case, the BASE_DIR should point to parent directory of sof as this is the directory assumed in tools/topology/CMakeLists.txt rules:
set(SOF_ALSA_TOOLS_DIR "${SOF_ROOT_SOURCE_DIRECTORY}/../tools/bin")
So BASE_DIR should match "${SOF_ROOT_SOURCE_DIRECTORY}".
This is causing failuress e.g. when we try to use "build-tools.sh -A" in docker in github (and might be a reason why @lgirdwood you "open-coded" the build recipe for github testbench workflow in this PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct. We are cloning into the ~/work/sof
workspace directory with contains the ~/work/sof/sof
FW directory. e.g. my SOF workspace directory looks like
~/work/sof$ ls
alsa-lib build-sof-staging LICENSE.workspace ruy sof-docs zephyr
alsa-utils flatbuffers modules sof sof-test
build-mtl gemmlowp nnlib-hifi4 sof-bin tflite-micro
build-rimage keys README.md sof.code-workspace tools
The workspace should include all ingredients used to build FW, topologies, tooling i.e. everything a developer needs to create a new module and deploy it. I'll fix the other makefiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lgirdwood Right so "$ZEPHYR_WORKSPACE=work/sof" is terms of
https://thesofproject.github.io/latest/getting_started/build-guide/build-with-zephyr.html
Ok I didn't realize that, I never use "sof" as the workspace directory name, but indeed then the problem is elsewhere setting the workspace path.
UPDATE2: ahaa, the build-from SOF source instructions do have "work/sof/sof". https://thesofproject.github.io/latest/getting_started/build-guide/build-from-scratch.html#step-1-set-up-the-workspace-directory . so then this is indeed aligned!
UPDATE: our docker images have used workspace path "/home/sof/work" with sof git checked out in "/home/sof/work/sof". This just means the workspace has to be manually be set with "--env SOF_WORKSPACE=/home/sof/work/".
@lrgirdwo In addition to the changes for the BASE_DIR i was testing this in the internal CI build and all the one specific flag does not work after installing the alsa tools and that is for the sof-ctl portion so for example '.scripts/build-tools.sh -A -c' the tools install great and I'm able to build topologies but the sof-ctl returns:
Is there a separate cmake file for sof-ctl that needs to be updated like the one for the topologies to work with the local ALSA install? |
Thanks - now fixed, was actually using system ALSA path |
This script builds a local installation of ALSA lib and associated ALSA utilities that does not impact the system ALSA installation. This will enable a later update to locally build topologies without using the docker container. Signed-off-by: Liam Girdwood <[email protected]>
Add a command line option to rebuild the ALSA libraries and tools required to build all topologies. Signed-off-by: Liam Girdwood <[email protected]>
Update testbench to build without docker and use a local ALSA lib and ALSA utils version from SOF ALSA git to align with latest topology developments. Signed-off-by: Liam Girdwood <[email protected]>
8b165a3
to
ddf1ce1
Compare
@@ -13,6 +13,7 @@ target_compile_options(sof-ctl PRIVATE | |||
) | |||
|
|||
target_include_directories(sof-ctl PRIVATE | |||
"${SOF_ROOT_SOURCE_DIRECTORY}/../tools/include" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this and works in my local environment, but with @cgturner1 's test docker image (thesofproject/sof:20250410_no-alsa) with no system ALSA installed, the build fails as linkage still uses system libasound:
: && /usr/bin/cc -g ctl/CMakeFiles/sof-ctl.dir/ctl.c.o -o ctl/sof-ctl -lasound && :
/usr/bin/ld: cannot find -lasound: No such file or directory
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kv2019i now fixed
Dont use the system ALSA installation which may be out of date with current topology development. Use the locally installed ALSA lib and utils installed in the SOF workspace. Signed-off-by: Liam Girdwood <[email protected]>
The installer workflow uses the legacy SDK build tools and is no longer valid for Zephyr based SDK migration. This also reduces effort in maintaining a SOF Docker container. Signed-off-by: Liam Girdwood <[email protected]>
ddf1ce1
to
60ba53a
Compare
@cgturner1 thinking aloud - is there a hard need for a docker container now after this update is working/merged ? This has been quite painful to make changes here - it would be great if CI could build topologies and tools without Docker in the future. |
After this is merged then there wont really be a need to continually update the docker but a lot of the current CI scripts are written around docker and the docker workspaces that removing that is a much bigger change than adding the command to build things using the -A script and build it locally inside the docker (which currently is working after the latest updates) So I think for step 1 is to just get these local builds working in a docker that doesn't have Alsa-tools since it will require the least amount of changes to the jenkins and then longer term can remove the dependency all together but that touches way more CI scripts and dependencies. One last open/question @lrgirdwo was doing final tests of this and was wondering if it can be possible to set the |
I think this topic was already discussed there: ... and in internal issue sofcitest/issues/692 I don't have time to look at this PR but the sheer length of it gives me the impression that a Docker container was a good idea. Sorry... EDIT: and also + many others. |
@cgturner1 Ok, so by default do you want the flags inverted i.e. always build ALSA unless a flag says dont build ALSA ? I can do that if preferred by you. |
ah didn't think about it that way i think the current optional way makes more sense to me where the flag includes it (to match the behavior of the other flags) for now i have it that when we want to build all tools + install rather than just running
i have:
I think for now we can leave as is and we can always revisit... I am trying to get time carved out tomorrow to have Kai merge as is and try to see if the internal CI changes work out worst case we back out on Friday and I figure out what changes were missing on the Jenkins side while not impacting production CI runs any more than a day |
@cgturner1 Merged now. We need to do this in steps and now proceed to update the jenkins task. |
Build topologies using a locally installed ALSA lib and ALSA utils without having to use Docker or upgrade the system ALSA installation.