Skip to content
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

xtensa-build-zephyr.py: make --deployable-build the default #8781

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Jan 23, 2024

[OLD commit message]

Everyone should use deployable builds by default.


[NEW commit message]

Every Linux developer should use deployable builds by default.

Until Peter Ujfalusi's very recent work in this script, we had a
complete /lib/firmware/ structure disconnect between the IPC4 output
of this script and the IPC4 expectations of the Linux kernel. To
workaround this disconnect, every CI and Linux developer used to
implement duplicate and inconsistent firmware deployment hacks.

People crafting sof-bin releases also had to organize IPC4 releases
manually, which was extremely error-prone and with limited test
coverage (Thanks Kai and Mengdong!)

Now that Peter gracefully fixed the layout, documented it in sof-docs
and implemented it in this script, the time for all Linux developers to
drop their inconsistent deployment hacks is overdue. All these hacks
must be replaced with a simple, one-line recursive copy which makes sure
the layout committed in version control is constantly tested by
everyone.

So, make deployable builds the new default.

The new default will also help with sof-bin releases, making sure they
use a well tested /lib/firmware/ layout.

The --no-deployable-build was recently introduced to help minimize
disruption and migration effort for people and automation who do NOT use
Linux. The /lib/firmware/ directory structure is irrelevant outside
Linux (but everyone is of course free to choose it)

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

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 24, 2024

@lrudyX , @wszypelt , all tests in https://sof-ci.01.org/sof-pr-viewer/#/build/PR8781/build13454079 failed like this:

E FileNotFoundError: [Errno 2] No such file or directory:
'C:\\Testing\\Builds\\pull-8781-merge_1931053\\
   cSOF_zephyr\\LNL\\build-sof-staging\\sof\\community\\sof-lnl.ri'

Could you please add the new --no-deployable-build option where appropriate? This would make QB ignore this PR and stick to the older behavior. It can be done before this PR is merged, --no-deployable-build is already available now. This PR only flips the default value.

@wszypelt
Copy link

Hi @marc-hb first thing is the link you provide shows me a blank page
(https://sof-ci.01.org/sof-pr-viewer/#/build/PR8781/build13454079)
as for the "--no-deployable-build option" i need to discuss it with the team

@abonislawski
Copy link
Member

If everyone should use deployable build then why QB shouldn't?

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 24, 2024

If everyone should use deployable build then why QB shouldn't?

I'm fine with that too but I expect that to take much more work and take much longer. The "deployable" layout matters only when you run a Linux kernel and these tests do not.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 24, 2024

Hi @marc-hb first thing is the link you provide shows me a blank page
(https://sof-ci.01.org/sof-pr-viewer/#/build/PR8781/build13454079)

You need to extract the number 13454079 and re-use it on the internal site. We lost the public export of these results a long time ago unfortunately.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@marc-hb can you say why everyone should use and any possible exceptions. Thanks.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 24, 2024

@marc-hb can you say why everyone should use and any possible exceptions.

I just increased the commit message length from 1 line to 28 lines. Hope this clarifies!

@wszypelt
Copy link

@marc-hb --no-deployable-build option added where appropriate. PR passed all tests

@marc-hb marc-hb marked this pull request as ready for review January 29, 2024 15:41
@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 29, 2024

Just one suspend/resume failure in https://sof-ci.01.org/sofpr/PR8781/build2242/devicetest/index.html and one pause resume "MAX" failure in https://sof-ci.01.org/sofpr/PR8781/build2241/devicetest/index.html

zephyr main branch incompatibility is known.

Nothing related to this PR, good to merge.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 30, 2024

After second thought, back to draft until the current regressions and zephyr updates are resolved (#8818). Enough moving pieces already.

@marc-hb marc-hb marked this pull request as draft January 30, 2024 07:25
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 6, 2024

SOFCI TEST

@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 7, 2024

Postponing again until after the latest, HWMv2 hurdle #8913

@marc-hb marc-hb marked this pull request as ready for review March 8, 2024 16:47
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 8, 2024

@marc-hb marc-hb marked this pull request as draft March 8, 2024 23:14
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 15, 2024

SOFCI TEST

EDIT: only one known pause-resume failure in https://sof-ci.01.org/sofpr/PR8781/build3472/devicetest/index.html

https://sof-ci.01.org/sofpr/PR8781/build3473/devicetest/index.html is almost finished and looking great so far.

@marc-hb marc-hb marked this pull request as ready for review March 15, 2024 18:05
@marc-hb marc-hb requested a review from lyakh March 15, 2024 18:05
Every Linux developer should use deployable builds by default.

Until Peter Ujfalusi's very recent work in this script, we had a
complete `/lib/firmware/` structure disconnect between the IPC4 output
of this script and the IPC4 expectations of the Linux kernel. To
workaround this disconnect, every CI and Linux developer used to
implement duplicate and inconsistent firmware deployment hacks.

People crafting sof-bin releases also had to organize IPC4 releases
manually, which was extremely error-prone and with limited test
coverage (Thanks Kai and Mengdong!)

Now that Peter gracefully fixed the layout, documented it in sof-docs
and implemented it in this script, the time for all Linux developers to
drop their inconsistent deployment hacks is overdue. All these hacks
must be replaced with a simple, one-line recursive copy which makes sure
the layout committed in version control is constantly tested by
everyone.

So, make deployable builds the new default.

The new default will also help with sof-bin releases, making sure they
use a well tested /lib/firmware/ layout.

The --no-deployable-build was recently introduced to help minimize
disruption and migration effort for people and automation who do NOT use
Linux. The `/lib/firmware/` directory structure is irrelevant outside
Linux (but everyone is of course free to choose it)

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 15, 2024

Only one known and totally unrelated failure at https://sof-ci.01.org/sofpr/PR8781/build3478/devicetest/index.html?model=MTLP_RVP_NOCODEC&testcase=multiple-pause-resume-50 , all the rest of that test plan is green.

Only one, totally unrelated suspend/resume failure in https://sof-ci.01.org/sofpr/PR8781/build3479/devicetest/index.html

quickbuild is still happy in https://sof-ci.01.org/sof-pr-viewer/#/build/PR8781/build13727404

Very small change, good to merge.

@kv2019i
Copy link
Collaborator

kv2019i commented Apr 2, 2024

@ujfalusi @lyakh Can you check? We'd need on more approval, this is ready to go.

@kv2019i kv2019i merged commit 9d7c33a into thesofproject:main Apr 2, 2024
41 of 43 checks passed
@marc-hb marc-hb deleted the deployable-by-default branch April 3, 2024 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants