Skip to content

topology: cmake: comments explaining why alsatplg is so verbose#2626

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.

Sign up for GitHub

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

Closed
marc-hb wants to merge 1 commit into from
Closed

topology: cmake: comments explaining why alsatplg is so verbose #2626

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tools/topology/CMakeLists.txt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -157,6 +157,11 @@ foreach(tplg ${TPLGS})

add_custom_command(
OUTPUT ${output}.tplg
# - In alsa-utils commit v1.2.2~15-gcbabe7a3f0cc, alsatplg (accidentally?)
# dropped the verbosity level; -v became a boolean.

@marc-hbmarc-hbMar 25, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

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

@jajanuszjajanuszMar 25, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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-hbmarc-hbMar 25, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

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

# - It unfortunately does not seem possible to base '-v' on ${VERBOSE}
# without adding a new layer of indirection / script between cmake and
# alsatplg

@marc-hbmarc-hbMar 25, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Example: zephyrproject-rtos/zephyr@4971d2a08452494d69

@jajanuszjajanuszMar 26, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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-hbmarc-hbMar 26, 2020•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

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 :-)

@jajanuszjajanuszMar 27, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 \$.

COMMAND alsatplg -v 1 -c ${output}.conf -o ${output}.tplg
DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${output}.conf
VERBATIM
Expand Down