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

CI: Pin to specific version of Zephyr SDK in default PRs #295

Closed
wmamills opened this issue May 5, 2024 · 6 comments · Fixed by #307
Closed

CI: Pin to specific version of Zephyr SDK in default PRs #295

wmamills opened this issue May 5, 2024 · 6 comments · Fixed by #307

Comments

@wmamills
Copy link
Contributor

wmamills commented May 5, 2024

Unlike open-amp CI, this script uses the very latest version of the Zephyr SDK on each PR check.
This is bad. While it is good to test against the latest SDK, that should not be mixed into checking a PR.

The default PR should use a pinned version of the SDK (and the same one as open-amp).

It would be good to have separate CI jobs that would test a known good library (current main should be fine) against the latest Zephyr SDK, latest Zephyr, latest Ubuntu etc

@wmamills
Copy link
Contributor Author

wmamills commented May 6, 2024

@arnopo what do you want to do about this? Right now libmetal is using 0.16.6 for CI and can change at any time. open-amp is using 0.16.1 (which is old but at least a fixed version.)

@arnopo
Copy link
Contributor

arnopo commented May 6, 2024

The answer is not simple.

  • if we fix the SDK and the Zephyr version. PRs related to a Zephyr update could fail as not build in a up to date environment. And we can miss some new Zephyr update in Zephyr that breaks compatibility.
  • if we keep like this drawback is that CI breaks each time we have an SDK update.

I would say I would prefer to see the SDK issue ( CI is broken around 2 times /years) than miss something that would be discover during the release or after the release.

If the hypothesis is that the Zephyr build with lastest SDK with the main branch, what would be nice is to find a way to not have the SDK version hardcoded but pointing to the lastest version.

@wmamills
Copy link
Contributor Author

wmamills commented May 6, 2024

Yes I would setup up a separate job to test against bleeding edge Zephyr. Not force individual contributors to deal with it.
We could automate the "know good" update but manually bumping it a few times a years seems OK to me.

Copy link

This issue has been marked as a stale issue because it has been open (more than) 45 days with no activity.

@github-actions github-actions bot added the Stale label Jun 21, 2024
@wmamills
Copy link
Contributor Author

This issue caused problems again in PR #302 .
If CI randomly fails for reasons unrelated to the CI, it just teaches people to ignore the CI results.
Alternatively if Upstream Zephyr has a breaking change and we have no PRs, we don't know about it until the next PR.

Testing libmetal against latest Zephyr upstream should be its own job and should be run weekly using libmetal main.
Testing in PRs and merges should be against a known good version of Zephyr and Zephyr SDK.

@arnopo
Copy link
Contributor

arnopo commented Aug 26, 2024

This issue caused problems again in PR #302 . If CI randomly fails for reasons unrelated to the CI, it just teaches people to ignore the CI results. Alternatively if Upstream Zephyr has a breaking change and we have no PRs, we don't know about it until the next PR.

Testing libmetal against latest Zephyr upstream should be its own job and should be run weekly using libmetal main. Testing in

Seems reasonable. What I cannot understand is why it fails with Zephyr 3.7.99 as this is the last release of zephyr.

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 a pull request may close this issue.

2 participants