chore: move xbox toolchain to sentry-xbox repository #1329
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| # elif defined(SENTRY_PLATFORM_XBOX) | ||
| # define SENTRY_SDK_NAME "sentry.native.xbox" |
There was a problem hiding this comment.
I didn't highlight this because it would be fine to do this in a closed repo. So, with the toolchain defs removed, no one could use the open alone repo anymore anyway, so having the def here wouldn't provide a lot in return. However, I am fine either way. You guys delineate which parts should be where.
There was a problem hiding this comment.
I've been thinking along the same line but @bruno-garcia asked me to put this back because it's possible sometimes can still use sentry-native directly if they use their own toolchain file (e.g. if they pull this directly to their build system instead of precompiling), which is a good point. This is a small piece of code anyway so should be alright to keep here
There was a problem hiding this comment.
This certainly makes sense.
| elseif(CMAKE_SYSTEM_NAME STREQUAL "Prospero") | ||
| set(PROSPERO TRUE) | ||
| elseif("${CMAKE_GENERATOR_PLATFORM}" MATCHES "Gaming\\.Xbox\\.(Scarlett|XboxOne)\\.x64") | ||
| set(XBOX TRUE) |
There was a problem hiding this comment.
Bug: Xbox Build Failures Due to Incorrect Variable Placement
Xbox support is broken. The new XBOX variable detection logic is incorrectly placed within an elseif chain that checks CMAKE_SYSTEM_NAME. Since Xbox builds set CMAKE_SYSTEM_NAME to "WINDOWS", the CMAKE_GENERATOR_PLATFORM condition for Xbox is never reached, and XBOX is never set. This, coupled with the removal of Xbox toolchain files that generated gdk_build.props (a file referenced by later CMake logic when XBOX is true), leads to build failures for Xbox targets.
There was a problem hiding this comment.
that's not true, there's no CMAKE_SYSTEM_NAME == windows condition in this if-elseif chain
Having the xbox support spread out between multiple repositories (sentry-native and sentry-xbox) induces a maintenance burden. Also, it's a bit of a pain when including toolchains in the downstream sentry-xbox due to having first fetch this repo before being able to use them in cmake.
This PR removes the toolchains from this repo. Additionally, I've removed changelog items from the unreleased versionnot relevant here anymore.