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

makefiles/docker.inc.mk: bump riotbuild docker image to last available #20840

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

mguetschow
Copy link
Contributor

Contribution description

This includes RIOT-OS/riotdocker#245 and RIOT-OS/riotdocker#241.

The hashes were obtained with dist/tools/buildsystem_sanity_check/check.sh.

Testing procedure

CI should not complain.

Issues/PRs references

See #20472 for the PR that introduced the pinning mechanism.

@github-actions github-actions bot added the Area: build system Area: Build system label Aug 28, 2024
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 28, 2024
@maribu maribu enabled auto-merge August 28, 2024 13:53
@riot-ci
Copy link

riot-ci commented Aug 28, 2024

Murdock results

✔️ PASSED

771c546 pkg/emlearn: add patch to remove redundant check

Success Failures Total Runtime
10192 0 10193 13m:48s

Artifacts

@maribu
Copy link
Member

maribu commented Aug 28, 2024

@mguetschow: It seems that the emlearn test is using some pickled data where the data was serialized with a incompatible version of emlearn (or its dependencies), so that deserialization fails. In fact, the previous docker version already complained about the version mismatch, but we apparently got lucky and the data was still compatible.

I wonder if we should just merge #20841 and let actual users of emlearn to worry about having a version of emlearn (or its deps) installed that is compatible with the models they use.

@benpicco
Copy link
Contributor

Are you sure it only requires the header?
There is also a python library that appears to be incompatible with the test in master - I wonder if this PR needs to include #20347 in order to pass CI.

@mguetschow
Copy link
Contributor Author

Are you sure it only requires the header? There is also a python library that appears to be incompatible with the test in master - I wonder if this PR needs to include #20347 in order to pass CI.

Yes, I can confirm that the header is enough. The python library in riotbuild is now incompatible with the model test in master, since that one uses the python library to generate model.h.

Including #20347 would be another fix, but that would leave the interdependencies between RIOT master and the python package versions in riotbuild in place for the future.

@maribu
Copy link
Member

maribu commented Aug 28, 2024

Yeah, let's try cherry-picking #20347 instead.

Adding the model.h to the repo is something we can still do afterwards avoid similar issues in the feature, as updating emlearn will then be possible without touching the CI container.

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: pkg Area: External package ports labels Aug 29, 2024
@mguetschow
Copy link
Contributor Author

Yeah, let's try cherry-picking #20347 instead.

Done, this includes #20347 now.

@maribu maribu enabled auto-merge August 29, 2024 09:26
@maribu maribu added this pull request to the merge queue Aug 29, 2024
Merged via the queue into RIOT-OS:master with commit b469a52 Aug 29, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: pkg Area: External package ports Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants