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

CMake: Fix unknown architecture and simplify OSX_ARCHITECTURES #1708

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

enetheru
Copy link
Contributor

@enetheru enetheru commented Feb 13, 2025

It was brought to my attention that @Naros was getting builds with the architecture name tag set to unknown

This is cosmetic and doesnt effect the build, but did expose an underlying problem with how I was treating the CMAKE_SYSTEM_PROCESSOR flag, and how I was generating the OUTPUT_NAME

It also made me re-examine how I was treating the OSX_ARCHITECTURES, in light of the recent MSVC_RUNTIME_LIBRARIES #1701 discussion

Changes:

  • Remove GODOT_ARCH option, it's managed by the toolchain files, or build vars.
  • Rename godot_arch_map to godot_arch_name to better describe its use
  • Add the known Android architectures to the godot_arch_name internal lists
  • Return ${CMAKE_SYSTEM_PROCESSOR} when architecture is unknown instead of unknown
  • Remove all reference to OSX_ARCHITECTURES, and universal and provide links to documentation on how to use OSX_ARCHITEXTURES in the cmake/macos.cmake header

This simplifies the case for MacOS, iOS, and makes the resulting naming more predictable

@enetheru enetheru marked this pull request as ready for review February 13, 2025 08:00
@enetheru enetheru requested a review from a team as a code owner February 13, 2025 08:00
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

I don't understand a lot of what's here, but I trust @enetheru and @Naros has tested it, so let's give it a shot :-)

@dsnopek dsnopek added bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup labels Feb 14, 2025
@dsnopek dsnopek added this to the 4.x milestone Feb 14, 2025
@dsnopek dsnopek merged commit 79f9bc9 into godotengine:master Feb 17, 2025
11 checks passed
@enetheru enetheru deleted the arch_confusion branch February 17, 2025 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants