lancaster-university / microbit-v2-samples

CODAL build tools and sample programs for the micro:bit v2.x
MIT License
64 stars 83 forks source link

Add CMAKE_SYSTEM_NAME and CMAKE_SYSTEM_VERSION #39

Closed martinwork closed 2 years ago

martinwork commented 2 years ago

On M1 mac, I see errors like:

arm-none-eabi-g++: error: arm64: No such file or directory
arm-none-eabi-g++: error: unrecognized command-line option '-arch'; did you mean '-march='?

This seems to be fixed by the magic of defining CMAKE_SYSTEM_NAME in ARM_GCC/toolchain.cmake. It doesn't seem to matter what CMAKE_SYSTEM_NAME is defined as.

The cmake docs (https://cmake.org/cmake/help/v3.20/variable/CMAKE_SYSTEM_NAME.html) suggest CMAKE_SYSTEM_VERSION must be defined too, though leaving it out doesn't appear to cause a problem.

Adding these definitions doesn't upset the Windows build. The MICROBIT.hex is unchanged and the same on mac as windows.

I'm building with cmake 3.20.5 and arm-none-eabi 10.2.1.

JohnVidler commented 2 years ago

Hmm. I'm a little concerned about setting these to generic values without knowing the side effects. Although it does look like this builds just fine on my local machine (MacOS 12.1).

As both variables are defined as the target system name and target system version (respectively), I suggest we align these with the actual build target we're using, so CMAKE_SYSTEM_NAME would be set to "micro:bit" and CMAKE_SYSTEM_VERSION set to "2.0.0", this way if we ever need to rely on these in the future, then they're already set to appropriate values (and other platforms can adjust these to match, as required; rp2040, for example)

I've tested these locally and they seem fine here, but I don't have a Windows box to hand to double check (although hopefully the CI actions will catch this if it breaks on Windows or Linux systems).

microbit-carlos commented 2 years ago

Maybe I am missunderstanding this, but is the problem that without setting these values the build system is trying to build for macOS Arm instead of arm-none-eabi? An by setting the OS variables to a string that CMake doesn't recognise it no longer tries to build for the local platform?

martinwork commented 2 years ago

Thanks @JohnVidler and @microbit-carlos Yes, the errors I was getting look like it's trying to build for my mac, and setting CMAKE_SYSTEM_NAME to anything enabled a cross-platform build, because CMAKE_SYSTEM_NAME != CMAKE_HOST_SYSTEM_NAME

I backed off from a specific name because of the messages like https://github.com/lancaster-university/microbit-v2-samples/pull/39/checks#step:8:18

System is unknown to cmake, create:
Platform/micro:bit to use this system, please send your config file to cmake@www.cmake.org so it can be added to cmake

I don't know what triggers the problem, only that defining Generic worked without the messages and the built hex was identical to my windows build. Maybe cmake was newer or doesn't recognise M1 macs properly?

After another look around, I found https://gitlab.kitware.com/cmake/cmake/-/issues/21489 The meaning of Generic is perhaps in https://gitlab.kitware.com/cmake/cmake/-/blob/v3.22.0-rc2/Modules/Platform/Generic.cmake

microbit-carlos commented 2 years ago

This looks relevant: https://gitlab.kitware.com/cmake/cmake/-/issues/23105

microbit-carlos commented 2 years ago

Looks like this was also the way Yotta was setting up CMake 🤔: https://github.com/ARMmbed/target-mbed-gcc/blob/master/CMake/toolchain.cmake#L12

Is this a common configuration/pattern for CMake cross-compilation? Looks like "Generic" is probably what we would need to set up anyway for embedded targets.

martinwork commented 2 years ago

This looks relevant: https://gitlab.kitware.com/cmake/cmake/-/issues/23105

@microbit-carlos Good spot! I added a question.

Is this a common configuration/pattern for CMake cross-compilation? Looks like "Generic" is probably what we would need to set up anyway for embedded targets.

I thought I understood, but I don't see a "mbedOS.cmake" in https://gitlab.kitware.com/cmake/cmake/-/tree/v3.22.0-rc2/Modules/Platform. I'm now worried about missing all the configurations in Darwin.cmake or Windows.cmake.

I have problems starting a clean build from an XCode script, as opposed to using Terminal, which I haven't looked into yet.

microbit-carlos commented 2 years ago

Is this a common configuration/pattern for CMake cross-compilation? Looks like "Generic" is probably what we would need to set up anyway for embedded targets.

I thought I understood, but I don't see a "mbedOS.cmake" in https://gitlab.kitware.com/cmake/cmake/-/tree/v3.22.0-rc2/Modules/Platform.

Right, sorry, I didn't expect mbedOS to be in the "official" CMake list of platforms, but I was wondering if setting that value to something not official (so something that doesn't have any additional config files in that directory from the CMake repo) was a common pattern to set up a "clean empty platform" for embedded targets.

Based on the link that you posted, https://gitlab.kitware.com/cmake/cmake/-/blob/v3.22.0-rc2/Modules/Platform/Generic.cmake, the "Generic" platform is pretty close to empty, so it seems like it's their recommended value for embedded platforms like us? Rather than have "micro:bit" string and have a warning issued in all builds. Thanks for asking about possible repercussion in that thread in the issue tracker, it'd be interesting to hear back from CMake.

I'm now worried about missing all the configurations in Darwin.cmake or Windows.cmake.

What files do you mean, from the CMake project like this one? https://gitlab.kitware.com/cmake/cmake/-/blob/v3.22.0-rc2/Modules/Platform/Darwin.cmake

I would think those are targeting builds to run on macOS or Windows.

Or do you mean other files from places like the Yotta repos?

martinwork commented 2 years ago

Right, sorry, I didn't expect mbedOS to be in the "official" CMake list of platforms,

No, no! Nothing to be sorry for! I was thinking that has to be an official Platform. Now I actually look, https://github.com/ARMmbed/target-mbed-gcc/tree/master/CMake has it's own Platform folder.

I would think those are targeting builds to run on macOS or Windows.

I think so too, but if not specified, CMAKE_SYSTEM_NAME == CMAKE_HOST_SYSTEM_NAME == Darwin on macOS, so I was thinking Darwin.cmake gets used, but I expect I'm wrong!

JohnVidler commented 2 years ago

Thanks to both of you for digging into this some more. I'm mostly just concerned about CMake setting any other internal variables based on the value we set in these variables (and doing so invisibly) which might break the build. If we go with Generic, are we also buying in to any other Generic target updates to CMake moving forwards?

I would think those are targeting builds to run on macOS or Windows.

I think so too, but if not specified, CMAKE_SYSTEM_NAME == CMAKE_HOST_SYSTEM_NAME == Darwin on macOS, so I was thinking Darwin.cmake gets used, but I expect I'm wrong!

So is that a CMake bug, or the expected behaviour and the other hosts just aren't as picky as M1-based kit?

martinwork commented 2 years ago

So is that a CMake bug, or the expected behaviour and the other hosts just aren't as picky as M1-based kit?

I don't know, but I think It's a bit of both! See https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html and https://gitlab.kitware.com/cmake/cmake/-/issues/23105

It was working before I tried M1. Defining CMAKE_SYSTEM_NAME let me compile the hex on my M1 and didn't change my Windows built hex.

My guess now is that set(CMAKE_SYSTEM_NAME Generic) is a workaround for a CMake bug on M1s, and maybe the default behaviour will be the best bet once the bug is fixed.

set(CMAKE_SYSTEM_NAME "micro:bit") might not be a good choice as I think it's used as a filename.

microbit-carlos commented 2 years ago

Based solely on the comment at the top of the generic system config file, it sounds to me like this is the blessed way to build for embedded targets? https://gitlab.kitware.com/cmake/cmake/-/blob/v3.22.0-rc2/Modules/Platform/Generic.cmake

This is a platform definition file for platforms without operating system, typically embedded platforms. It is used when CMAKE_SYSTEM_NAME is set to "Generic"

martinwork commented 2 years ago

This is also helpful... https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html

"CMAKE_SYSTEM_NAME This variable is mandatory... If your target is an embedded system without an OS, set CMAKE_SYSTEM_NAME to “Generic.”"

... contradicting my last thought! :)

JohnVidler commented 2 years ago

set(CMAKE_SYSTEM_NAME "micro:bit") might not be a good choice as I think it's used as a filename.

Very good point, I forgot that it just uses it as a lookup in the list of targets - this would make it break on Windows targets (among others), I think, where you can't have colons in filenames.

"CMAKE_SYSTEM_NAME This variable is mandatory... If your target is an embedded system without an OS, set CMAKE_SYSTEM_NAME to “Generic.”"

Generic it is then! Unless we fancy like supplying a dedicated micro:bit one to the cmake folks, and I would suggest we don't do this, as it's a slippery slope to adding all platforms in there :)

That does leave CMAKE_SYSTEM_VERSION though, and it looks like the Generic file doesn't use it in any way, so I still suggest we set this to the build version we're using (2.0.0) for now, then if we do need to make any distinctions between build version we have the option if we end up adding a system definition. I suppose the only danger of this is if we end up using the variable internally to our own CMakeLists and then the Generic target also starts using them for some reason, but I can't think of a good reason for it doing so.

martinwork commented 2 years ago

See confirmation in https://gitlab.kitware.com/cmake/cmake/-/issues/23105#note_1110216

I think using Generic and 2.0.0 sounds fine. Should anything unhelpful happen in Generic, I suppose the yotta example shows how to use a local custom platform.