-
Notifications
You must be signed in to change notification settings - Fork 349
[SKIP CI] .github: add zephyr build #4114
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
Conversation
It still looks smart but the \n addition compared to the original
version in xtensa-build-all.sh broke it for more advanced cases:
die '%s %d' str 5
-bash ERROR: -bash: printf: 5\n: invalid number
As reported by shellcheck. shellcheck saves lives.
In scripts/xtensa-build-zephyr.sh line 22:
>&2 printf "$@\n"
^-- SC2145: Argument mixes string and array.
Use * or separate argument.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
This script lives in a sof.git/ clone yet it was systematically cloning a second sof.git/. Besides the obvious confusion and risk of editing the wrong files, this meant it was not possible to build code that has not been merged yet! This was a problem for both CI and developers. Fixed by using symbolic links to ourselves instead. Note it is _still_ possible to build from another sof.git clone if desired, however this script will never git re-clone a second sof.git itself, that second clone has to be created (e.g.: by west) before this script runs. When cloning a brand new zephyrproject, use a shallow zephyr clone and download only the two zephyr modules we actually use. This speeds up automation considerably and makes it much faster for non-Zephyr developers to reproduce Zephyr issues. Developers can always git unshallow and west update once if they want to. Rename the default west top to "zephyrproject" to not just match the zephyr documentation but to also avoid creating a double zephyr/zephyr/ directory. See the new print_usage() for a few more implementation details. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
f4f36ed to
eef0947
Compare
|
The checktree failure in https://github.com/thesofproject/sof/pull/4114/checks?check_run_id=2462963981 has been introduced by #4111, it's unrelated |
|
LGTM to me, but wont approve as my bash is sub par |
zephyr/docker-build.sh
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Submitted libssl-dev addition to the docker image in zephyrproject-rtos/docker-image#58
While not always easy to write, I believe all shell scripts should be mostly readable even by beginners. For every cryptic shell "trick" there's hopefully a comment next to it. I hope you would have asked if anything had confused you. PS: shellcheck is an outstanding teacher. I mean for a program. |
This should be fixed with: #4116 |
lgirdwood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @marc-hb this was really needed.
|
SOFCI TEST |
Yes i usually get the gist, ive read most of the way through my linux phrase book which is all bashism's for the most part, but study and practice are two very different levels of knowledge :) |
Use the official image from the Zephyr project https://github.com/zephyrproject-rtos/docker-image Signed-off-by: Marc Herbert <marc.herbert@intel.com>
I believe this only works for jenkins which I disabled with Q&A related to this PR: zephyrproject-rtos/zephyr#34713 |
|
Thanks @marc-hb. |
3 commits, see messages, especially:
xtensa-build-zephyr: do not clone a second version of sof.gitRelated rimage.h background info: zephyrproject-rtos#7 (comment)