xmos / xcore_iot

Other
29 stars 39 forks source link

Building an app with xs3a.cmake gives a warning #507

Closed xhuw closed 1 year ago

xhuw commented 1 year ago

System information

Describe the current behavior

When using xs3a.cmake as the toolchain for my project prints

System is unknown to cmake, create:
Platform/XCORE_XS3A to use this system, please post your config file on discourse.cmake.org so it can be added to cmake

Describe the expected behavior

cmake should look happy when it does a build

Standalone code to reproduce the issue

In a directory with a CMakeLists.txt and a clone of the sdk run:

cmake -DCMAKE_TOOLCHAIN_FILE=xcore_sdk/xmos_cmake_toolchain/xs3a.cmake

Other info or logs

@CiaranWoodward sent me some helpful links: 1 2 3 4 5

However If the CMAKE_SYSTEM_NAME is set to Generic then the warning is gone. Do we need to have the system name set to something which cmake doesn't support? We could use a custom variable for differentiation until we have upstreamed support.

CiaranWoodward commented 1 year ago

As CMAKE_SYSTEM_NAME is for the operating system name, putting set(CMAKE_SYSTEM_NAME Generic) in the toolchain file would be the standard thing to do for baremetal.

keithm-xmos commented 1 year ago

We do currently use CMAKE_SYSTEM_NAME in CMakeLists to differentiate between builds targeting xcore or other (x86 usually). So, this change request has a ripple effect and will require some planning.

How would you recommend we specify that the target system is xcore? Several of our libraries can be built for x86 and xcore so we do need a flag.

CiaranWoodward commented 1 year ago

CMAKE_SYSTEM_PROCESSOR may be suitable because it's fairly freeform, or a custom variable like XMOS_TARGET_PROCESSOR.

@xhuw is this actually breaking anything or is it just making the cmake output messy?

xhuw commented 1 year ago

Not breaking anything, so low priority for me. Setting up a new project at the moment and raising issues on the SDK as I notice them.

The only issue is the messy output, so a fix would be a nice to have for new users, or CMake beginners who might think it's something they need to resolve

xmos-jmccarthy commented 1 year ago

XCORE the silicon supports the XCORE operating system, so CMAKE_SYSTEM_NAME is appropriate. Perhaps we could use CMAKE_SYSTEM_PROCESSOR for the architecture, XS3A, XS2A, etc. and XCORE for the CMAKE_SYSTEM_NAME.

I would not want to replace the current use of CMAKE_SYSTEM_NAME, as there are many useful thirdparty CMake ready libraries out there that use it explicitly for Host builds. I wouldn't want us to be in a situation where lib_foo cmake adds something when CMAKE_SYSTEM_NAME == "Win" that I miss because I am on Linux, or vice versa. CMAKE_SYSTEM_NAME being set to "Generic" is common in other microcontrollers, but they do not have a hardware OS like xcore does. That being said, this is primarily just semantics.

The ideal solution would be completion of: https://github.com/xmos/xmos_cmake_toolchain/issues/3

So that we are added to the list: https://gitlab.kitware.com/cmake/cmake/-/blob/v3.22.0-rc2/Modules/CMakeDetermineSystem.cmake#L12-31

Then there would be no error, and we wouldn't have to submodule https://github.com/xmos/xmos_cmake_toolchain everywhere as it would be provided by the cmake installation.

keithm-xmos commented 1 year ago

Fixed by https://github.com/xmos/xmos_cmake_toolchain/pull/4