Skip to content

testbench: dont build testbench, tools and topologies in the source directory #9871

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

lgirdwood
Copy link
Member

Currently all topologies, testbench and some tools are built within the FW source directory unlike FW which has separate binary destination directories outside of the main FW source directory.

This series moves all tools, testbench and topologies binaries to outside the source directory to the parent SOF "workspace" directory alongside the FW build output directories.

@lgirdwood
Copy link
Member Author

I expect this to break CI if we have hard coded assumptions of where to find certain binaries.
@singalsu pls take a look.

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general a very good idea! Having tools' build directories withing the source tree was always quite inconvenient

echo Converting raw output to "$OUTWAV"
if [[ "$BITS" == "24" ]]; then
sox --encoding signed-integer -L -r "$RATE_OUT" -c "$CHANNELS_OUT" -b 32 "$OUTFILE1" "$OUTWAV" vol 256
sox --encoding signed-integer -L -r "$RATE_OUT" -c "$CHANNELS_OUT" -b 32 \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original indentation seems to have been correct. Also this and the following line shouldn't have equal indentation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is a mixture of tabs/spaces for indentation. Fixed this part.

echo "Error: environment variable SOF_WORKSPACE need to be set to top level sof directory"

# check if we are in the sof FW directory
if [ -f "Kconfig.sof" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we maybe use something like $(dirname "$0") to identify the target directory, where we have to switch to? Then it'll be possible to call the script from anywhere

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That adds a bit of complexity if we say this is allowed to be executed from anywhere, in general this will be called from the SOF directory or the parent SOF workspace directory.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, simplified this for non default installs - it will now check for SOF_WORKSPACE environment if set.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers, @singalsu probably better to comment on the end-user impact on testbench scripting (see my comment).

This will need to be merged at the same time with Jenkins build changes. Another option is to make the build-outside-src an option, merge this in SOF and let Jenkins adopt this when it is ready (no flag day approach).

@lgirdwood lgirdwood force-pushed the lrg/topic/testbench branch from 5d17e7d to a4b62b7 Compare March 5, 2025 13:34
@lgirdwood
Copy link
Member Author

This depends on #9870 so DNM as for now.

@lgirdwood lgirdwood added the DNM Do Not Merge tag label Mar 5, 2025
Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far, please add these changes to keep Matlab script tests working

diff --git a/tools/test/audio/comp_run.sh b/tools/test/audio/comp_run.sh
index 096ff2c34..6beaf74f5 100755
--- a/tools/test/audio/comp_run.sh
+++ b/tools/test/audio/comp_run.sh
@@ -132,13 +132,13 @@ run_testbench ()
 parse_args "$@"
 
 # Path to topologies
-TPLG_DIR=../../build_tools/test/topology
+TPLG_DIR=../../../../build_tools/test/topology
 
 # Testbench path and executable
 if [[ -z $XTRUN ]]; then
-    PATH_TESTBENCH=../../testbench/build_testbench/install/bin/$TESTBENCH
+    PATH_TESTBENCH=../../../../build_testbench/install/bin/$TESTBENCH
 else
-    BUILD_DIR=../../testbench/build_xt_testbench
+    BUILD_DIR=../../../../build_xt_testbench
     PATH_TESTBENCH="$BUILD_DIR"/$TESTBENCH
     source "$BUILD_DIR"/xtrun_env.sh
     XTRUN_CMD=$XTENSA_PATH/$XTRUN
@@ -168,7 +168,7 @@ PIPELINES=
     [[ $DIRECTION == "playback" ]] && PIPELINES="-p 1,2"
     [[ $DIRECTION == "capture" ]] && PIPELINES="-p 3,4"
     TPLGFN=sof-hda-benchmark-${COMP}${BITS_IN}.tplg
-    TPLG_DIR="../../build_tools/topology/topology2/development"
+    TPLG_DIR="../../../../build_tools/topology/topology2/development"
     TPLG_BUILD_TIP="Please run scripts/build-tools.sh"
 }

@lgirdwood lgirdwood force-pushed the lrg/topic/testbench branch from a4b62b7 to 36f7844 Compare March 30, 2025 17:42
@lgirdwood
Copy link
Member Author

Changes for V2:

  1. Check for SOF_WORKSPACE environment for non default installs.
  2. Added @singalsu patch.

Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good cleanup, thanks!

@lgirdwood lgirdwood force-pushed the lrg/topic/testbench branch from 36f7844 to 4417390 Compare April 7, 2025 18:17
@lgirdwood
Copy link
Member Author

depends on #9870

@lgirdwood
Copy link
Member Author

@jsarha fyi - was meaning this one.

@lgirdwood lgirdwood force-pushed the lrg/topic/testbench branch from 4417390 to 03dbc88 Compare May 20, 2025 19:59
@lgirdwood lgirdwood removed the DNM Do Not Merge tag label May 20, 2025
@lgirdwood
Copy link
Member Author

Rebased now that #9870 is merged

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs Jenkins update to merge.

@lgirdwood
Copy link
Member Author

Needs Jenkins update to merge.

Yeah - hopefully the last one. @cgturner1 pls see jenkins build i.e. we no longer build tools in the src directory.

/usr/bin/mkdir -p -m 0755 /home/cwd_user/work/sof/tools/var/lib/alsa
make[3]: Leaving directory '/home/cwd_user/work/sof/alsa-utils'
make[2]: Leaving directory '/home/cwd_user/work/sof/alsa-utils'
make[1]: Leaving directory '/home/cwd_user/work/sof/alsa-utils'
Build and installation complete for /home/cwd_user/work/sof/alsa-utils
All repositories processed.
mkdir: cannot create directory '/home/sof/work/sof.git/../build-tools-tplg': Permission denied

@cgturner1
Copy link

ok will need to take a look...a lot of the issues stem from the user settings (in scripts/sudo-cwd.sh) being done in the docker which seem to be from the way it mounts uses the local volumes as a workspace

and it sems to restrict the user "cwd" from creating directories in certain places ...ideally this docker should run as the sof user and we would be fine..i'll need to figure out the change to get this to cooperate at least to get the mkdir to work.

will run some tests and update when i get this updated

Allow both scripts to be executed from the SOF FW directory or the parent
SOF workspace directory using an environment variable or the default
SOF workspace.

This simplifies usage and opens the way for vscode integration.

Signed-off-by: Liam Girdwood <[email protected]>
Dont built binaries in the source directory, build in the workspace.

Signed-off-by: Liam Girdwood <[email protected]>
Fix the indentation.

Signed-off-by: Liam Girdwood <[email protected]>
Move all testbench build and test collateral outside of the SOF source
directory to the workspace directory.

Signed-off-by: Liam Girdwood <[email protected]>
Move the test data files outside of the source directory and add
more context to their names for easier lookup. Do the same with
the outfile and generate it if we keep the temporary files.

Signed-off-by: Liam Girdwood <[email protected]>
Add a simple deployment option fro topology2 to copy topologies to
the staging build directory. Simple today as only Intel use tplg2.

Signed-off-by: Liam Girdwood <[email protected]>
@lgirdwood lgirdwood force-pushed the lrg/topic/testbench branch from 03dbc88 to 2aba76b Compare July 14, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants