Skip to content

cmake: set BUILD_ARCH with CMAKE_HOST_SYSTEM_PROCESSOR - #7369

Open
nilason wants to merge 2 commits into
OSGeo:mainfrom
nilason:cmake_rm_custom_host_arch
Open

cmake: set BUILD_ARCH with CMAKE_HOST_SYSTEM_PROCESSOR#7369
nilason wants to merge 2 commits into
OSGeo:mainfrom
nilason:cmake_rm_custom_host_arch

Conversation

@nilason

@nilason nilason commented May 6, 2026

Copy link
Copy Markdown
Contributor

Set BUILD_ARCH with CMAKE_HOST_SYSTEM_PROCESSOR and remove custom function get_host_arch.

I see no reason to not use CMake's variable for setting this, after all that is deciding what/how to build anyway. The custom function requires maintenance, macOS is currently not supported and defaults to x86_64.

The only effect this has is the build_platform output of g.version -g. Otherwise BUILD_ARCH is mainly a remnant of Autotools builds, where build output directory is named after Autotool's host variable (e.g. dist.aarch64-apple-darwin25.3.0; note: this is not the same as just uname -m) and this is passed to various Python build scripts, in CMake it doesn't play any role.

Remove custom function 'get_host_arch'.
@nilason nilason added this to the 8.5.0 milestone May 6, 2026
@nilason nilason added the backport to 8.5 PR needs to be backported to release branch 8.5 label May 6, 2026
@github-actions github-actions Bot added the CMake label May 6, 2026
@nilason
nilason requested a review from HuidaeCho May 6, 2026 12:53
@wenzeslaus wenzeslaus modified the milestones: 8.5.0, 8.5.1 May 8, 2026
Comment thread CMakeLists.txt

include(get_host_arch)
get_host_arch(BUILD_ARCH)
set(BUILD_ARCH ${CMAKE_HOST_SYSTEM_PROCESSOR})

@HuidaeCho HuidaeCho May 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall, it looks good to me. Can we apply lowercase on Windows? Currently,

  • Linux: x86_64
  • macOS: x86_64, arm64
  • Windows: AMD64 (should we unify it to x86_64?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can we apply lowercase on Windows? ... (should we unify it to x86_64?)

I think it should be as straightforward as possible, that is, leave it as is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The question is: what do we want to show with build_platform? What relevant information should be output from a CMake build? And when/where would that be helpful? E.g., if a package is cross compiled, it may be of interest what was the host platform.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea of build_platform seems to be "platform the binary was build for" (CMAKE_SYSTEM_PROCESSOR) although the naming suggests it was derived from "platform we are building on...because that's the only thing we are sure about" (CMAKE_HOST_SYSTEM_PROCESSOR).

CMAKE_SYSTEM_PROCESSOR seems more appropriate at this point.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
set(BUILD_ARCH ${CMAKE_HOST_SYSTEM_PROCESSOR})
set(BUILD_ARCH ${CMAKE_SYSTEM_PROCESSOR})

@wenzeslaus wenzeslaus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change to CMAKE_SYSTEM_PROCESSOR because the idea of build_platform where this is used seems to be "platform it is build for", not "platform it was build on".

Comment thread CMakeLists.txt

include(get_host_arch)
get_host_arch(BUILD_ARCH)
set(BUILD_ARCH ${CMAKE_HOST_SYSTEM_PROCESSOR})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
set(BUILD_ARCH ${CMAKE_HOST_SYSTEM_PROCESSOR})
set(BUILD_ARCH ${CMAKE_SYSTEM_PROCESSOR})

Comment thread CMakeLists.txt

include(get_host_arch)
get_host_arch(BUILD_ARCH)
set(BUILD_ARCH ${CMAKE_HOST_SYSTEM_PROCESSOR})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since you mentioned in the call that you would be interested in aligning more with Autotools, I asked Claude to trace where the Autotools value comes from and what consumes it, and I reviewed the result; suggestions below are based on that.

In Autotools, ARCH is set from AC_CANONICAL_HOST (ARCH="${host}" in configure.ac), i.e., the GNU host triplet: the system the binaries run on, not the machine compiling them. That value lands in include/Make/Platform.make (ARCH = @host@), which is what g.version gets via -DARCH and what grass --config arch prints.

The CMake equivalent of GNU host is CMAKE_SYSTEM_PROCESSOR (the target system), while CMAKE_HOST_SYSTEM_PROCESSOR is the build machine (GNU build). They are identical for native builds and differ only when cross-compiling, where CMAKE_SYSTEM_PROCESSOR is the one matching the Autotools semantics. I think that also answers your question above about what build_platform should show for a cross-compiled package: the platform the package runs on.

Suggested change
set(BUILD_ARCH ${CMAKE_HOST_SYSTEM_PROCESSOR})
set(BUILD_ARCH "${CMAKE_SYSTEM_PROCESSOR}")

(The quotes just avoid set() silently unsetting BUILD_ARCH in the unlikely case the variable is empty; CMake documents it as possibly empty when undeterminable.)

If closer parity with the Autotools triplet (e.g. aarch64-apple-darwin25.3.0) is ever wanted, the OS could be added:

set(BUILD_ARCH "${CMAKE_SYSTEM_PROCESSOR}-${CMAKE_SYSTEM_NAME}")

giving arm64-Darwin, x86_64-Linux, AMD64-Windows — but that changes the value on all platforms, so plain CMAKE_SYSTEM_PROCESSOR seems like the right scope for this PR.

A side note from the same trace: grass --config arch reads ARCH from the installed include/Make/Platform.make, which CMake builds don't generate, so --config arch errors out under CMake regardless of this PR — a separate gap, not something to fix here.

@echoix echoix added the conflicts/needs rebase Rebase to or merge with the latest base branch is needed label Jul 17, 2026
@echoix echoix removed the conflicts/needs rebase Rebase to or merge with the latest base branch is needed label Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport to 8.5 PR needs to be backported to release branch 8.5 CMake

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants