Skip to content

[SKIP CI] .github/zephyr: compare Windows and Linux builds - #6950

Merged
kv2019i merged 3 commits into
thesofproject:mainfrom
marc-hb:compare-win
Jan 23, 2023
Merged

[SKIP CI] .github/zephyr: compare Windows and Linux builds#6950
kv2019i merged 3 commits into
thesofproject:mainfrom
marc-hb:compare-win

Conversation

@marc-hb

@marc-hbmarc-hb commented Jan 13, 2023•
edited
Loading

Copy link
Copy Markdown
Collaborator

The End.

This makes sure Windows builds and Linux builds produce identical binaries.

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

@marc-hb
marc-hbforce-pushed the compare-win branch 6 times, most recently from a32c62e to 8b74913CompareJanuary 13, 2023 06:08
@marc-hb

marc-hb commented Jan 13, 2023•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Firmware differences between Linux and Windows found in https://github.com/thesofproject/sof/actions/runs/3908863755/jobs/6679699349 should be all fixed by #6920

@marc-hb
marc-hb marked this pull request as ready for review January 13, 2023 06:43
@marc-hb
marc-hb marked this pull request as draft January 13, 2023 06:43
@lgirdwood

lgirdwood commented Jan 18, 2023

Copy link
Copy Markdown
Member

Firmware differences between Linux and Windows found in https://github.com/thesofproject/sof/actions/runs/3908863755/jobs/6679699349 should be all fixed by #6920

@marc-hb#6920 now merged.

@marc-hb

marc-hb commented Jan 18, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Windows and Linux have a different zephyr git describe and BUILD_VERSION in
https://github.com/thesofproject/sof/actions/runs/3951969148/jobs/6766482099

Most likely my fault: commit 68b49c1

marc-hb added a commit to marc-hb/sof that referenced this pull request Jan 18, 2023
@marc-hb
scripts/xtensa-build-zephyr.py: checksum [sof_]version.h files
f493010
`git describe` differences happen, especially when trying to optimize
cloning and this is for instance what happened in
thesofproject#6950 (comment)
where zephyr tags were fetched on Linux but not on Windows
This also makes plain git version differences more obvious; no need to
scroll all the way up and scan the build logs.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hbmarc-hb mentioned this pull request Jan 18, 2023
Merged
@marc-hb
marc-hbforce-pushed the compare-win branch 2 times, most recently from ac6c555 to acc1433CompareJanuary 18, 2023 22:35
@marc-hb
.github/zephyr: align git fetch and describe across Windows and Linux
6275448
Fetching different tags causes `git describe` output to be different.
Fixes commit 68b49c1 (".github/zephyr: switch Windows build to west
update --narrow")
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
marc-hbforce-pushed the compare-win branch 5 times, most recently from deb3252 to 81c30deCompareJanuary 19, 2023 02:28
@marc-hb
marc-hb marked this pull request as ready for review January 19, 2023 02:45
@marc-hb

marc-hb commented Jan 19, 2023•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

All tests passing now. Binaries generated by Windows and Linux are compared and found identical in https://github.com/thesofproject/sof/actions/runs/3954767889/jobs/6772604198

EDIT: and again in newer https://github.com/thesofproject/sof/actions/runs/3963690378/jobs/6791978755

@aborisovich

aborisovich commented Jan 20, 2023

Copy link
Copy Markdown
Contributor

Windows and Linux have a different zephyr git describe and BUILD_VERSION in https://github.com/thesofproject/sof/actions/runs/3951969148/jobs/6766482099

Most likely my fault: commit 68b49c1

I remember that I did not made --narrow there during Windows CI implementation deliberately...

aborisovich
aborisovich suggested changes Jan 20, 2023
View reviewed changes

@aborisovichaborisovich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great @marc-hb, I'm really glad the output we have on both OSes is the same.
Found a typo, please fix. And some minor things

Comment thread.github/workflows/zephyr.yml Outdated
Comment thread.github/workflows/zephyr.yml Outdated
Comment threadzephyr/scripts/clean-expected-release-differences.sh Outdated
marc-hb
marc-hb commented Jan 20, 2023
View reviewed changes

@marc-hbmarc-hb left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Thanks for the review, see answers below.

Comment thread.github/workflows/zephyr.yml Outdated
Comment thread.github/workflows/zephyr.yml Outdated
Comment threadzephyr/scripts/clean-expected-release-differences.sh Outdated
@marc-hb
.github/zephyr: compare Windows and Linux builds
f8530e4
The End.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
xtensa-build-zephyr.py: require zephyr.strip
4e8be20
Make CONFIG_BUILD_OUTPUT_STRIPPED mandatory so we can always compare
builds. It makes practically zero build space and time difference and
has huge reproductibility value, see discussion in
zephyrproject-rtos/zephyr#51954
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@kv2019i
kv2019i merged commit cc00c57 into thesofproject:mainJan 23, 2023
@marc-hb
marc-hb deleted the compare-win branch January 25, 2023 01:39
kv2019i pushed a commit that referenced this pull request Jan 26, 2023
@marc-hb@kv2019i
scripts/xtensa-build-zephyr.py: checksum [sof_]version.h files
f85320b
`git describe` differences happen, especially when trying to optimize
cloning and this is for instance what happened in
#6950 (comment)
where zephyr tags were fetched on Linux but not on Windows
This also makes plain git version differences more obvious; no need to
scroll all the way up and scan the build logs.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kv2019ikv2019ikv2019i approved these changes
@dbalutadbalutaAwaiting requested review from dbalutadbaluta is a code owner
@cujomalaineycujomalaineyAwaiting requested review from cujomalainey
@lgirdwoodlgirdwoodAwaiting requested review from lgirdwoodlgirdwood is a code owner
@paulstelian97paulstelian97Awaiting requested review from paulstelian97
@miRooxmiRooxAwaiting requested review from miRoox
@aiChaoSONGaiChaoSONGAwaiting requested review from aiChaoSONG
@andyrossandyrossAwaiting requested review from andyross
@dcpleungdcpleungAwaiting requested review from dcpleung
@greg-intelgreg-intelAwaiting requested review from greg-intel
@keqiaozhangkeqiaozhangAwaiting requested review from keqiaozhang
@MaureenHelmMaureenHelmAwaiting requested review from MaureenHelm
+1 more reviewer
@aborisovichaborisovichaborisovich approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@marc-hb@lgirdwood@aborisovich@kv2019i