https://sof-ci.01.org/sofpr/PR7546/build6851/build/mtl_zph_ipc4.txt is still invoking rimage with -b 1 as it always has.

https://sof-ci.01.org/sof-pr-viewer/#/build/PR7546/build11924100 has the usual error below.

Everything else is green including https://sof-ci.01.org/sofpr/PR7546/build6853/devicetest and https://sof-ci.01.org/sofpr/PR7546/build6852/devicetest

@wszypelt a green report would be so unusual that it would be almost worrying!

cc:

22:33:53,387 INFO  - Executing pre-execute action...
22:33:53,387 INFO  - Running step...
22:42:34,319 INFO  - Executing post-execute action...
22:42:34,319 ERROR - Step 'master>SCANS>TESTS>TRIGGER PYTHON SMOKE_IPC4 TESTS' is failed: Step is failed since the triggered build is failed, cancelled, or timed out.

Someone re-run https://sof-ci.01.org/sof-pr-viewer/#/build/PR7546/build11933012 and even that is green now. Everything is.

Thanks for the review! Very hard to find reviewers in this area.

but I am not fan of the solution you took for sign_cmd += ["-b", "1" ] . It makes no sense in my opinion because it is just hardcoded value in the script without any specific reason

So the rationale for hardcoding sign_cmd += ["-b", "1" ] was to keep the binary 100% unchanged. Thanks for pointing this out; it's not a very good reason. In fact I'm now leaning toward dropping this line. Then rimage will default to -b 0 when there is no -b. Also, if I drop this line then users can more easily override the value using west config or whatever other CMake way they find without wondering what happens when there are two -b options on the same rimage command line; always confusing.

Would dropping sign_cmd += ["-b", "1" ] work for you?

(and eventually consider removing this build counter from rimage if it has no use cases).

Maybe. SOF_BUILD is disabled by default but it still works in XTOS builds though. Maybe some SOF_BUILD fan could also find a way to finally make it work when building Zephyr? Dunno.

pass SOF FW version to rimage CMake at configure time.

There seems to be a misunderstanding here? -b is an rimage parameter passed when running rimage.

Generate sof_versions.h file using configure_file function.

But this would make sof_versions.h more static that it is now? See commit 23d2a01. Either way it looks off-topic to me in this PR but maybe I'm missing something.

FetchContent() suggestion.

Adopting FetchContent() would be a much bigger change than this small, "no-op" PR! I mean SOF_BUILD never worked before this small PR and it still does not work after this PR and that's pretty much it...

I see where you're coming from because FetchContent() clearly looks like a tool designed to solve dependencies like SOF=>rimage . However I see a number of reasons why FetchContent would not fit here. Main reason: I think it would go against the spirit and the letter of the Zephyr build.

• Some Zephyr tests have a DIRECT dependency rimage to run on hardware WITHOUT depending on any SOF code! So FetchContent() in SOF would not help them, they would have to FetchContent in Zephyr too...
• ... but Zephyr requires all dependencies to be listed in a centralized place: either in a west manifest: https://docs.zephyrproject.org/latest/contribute/external.html#contributing-external-tooling (or in module.yml for binaries https://docs.zephyrproject.org/latest/contribute/bin_blobs.html#bin-blobs)
• Tracking dependencies in a single place makes https://www.cisa.gov/sbom easier. I think SBOM is one of the reasons Zephyr wants it done like this.
• Downloading at configuration/build time goes against "Hermetic" builds https://github.com/tektoncd/pipeline/blob/main/docs/hermetic.md ("This means they do not have network access,...)

Please start a new issue if you want to keep recommending FetchContent()?

Would dropping sign_cmd += ["-b", "1" ] work for you?

Sure, it's my only concern.
Maybe I'll submit a feature request about my idea. Because you see, FetchContent may but does not have to clone. We may still stick to west in terms of cloning, just use functions from fetch content to populate the CMake cache of rimage and run its configure step as part of SOF configure step. Then, built rimage executable would have 'hardcoded version' of SOF inside of it. And we could use that version when no rimage input arguments related to version are provided (aka 'default'). So rimage would use not only information provided in the imput args but also information given from the build. Well, anyway that was just an idea.
I think its worth considering because:

• rimage anyway relies on other data from SOF (headers?)
• we parse version information from SOF using python regex just to pass it as runtime input argument of rimage executable
• so in fact we 'pretend' rimage to be 'stand-alone' tool while it strongly depends on SOF anyway.
• changing SOF version or revision would cause change in rimage cache and would trigger rebuilding to contain new proper version

Would dropping sign_cmd += ["-b", "1" ] work for you?

I was dropping this line and then I had second thoughts and I took a second look at #6276. I found internal issue 257. So the whole reason SOF_BUILD was added to xtensa-build-zephyr.py in the first place is apparently because some broken test is checking for SOF_BUILD to be hardcoded to 1!! So I kept the hardcoding for now and added a FIXME :-(

zmain failures are known and unrelated: #7614

Seems ready to go, but need a clean CI pass for required checks.

Seems ready to go, but need a clean CI pass for required checks.

The Ubuntu mirrors were very unreliable yesterday. Today they worked!

https://sof-ci.01.org/sofpr/PR7546/build7928/devicetest has a couple failures as usual, nothing that could be caused by this.

The zmain branch needs the Zephyr SDK 0.16.1, known issue.

Everything else is green now.