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

RFC: cmake: limit Zephyr SDK to 0.16.x series for Zephyr 3.7 LTS #80195

Merged

Conversation

tejlmand
Copy link
Collaborator

@tejlmand tejlmand commented Oct 22, 2024

Zephyr SDK compatibility level 0.16 is now an LTS intended for
Zephyr v3.7 LTS.

Thus, limit the upper Zephyr SDK lookup to <0.17, so that developers
working with Zephyr v3.7 LTS and latest Zephyr main can install both
Zephyr SDK 0.16.x along side Zephyr SDK >=0.17 and have the Zephyr SDK
0.16.x series being used for v3.7 LTS development.


This PR depends on a manual backport of #80194 which is included in this PR.
Fixes: #80200

@tejlmand tejlmand added the DNM This PR should not be merged (Do Not Merge) label Oct 22, 2024
@tejlmand tejlmand changed the title cmake: limit Zephyr SDK to 0.16.x series for Zephyr 3.7 LTS RFC: cmake: limit Zephyr SDK to 0.16.x series for Zephyr 3.7 LTS Oct 22, 2024
@tejlmand
Copy link
Collaborator Author

@stephanosio @nashif @keith-packard based on zephyrproject-rtos/sdk-ng#809 (comment) I think it makes sense to place an upper limit on the Zephyr SDK, as we now consider the Zephyr SDK 0.16.x series an LTS.

Should we bring this to the TSC ?

@stephanosio
Copy link
Member

based on zephyrproject-rtos/sdk-ng#809 (comment) I think it makes sense to place an upper limit on the Zephyr SDK, as we now consider the Zephyr SDK 0.16.x series an LTS.

Yes, thanks for implementing this.

Though, ultimately, we would want find_package(Zephyr-sdk a.b) to only look for the SDKs versions a.b.X as per the semantic versioning scheme discussed in zephyrproject-rtos/sdk-ng#608 -- this way, we do not have to make changes like this to previous Zephyr releases in the future.

Should we bring this to the TSC ?

Not sure if a TSC approval is needed for this change since this more or less is fixing a bug where an incompatible version of SDK is picked up by the Zephyr LTS release.

It would be good to mention it at a TSC meeting to raise more awareness, nevertheless.

Fixes: zephyrproject-rtos#80200

CMake `find_package(<package> <version>)` support the use of ranges,
like `1.0.0...4.0.0`.

Update the FindZephyr-sdk.cmake module to support this.
This allows looking up the Zephyr SDK with an upper boundry, for example
`find_package(Zephyr-sdk 0.16...<0.17)`.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
Zephyr SDK compatibility level 0.16 is now an LTS intended for
Zephyr v3.7 LTS.

Thus, limit the upper Zephyr SDK lookup to <0.17, so that developers
working with Zephyr v3.7 LTS and latest Zephyr main can install both
Zephyr SDK 0.16.x along side Zephyr SDK >=0.17 and have the Zephyr SDK
0.16.x series being used for v3.7 LTS development.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
@tejlmand tejlmand force-pushed the toolchain/zephyr_sdk_v3_7 branch from 0dfdc46 to 61a6e32 Compare October 22, 2024 08:26
@tejlmand
Copy link
Collaborator Author

Though, ultimately, we would want find_package(Zephyr-sdk a.b) to only look for the SDKs versions a.b.X as per the semantic versioning scheme discussed in zephyrproject-rtos/sdk-ng#608 -- this way, we do not have to make changes like this to previous Zephyr releases in the future.

but in such case the I believe we should have a Zephyr SDK v1.0.0 release, and actually start using the major version.
A bump from 0.16.x to 0.17.0 should generally not be considered a breaking change.

since this more or less is fixing a bug where an incompatible version of SDK is picked up by the Zephyr LTS release.

as long as we are using versions <1.0.0 then I think it's stretching it a bit to consider it a bug, but guess we're getting into semantics at this point 😉

It would be good to mention it at a TSC meeting to raise more awareness, nevertheless.

yep, I was not expecting any push-back on this, but awareness on those kind of changes are usually good.

Copy link
Collaborator

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

I'm trusting that the cmake bits implement what it says on the tin...

@tejlmand tejlmand removed the DNM This PR should not be merged (Do Not Merge) label Oct 24, 2024
@pdgendt
Copy link
Collaborator

pdgendt commented Oct 24, 2024

Should this also get backported to 3.6 as it is still active?

@henrikbrixandersen
Copy link
Member

Should this also get backported to 3.6 as it is still active?

Yes, I think it should.

@carlescufi carlescufi merged commit 6da35a8 into zephyrproject-rtos:v3.7-branch Oct 24, 2024
24 of 26 checks passed
@pdgendt
Copy link
Collaborator

pdgendt commented Oct 24, 2024

Hmn marking backport only works for PRs that are merged to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants