topology: cmake: comments explaining why alsatplg is so verbose

[marc-hb]

... and why it's very difficult to fix. Learned the hard way.

Signed-off-by: Marc Herbert marc.herbert@intel.com

[marc-hb]

"accident" at alsa-project/alsa-utils@cbabe7a3f0cc84ecd3#r38034335

[jajanusz]

I'm not sure if I understand. What do you mean by 'bacame a boolean', what other arg do I have to pass instead of -v <level>, something like -v true?

[marc-hb]

I'm not 100% sure what happens when you misconfigure getopt but in my testing I observed that -v whatever is verbose, including -v 0

[marc-hb]

Example: zephyrproject-rtos/zephyr@4971d2a08452494d69

[jajanusz]

This comment is not true. It is possible in many other ways. I'm not sure how you'd like to use it but if you want to base it on VERBOSE env variable, it can be done with parameter expressions or some subshells invoked inline. If you want to base it on alsa version then you'd need to do some checks here in cmake.

If your problem is that you want to just be able to override this -v 1 part, then we can add VERBOSE_ALSA env var switch for this. So it'd look like COMMAND alsatplg \${VERBOSE_ALSA--v 1} -c ${output}.conf -o ${output}.tplg and you can launch it with VERBOSE_ALSA="" make ..., to disable verbose.

[marc-hb]

it can be done with parameter expressions

To be portable, cmake COMMAND invokes alsatplg directly, it doesn't go through an OS dependent shell tries hard to quote everything away from the shell (especially with VERBATIM) - unless you explicitly ask for a non-portable shell

So it'd look like COMMAND alsatplg ${VERBOSE_ALSA--v 1}

First thing I naively tried was something like this: ${VERBOSE:+-v 1}. cmake (correctly) quotes everything to make sure alsatplg gets passed exactly this and that the intermediate shell does nothing with it.

some subshells invoked inline

= adding a new (subshell) layer of indirection. I can rephrase to include that.

you'd need to do some checks here in cmake.

VERBOSE works after cmake has run, that's the beauty of it. So yes, you can probably decide this at cmake time (I can give it a try) but not at make/ninja time because by then cmake has run, it's too late.

This comment is not true.

I suggest a closer look at the level of cmake wizardry in the Zephyr project before assuming
zephyrproject-rtos/zephyr@4971d2a08452494d69 didn't know what they were doing :-)

[jajanusz]

some subshells invoked inline

= adding a new (subshell) layer of indirection. I can rephrase to include that.

Well, yea, you can think of it as level of indirection ;)

However, you still can do this with params expressions, they just need to be properly escaped - You can disable VERBATIM if it helps. We support windows only for stuff that is really done there like building FW, I guess no1 is going to run alsa tools on windows and I was adding VERBATIM usually cos of cross-platform support.
I didn't try the sample that I wrote (\${VERBOSE_ALSA--v 1}), it was just hint . You may need to escape $ as $$ instead of \$.

[marc-hb]

The only "Internal Intel CI System/merge/build" failure is:

mkdir build_fw && cd build_fw
cmake -GNinja -DTOOLCHAIN=xt -DROOT_DIR=%CONFIG_PATH%\xtensa-elf ..
ninja %make_command%
ninja bin -v
Command return code: 1
Command error output: CMake Warning at CMakeLists.txt:79 (message):
Couldn't get compiler version

It happened only ICL_SSP and only for TestDmicBothFifoSimpleCapture16000hz16b16b2ch ?!
As this PR is a pure comment change in a seemingly unrelated CMakeLists.txt I think this failure can be treated as an unrelated glitch.

[jajanusz]

The only "Internal Intel CI System/merge/build" failure is:

mkdir build_fw && cd build_fw
cmake -GNinja -DTOOLCHAIN=xt -DROOT_DIR=%CONFIG_PATH%\xtensa-elf ..
ninja %make_command%
ninja bin -v
Command return code: 1
Command error output: CMake Warning at CMakeLists.txt:79 (message):
Couldn't get compiler version

It happened only ICL_SSP and only for TestDmicBothFifoSimpleCapture16000hz16b16b2ch ?!
As this PR is a pure comment change in a seemingly unrelated CMakeLists.txt I think this failure can be treated as an unrelated glitch.

As long as you change only comments, we can ignore pretty much any CI except checkpatch.
We can merge just comments if you want, but I wanted to at least partially solve your initial issue.

[marc-hb]

However, you still can do this with params expressions, they just need to be properly escaped - You can disable VERBATIM if it helps.

Done in PR #2654