Utilize standalone rimage tool

[dcpleung]

This modifies the build script to utilize an externally built rimage tool, and also removes the rimage tool from source tree.

Requires thesofproject/rimage#1 and thesofproject/rimage#3

[plbossart]

Why is this needed?

[jajanusz]

@plbossart We want to decouple rimage from SOF as it is not audio-specific stuff, it's just image composing tool for DSP's FW. It's weird f.e. for Zephyr to pull whole SOF repo just for rimage.

[jajanusz]

I agree that it should be decoupled from FW, but we shouldn't just like that cut it out and expect user to have it installed. It's step backward and will cause many troubles.
We had it like this before that user had to build rimage manually and was expect by buildsystem, it's PITA - for user and for developers that see the same questions and unrelated problem caused by this being reported. It has to be built as part of build process automatically, so first we need to have rimage uploaded to different repo and then remove it from here and update build system to automatically pull repo with rimage and build it (like we have it with cmocka).

[lgirdwood]

@jajanusz @dcpleung it makes sense to create a subrepo for rimage in sof. This keep everything in sync with the external repo.

[jajanusz]

@jajanusz @dcpleung it makes sense to create a subrepo for rimage in sof. This keep everything in sync with the external repo.

Yea good idea, git submodule will make things easier

[dcpleung]

Understood. I will work on getting rimage repo as a submodule.

[plbossart]

Are you really sure git submodules are the answer?
I have seen nothing but issues with submodules, and I would like to double-check this is compatible with GitHub (PRs, merge, etc)

see e.g. https://github.blog/2016-02-01-working-with-submodules/

If we go with multiple repositories then it's just easier to use "repo".

[dcpleung]

Updated to use git submodule.

[lgirdwood]

@plbossart submodules do have a downside in that users need to remeber and manually update each submodule when updating the main repo. A script would eliminate any forgetfullnes though (which qemu do run for their 3 submodules as part of the build process so maybe this is the best method).
Fwiw, repo has a manual cost too since we need to keep the manifest updated.

[lgirdwood]

@xiulipan @zrombel do we need any CI changes to deal with a rimage git submodule or would including submodule update in part of build be good enough for CI.

[lgirdwood]

@dcpleung could rimage be updated via a script and could this script be invoked at the cmake stage where we set toolchain etc. I'm trying to find a way that wont impact CI and means users can test local rimage updates.

mkdir build_byt && cd build_byt
cmake -DTOOLCHAIN=xtensa-byt-elf -DROOT_DIR=`pwd`/../../xtensa-root/xtensa-byt-elf ..
```. 

[dcpleung]

@dcpleung could rimage be updated via a script and could this script be invoked at the cmake stage where we set toolchain etc. I'm trying to find a way that wont impact CI and means users can test local rimage updates.

mkdir build_byt && cd build_byt
cmake -DTOOLCHAIN=xtensa-byt-elf -DROOT_DIR=`pwd`/../../xtensa-root/xtensa-byt-elf ..
```. 

It is possible to add a cmake custom command/target to run git submodule update or the script. However, I am not sure the impact since I haven't used submodule much.

[plbossart]

@plbossart submodules do have a downside in that users need to remeber and manually update each submodule when updating the main repo. A script would eliminate any forgetfullnes though (which qemu do run for their 3 submodules as part of the build process so maybe this is the best method).

Indeed, we need to take the user out of the equation. It's got to be scripted/automated.

Fwiw, repo has a manual cost too since we need to keep the manifest updated.

not really, and we already have one manifest btw https://github.com/thesofproject/sof-manifests

[jajanusz]

@dcpleung could rimage be updated via a script and could this script be invoked at the cmake stage where we set toolchain etc. I'm trying to find a way that wont impact CI and means users can test local rimage updates.

mkdir build_byt && cd build_byt
cmake -DTOOLCHAIN=xtensa-byt-elf -DROOT_DIR=`pwd`/../../xtensa-root/xtensa-byt-elf ..
```. 

It is possible to add a cmake custom command/target to run git submodule update or the script. However, I am not sure the impact since I haven't used submodule much.

You basically can do it similar to what we do in test/cmocka, just for download command of ExternalProject instead of git clone use submodule init/update.

[xiulipan]

@lgirdwood We will use scripts to do the build.
Please also update the build scripts after we have make an alignment about how to deal with the rimage sync with sof.

[jajanusz]

@xiulipan @dcpleung @lgirdwood @plbossart You need no changes in CI, everything can be done in cmake files if done properly

[lgirdwood]

@jajanusz I was hoping you would chime in here. :) Ideally we want to synchronise any external submodules prior to a build but also need to be able to disable this and force a local rimage verion (for say anyone hacking and testing rimage). How would we do this in cmake ?

To give an example, qemu does submodule updates as part of "configure" which is a python script in qemu.

./configure (updates to latest submodules)
make
make install

configure is only needed to be run when changing the qemu configuration or if you need to update it's submodules. It also allows for pulling certain commits/tags of submodules.

I guess we could have this as part of a make rule or as part of the logic for generating the makefiles ?

[jajanusz]

@jajanusz I was hoping you would chime in here. :) Ideally we want to synchronise any external submodules prior to a build but also need to be able to disable this and force a local rimage verion (for say anyone hacking and testing rimage). How would we do this in cmake ?

To give an example, qemu does submodule updates as part of "configure" which is a python script in qemu.

./configure (updates to latest submodules)
make
make install

configure is only needed to be run when changing the qemu configuration or if you need to update it's submodules. It also allows for pulling certain commits/tags of submodules.

I guess we could have this as part of a make rule or as part of the logic for generating the makefiles ?

@dcpleung @lgirdwood You need mix of:

sof/test/cmocka/CMakeLists.txt

Lines 14 to 28 in decd206

	# Build Cmocka locally
	ExternalProject_Add(cmocka_git
	GIT_REPOSITORY https://github.com/thesofproject/cmocka
	PREFIX "${PROJECT_BINARY_DIR}/cmocka_git"
	BINARY_DIR ${cmocka_binary_directory}
	CMAKE_ARGS -DCMAKE_BUILD_TYPE=Release
	-DCMAKE_TOOLCHAIN_FILE=${CMAKE_CURRENT_SOURCE_DIR}/cmocka-xtensa-xt-toolchain.cmake
	-DWITH_SHARED_LIB=OFF
	-DWITH_STATIC_LIB=ON
	-DWITH_EXAMPLES=OFF
	-DWITH_POSITION_INDEPENDENT_CODE=OFF
	-DWITH_TINY_CONFIG=ON
	BUILD_BYPRODUCTS "${cmocka_binary_directory}/src/libcmocka-static.a"
	INSTALL_COMMAND ""
	)

&

sof/src/arch/xtensa/CMakeLists.txt

Lines 205 to 215 in decd206

	ExternalProject_Add(rimage_ep
	DEPENDS check_version_h
	DOWNLOAD_COMMAND ""
	SOURCE_DIR "${PROJECT_SOURCE_DIR}/rimage"
	PREFIX "${PROJECT_BINARY_DIR}/rimage_ep"
	BINARY_DIR "${PROJECT_BINARY_DIR}/rimage_ep/build"
	BUILD_ALWAYS 1
	CMAKE_ARGS -DVERSION_H_PATH=${VERSION_H_PATH}
	-DPEM_KEY_PREFIX=${PROJECT_SOURCE_DIR}/rimage/keys
	INSTALL_COMMAND ""
	)

in the place of current rimage_ep target, it should be easy, just fix paths and adujst arguments (I assume rimage in other repo is also cmake proj).
If you want I can do it in beginning of december, cos I'm going for 2 weeks holiday tomorrow ;p

[jajanusz]

@dcpleung @lgirdwood I guess it'd be something like this but I didn't check it.

ExternalProject_Add(rimage_ep 
 	DEPENDS check_version_h 
 	GIT_REPOSITORY https://github.com/thesofproject/rimage # ??
 	PREFIX "${PROJECT_BINARY_DIR}/rimage_ep" 
 	BINARY_DIR "${PROJECT_BINARY_DIR}/rimage_ep/build" 
 	BUILD_ALWAYS 1 
 	CMAKE_ARGS -DVERSION_H_PATH=${VERSION_H_PATH} 
 		-DPEM_KEY_PREFIX=${PROJECT_SOURCE_DIR}/rimage/keys 
 	INSTALL_COMMAND "" 
 ) 

[jajanusz]

@dcpleung @lgirdwood I guess it'd be something like this but I didn't check it.

ExternalProject_Add(rimage_ep 
 	DEPENDS check_version_h 
 	GIT_REPOSITORY https://github.com/thesofproject/rimage # ??
 	PREFIX "${PROJECT_BINARY_DIR}/rimage_ep" 
 	BINARY_DIR "${PROJECT_BINARY_DIR}/rimage_ep/build" 
 	BUILD_ALWAYS 1 
 	CMAKE_ARGS -DVERSION_H_PATH=${VERSION_H_PATH} 
 		-DPEM_KEY_PREFIX=${PROJECT_SOURCE_DIR}/rimage/keys 
 	INSTALL_COMMAND "" 
 ) 

Ofc it's for pure git repo download variant, if you want to go with submodules then instead of GIT_REPOSTITORY use something like DOWNLOAD_COMMAND " git submodule update --init"

[dcpleung]

I can add the submodule update if we are still going to the submodule route.

[lgirdwood]

I can add the submodule update if we are still going to the submodule route.

Please do.

[lgirdwood]

So I guess there are now no blockers on CI side to merge, let merge as soon as conflict are resolved.

[dcpleung]

Conflicts resolved and updated to latest commits from rimage.

[lgirdwood]

Waiting on CI, but build fails on TGL due to overlap. @zrombel does CI need anything to use the standalone tool ?

[zrombel]

CI is all good but it seems that there is a problem with building TGL with standalone rimage. I get same error when building locally where building with old rimage doesn't fail. I've asked @jajanusz to look into it.

[jajanusz]

CI is failing cos rimage is incorrectly parsing ELF, you are missing elf.c changes related to LMA/VMA from this PR (rimage* commits) : #2516

[ktrzcinx]

@dcpleung can you update rimage commit reference ?

[dcpleung]

Rebased and updated the rimage submodule SHA.

[ktrzcinx]

@dcpleung thanks

[jajanusz]

@dcpleung
Great, one nitpick left.
Some commits titles don't follow format used in this repo and in kernel -> topic: what changed.
Please update them.
Thanks.

[marc-hb]

Code is cloned on master machine and copied to build machines where build process is being performed and from what I can see in build log build machines doesn't have access to public github server.

@zrombel Is there an exception for cmocka?

        ExternalProject_Add(cmocka_git
                GIT_REPOSITORY https://github.com/thesofproject/cmocka

[marc-hb]

Reworked in cmake/git-submodules: don't throw away work in progress at build time #3048

[zrombel]

@marc-hb No, CI is using prebuilt cmoka on VM's.

if(DEFINED CMOCKA_DIRECTORY)
	# Prebuilt Cmocka lib
	set_property(TARGET cmocka PROPERTY IMPORTED_LOCATION "${CMOCKA_DIRECTORY}/lib/libcmocka-static.a")
	set(CMOCKA_INCLUDE_DIR "${CMOCKA_DIRECTORY}/include")
else()
	set(cmocka_binary_directory "${PROJECT_BINARY_DIR}/cmocka_git/build")

	# Build Cmocka locally
	ExternalProject_Add(cmocka_git

[marc-hb]

While testing "tarbomb" fix #3132 I noticed the conversion of rimage to a submodule broke tarball releases. Interesting no one noticed - does anyone still use tarballs?

[lgirdwood]

@marc-hb probably not too many folks, but it does need fixed. Can you take a look and send a PR. Thanks

[marc-hb]

the conversion of rimage to a submodule broke tarball releases (cause they miss rimage entirely)

I did some research and found a couple ways to fix this but no simple and easy way. Long story short some sort of script that iterates on the submodule(s) is required. There is already a couple of "git-archive-all" scripts here and there already but nothing "standard" and certainly nothing built in git; so this would be a new/extra dependency and not necessarily a well maintained one.

The other packaging option I considered is CMake's CPack. It's standard in CMake and should do the job, however it's wildcard-based and not version control based and requires a pristine checkout. Mmmm....

BTW I got confirmation on Slack from @mbolivar himself (thx) that west (used by Zephyr instead of submodules) doesn't support source releases more than git submodules do. In other words, any west project that wants to create a source tarball has to iterate on the list of git clones too. This feature has never been requested in west - another demonstration very few people still care about release tarballs?

In conclusion I think doing nothing and waiting for someone to ask is probably a good option because:

• source tarballs seem to have few users left
• those few tarball users left can perfectly treat rimage as a standalone project and external dependency, which is more or less the point of a submodule. I made sure "extracting" the rimage source code inside sof-v1.2.3/ Just Works. A symbolic link to wherever works too. Missing rimage produces a very good and prompt "rimage not found" error message (only the suggestion to use git submodules becomes off the mark)

We could even drop the symbolic link thanks to some new cmake -DRIMAGE_LOCATION=~/rimage CMake option if desired; should be easy enough.

[lgirdwood]

@marc-hb ack - I've no objection to dropping tarball support.

[marc-hb]

Thanks! To be shorter and hopefully clearer: as long as you manually re-add the rimage/ directory yourself, the source tarball still works perfectly fine.

[marc-hb]

First submodule "usability" issue, I'm surprised it took that long: #3200

[plbossart]

well, I broke it, but it's not like I didn't warn folks that git submodules were the worst idea since ClearCase: #2086 (comment)

[marc-hb]

Never replace a directory with a submodule of the same name because this creates a "flag day" that breaks git checkout, git bisect and upsets everyone including @plbossart. Demonstration:

cd sof/
git stash
git checkout bf795c5b249d
error: The following untracked working tree files would be overwritten by checkout:
	rimage/CMakeLists.txt
Please move or remove them before you switch branches.
Aborting

This would not happen if this rimage submodule had been created with a different name; for instance rimage-sub or submodules/rimage. Anything but a name already used in the past.

It's too late now but IMHO good to know with submodules in general which is why I'm posting this comment.

In related news I'm proposing to drop submodules in SOF and use West instead, thx for discussing the proposal at

https://github.com/orgs/thesofproject/teams/sof-developers/discussions/46

... which is not public, so transferred to issue #3517 instead.

[marc-hb]

This is a great idea to have the rImage as an indepent image build tool for DSPs. But in the current form it still depends on the SOF code which is just specific use, one of many others. The rimage code (the one in dedicated repo) still includes SOF headers to produce additional SOF specific files. Any change in this area in SOF requires sync'ed changes in rimage which is not acceptable if rimage is supposed to be independent. Therefore one piece is still missing: some form of config files defining maps between specific FW parts of elf-s (like SOF) and the specific FW's output files, where the configs are maintained by the specific FWs inside their code trees. Technically, any include of specific FW header files + FW specific code left in rImage means that it cannot be safely separated as independent tool.

Suggestion to more rimage back to sof.git:

• [FEATURE] remove SOF build time dependency to rimage headers #8178