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

list_hardware.py: sort rglob(SOC_YML) HWMv2 results #70132

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Mar 12, 2024

This makes .config, autoconf.h, and configs.c deterministic again.

Directory listing is not deterministic, it must always be sorted. https://reproducible-builds.org/docs/stable-inputs/

Fixes commit 61bbfb5 ("scripts: introduce list_hardware.py for listing of architectures and SoCs") in collab-hwm branch which was squashed in mega HWMv2 commit 8dc3f85 ("hwmv2: Introduce Hardware model version 2 and convert devices")

SOF CI builds with both Windows and Linux and compares the outputs. This catches practically 100% of build reproducibility issues and caught this one too:

https://github.com/thesofproject/sof/actions/runs/8241692987/job/22539664560

HWMv2 was a "big bang" integration on both the Zephyr and SOF sides. So this rglob() was a needle in a haystack but with hindsight, this issue is really trivial to reproduce and verify:

apt-get install disorderfs
mkdir disorderedWorkspace/
disorderfs --shuffle-dirents=yes  workspace/ disorderedWorkspace/

... then just build samples/hello_world/ twice in disorderedWorkspace/ with any --board and compare the build directories.

cc:

This makes .config, autoconf.h, and configs.c deterministic again.

Directory listing is not deterministic, it must always be sorted.
https://reproducible-builds.org/docs/stable-inputs/

Fixes commit 61bbfb5 ("scripts: introduce list_hardware.py for
listing of architectures and SoCs") in collab-hwm branch which was
squashed in mega HWMv2 commit 8dc3f85 ("hwmv2: Introduce Hardware
model version 2 and convert devices")

SOF CI builds with both Windows and Linux and compares the outputs. This
catches practically 100% of build reproducibility issues and caught this
one too:

 https://github.com/thesofproject/sof/actions/runs/8241692987/job/22539664560

HWMv2 was a "big bang" integration on both the Zephyr and SOF sides. So
this `rglob()` was a needle in a haystack but with hindsight, this issue
is really trivial to reproduce and verify:

```
apt-get install disorderfs
mkdir disorderedWorkspace/
disorderfs --shuffle-dirents=yes  workspace/ disorderedWorkspace/
```

... then just build `samples/hello_world/` twice in disorderedWorkspace/
with any --board and compare the build directories.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb added a commit to marc-hb/sof that referenced this pull request Mar 13, 2024
@marc-hb marc-hb marked this pull request as ready for review March 13, 2024 00:09
@zephyrbot zephyrbot added the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Mar 13, 2024
@marc-hb marc-hb added bug The issue is a bug, or the PR is fixing a bug area: Build System area: Kconfig HWMv2 labels Mar 13, 2024
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 13, 2024

@tejlmand : based on the git blame you probably want to add yourself (and others?) to the MAINTAINERS.yml section for this file; and maybe for some other HWMv2 files too. The "Pull Request" assigner found no reviewer and no assignee.

@marc-hb marc-hb requested a review from tejlmand March 13, 2024 00:13
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 13, 2024

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm, and I agree build reproducibility is very important and in this case trivial to fix.

But i'm a little worried if we suddenly start to require that every time we search for files (or create any other kind of list) that we start to require sorting of such lists.

@fabiobaltieri fabiobaltieri merged commit 251f52c into zephyrproject-rtos:main Mar 13, 2024
33 checks passed
@marc-hb marc-hb deleted the sorted-list-hardware branch March 13, 2024 17:06
@marc-hb
Copy link
Collaborator Author

marc-hb commented Mar 20, 2024

But i'm a little worried if we suddenly start to require that every time we search for files (or create any other kind of list) that we start to require sorting of such lists.

I'm afraid this is a price to pay. Directory listing has never been deterministic and I don't see this changing any time soon. The good news is: it's a relatively small price to pay. The hardest part is to automate the testing in CI but SOF already has that thanks to Windows builds.

Zephyr came close but you pushed back here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System area: Kconfig bug The issue is a bug, or the PR is fixing a bug HWMv2 Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants