Skip to content

fix(cmake): Protect CMAKE_SYSTEM_VERSION to avoid empty values when cross-building#1720

Merged
jpnurmi merged 4 commits into
getsentry:masterfrom
uilianries:fix/cmake-windows-arm
May 14, 2026
Merged

fix(cmake): Protect CMAKE_SYSTEM_VERSION to avoid empty values when cross-building#1720
jpnurmi merged 4 commits into
getsentry:masterfrom
uilianries:fix/cmake-windows-arm

Conversation

@uilianries
Copy link
Copy Markdown
Contributor

Hello!

As explained on #1716, the missing quotes when comparing values and one is missing (CMAKE_SYSTEM_VERSION in this case), will fail CMake.

Also, quoting the book "Professional CMake - A Practical Guide":

Only use an unquoted argument if it is guaranteed that a variable by that name exists.

Professional CMake - Graig Scott, page 57 - The if() command

This PR guards CMAKE_SYSTEM_VERSION, so if it's empty, that if condition will not break.

fix #1716

Signed-off-by: Uilian Ries <uilianries@gmail.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 94fdc97. Configure here.

Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
@jpnurmi jpnurmi requested a review from supervacuus May 13, 2026 07:38
Copy link
Copy Markdown
Collaborator

@supervacuus supervacuus left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks! Please still add a change-log entry since this is certainly a publicly visible fix.

@supervacuus
Copy link
Copy Markdown
Collaborator

@jpnurmi, Warden fails because this is coming from a fork. No need to rerun. Warden failure is not a merge blocker.

@jpnurmi
Copy link
Copy Markdown
Collaborator

jpnurmi commented May 13, 2026

@uilianries Thanks for the fix! Could you add an entry to CHANGELOG.md?

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

**Fixes**:

- Protect CMAKE_SYSTEM_VERSION to avoid empty values when cross-building. ([#1720](https://github.com/getsentry/sentry-native/pull/1720))

@uilianries
Copy link
Copy Markdown
Contributor Author

@jpnurmi Done, changelog updated. Nice to see you are using keepachangelog format! 😄

Comment thread CMakeLists.txt
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Comment thread CMakeLists.txt
@jpnurmi jpnurmi merged commit de88171 into getsentry:master May 14, 2026
54 of 55 checks passed
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 this pull request may close these issues.

Could not cross-build on Windows to armv8 due guard empty CMAKE_SYSTEM_VERSION when linking synchronization

3 participants