Skip to content

Conversation

@aborisovich
Copy link
Contributor

@aborisovich aborisovich commented May 18, 2023

Updated Zephyr workflow - installed new Zephyr SDK v0.16.1 that is required by Zephyr main branch.
New Zephyr SDK is NOT backward compatible to older Zephyr revisions so the upgrade in west.yml is done to the commit: zephyrproject-rtos/zephyr@9fc99928caf0 that requires this toolchain.
Build-windows job changes:

  • new Zephyr SDK had changed extension and now is compressed using 7z format.
  • from build-windows workflow removed unzip package that is no longer required by the setup.cmd Zephyr SDK installation script

Build-linux job changes:

  • updated script zephyr\docker-run.sh that clones Zephyr Docker container to clone latest tag versioned v0.26.4.
  • new container contains Zephyr SDK v0.16.1
  • upgraded runner OS to Ubuntu 22.04 to match OS from Docker container

Sparse-Zephyr workflow:

  • upgraded runner OS to Ubuntu 22.04 to match OS from the Zephyr workflow.

Signed-off-by: Andrey Borisovich andrey.borisovich@intel.com

@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from 89f0399 to 2aff07a Compare May 18, 2023 16:27
@aborisovich aborisovich changed the title Upgrade zephyr sdk workflows: Zephyr build-windows uses new 0.16.1 Zephyr SDK May 18, 2023
@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from 2aff07a to 05e99c4 Compare May 18, 2023 17:19
@aborisovich aborisovich changed the title workflows: Zephyr build-windows uses new 0.16.1 Zephyr SDK [DNM] workflows: Zephyr build-windows uses new 0.16.1 Zephyr SDK May 18, 2023
@aborisovich aborisovich linked an issue May 18, 2023 that may be closed by this pull request
3 tasks
@aborisovich aborisovich marked this pull request as ready for review May 18, 2023 17:25
@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from 05e99c4 to e9c3fbe Compare May 18, 2023 17:35
@aborisovich aborisovich changed the title [DNM] workflows: Zephyr build-windows uses new 0.16.1 Zephyr SDK [DNM] workflows: Zephyr workflow uses new 0.16.1 Zephyr SDK May 18, 2023
@aborisovich
Copy link
Contributor Author

@marc-hb we got all green on the Zephyr workflows.
Internal Intel CI had not finished yet but I think no point waiting for those.
This was with west.yml set to Zephyr main branch (what checks build-windows with zmain revision).
Dropping DNM and zmain revision from west.yml, let's see whether current SOF manifest rev works ok with new SDK.

@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from e9c3fbe to 7162f04 Compare May 18, 2023 17:55
@aborisovich aborisovich changed the title [DNM] workflows: Zephyr workflow uses new 0.16.1 Zephyr SDK workflows: Zephyr workflow uses new 0.16.1 Zephyr SDK May 18, 2023
Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

Looks great, thank you! Very glad it seems to have gone smoothly.

Just to be on the safe side please add this:

--- a/.github/workflows/sparse-zephyr.yml
+++ b/.github/workflows/sparse-zephyr.yml
@@ -15,9 +15,9 @@ jobs:
   # sof/scripts/parse_sparse_output.sh
   warnings-subset:
 
-    # We're sharing binaries with the zephyr-build container so keep
+    # We're sharing the sparse binary with the zephyr-build container so keep
     # this in sync with it.
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
 
     strategy:
       fail-fast: false

@aborisovich
Copy link
Contributor Author

Ok, upgraded OS for Sparse Zephyr workflow to Ubuntu 22,04.
Looks like we are good to go if all checks are green.

@aborisovich
Copy link
Contributor Author

Wow - codestyle / checkpatch failed with the following message:

WARNING: A patch subject line should describe the change not the tool that found it
#4:
Subject: [PATCH] workflows: Sparse Zephyr upgraded runner OS to Ubuntu 22.04

I sometimes think this checkpatch is chatGTP powered or something...

@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from ec3c640 to 9a878e3 Compare May 18, 2023 18:18
@plbossart
Copy link
Member

Wow - codestyle / checkpatch failed with the following message:

WARNING: A patch subject line should describe the change not the tool that found it
#4:
Subject: [PATCH] workflows: Sparse Zephyr upgraded runner OS to Ubuntu 22.04

I sometimes think this checkpatch is chatGTP powered or something...

it's been that way for a very long time...

@marc-hb
Copy link
Collaborator

marc-hb commented May 18, 2023

checkpatch.pl is a Linux kernel tool. The Linux kernel tool has no CI scripts or configuration in the same git repo, nothing like the .github/ directory or similar. So these false positives are rare in the kernel.

While this warning is wrong in this case, it is unfortunately very common to see people submitting bad commit messages like "fix warning from tool X" without describing what the actual code change is. Then this warning is very useful.

@marc-hb
Copy link
Collaborator

marc-hb commented May 18, 2023

Also: https://www.kernel.org/doc/html/latest/dev-tools/checkpatch.html

Checkpatch is not always right. Your judgement takes precedence over checkpatch messages. If your code looks better with the violations, then its probably best left alone.

False positives are OK because checkpatch only check PATCHES which means warnings do NOT stick after merge like with other linters.

@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from 9a878e3 to dfcfd35 Compare May 18, 2023 18:47
@aborisovich
Copy link
Contributor Author

Ok, upgraded Zephyr revision in west.yml. We should be good to go now, however we need to wait for Intel Internal CI results to make sure we have no regression introduced.

@marc-hb
Copy link
Collaborator

marc-hb commented May 18, 2023

Thanks again @aborisovich , this solves a big problem!

Something's not right in
https://github.com/thesofproject/sof/actions/runs/5017216786/jobs/8995052856?pr=7645, sparse compilation still looks for the old toolchain. I will look at it after a few meetings.

CMake Error at /zep_workspace/zephyr/cmake/compiler/gcc/target.cmake:12 (message):
  C compiler
  /home/cwd_user/zephyr-sdk-0.16.1/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc
  not found - Please check your toolchain installation

EDIT: it's not sparse, it's everything MTL: https://github.com/thesofproject/sof/actions/runs/5017216783/jobs/8995053803?pr=7645

@marc-hb
Copy link
Collaborator

marc-hb commented May 18, 2023

I will look at it after a few meetings.

OK this didn't take long: @aborisovich can you please bump sof/west.yml a bit further to zephyrproject-rtos/zephyr@9fc99928caf0 ? We need that one too for MTL

however we need to wait for Intel Internal CI results to make sure we have no regression introduced.

Note the Zephyr SDK is NOT used in sof-ci/jenkins, XCC is used instead. However the Zephyr uprev can of course make a difference! Speaking of which, TGLU_UP_HDA_IPC4ZPH looks pretty bad in https://sof-ci.01.org/sofpr/PR7645/build8107/devicetest/index.html :-( No idea why yet.

https://sof-ci.01.org/sofpr/PR7645/build8106/devicetest/index.html is mostly fine.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @aborisovich !

@kv2019i
Copy link
Collaborator

kv2019i commented May 19, 2023

The manifest mtl fails to

https://github.com/thesofproject/sof/actions/runs/5017216783/jobs/8995053803?pr=7645
/home/cwd_user/zephyr-sdk-0.16.1/xtensa-intel_s1000_zephyr-elf/bin/xtensa-intel_s1000_zephyr-elf-gcc

While main mtl test passes and correct compiler is found:

https://github.com/thesofproject/sof/actions/runs/5017216783/jobs/8995053803?pr=7645
-- Found assembler: /home/cwd_user/zephyr-sdk-0.16.1/xtensa-intel_ace15_mtpm_zephyr-elf/bin/xtensa-intel_ace15_mtpm_zephyr-elf-gcc

Why is this happening, the PR is updating the manifest, right...?

@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from dfcfd35 to df43f1e Compare May 19, 2023 10:35
Andrey Borisovich added 6 commits May 19, 2023 12:38
Commit zephyrproject-rtos/zephyr@9fc99928caf0
requires new Zephyr SDK 0.16.1 to work and breaks compatibility to
older Zephyr revisions.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
Zephyr main branch requires new Zephyr SDK.
Upgraded version of Zephyr SDK to newest available v0.16.1 in the
build-windows job. New SDK is backward compatible with old Zephyr
revisions so the upgrade is applied to all CI.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
Installation of new Zephyr SDK 0.16.1 does not require unzip anymore
in the setup.cmd script. This tool had been replaced by 7z that is
by default present on all Github Windows runners.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
New Docker container tagged v0.26.4 contains new Zephyr SDK v0.16.1
needed to build with Zephyr main branch.
New Zephyr SDK is backward compatible with the older Zephyr revisions
so the upgrade is done for SOF manifest revisions too.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
New Zephyr Docker container v.26.4 is based on Ubuntu 22.04,
upgrading build-linux job OS to match the one in the container.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
Runner OS for the Zephyr workflow had been updated to Ubuntu 22.04,
so upgrading this workflow too.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
@aborisovich aborisovich force-pushed the upgrade-zephyr-sdk branch from df43f1e to 52c0502 Compare May 19, 2023 10:38
@aborisovich
Copy link
Contributor Author

All green now, thanks @marc-hb .
Let's wait for Intel Internal CI results and merge if positive.

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

Great job, thanks a lot @aborisovich ! Please merge this first thing on Monday, thanks!

https://sof-ci.01.org/sofpr/PR7645/build8158/devicetest/index.html?model=TGLU_RVP_NOCODEC_IPC4ZPH&testcase=multiple-pause-resume-5 has the known corrupted heap issue 7191

https://sof-ci.01.org/sofpr/PR7645/build8157/devicetest is all green.

Checkpatch is wrong.

Everything else is green!

@kv2019i kv2019i merged commit 38f3f5d into thesofproject:main May 19, 2023
@aborisovich aborisovich deleted the upgrade-zephyr-sdk branch June 11, 2023 15:31
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.

Zephyr main branch requires 0.16.x SDK (Intel toolchain change)

5 participants