raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.63k stars 902 forks source link

Support for CMake find_package and imported targets #1476

Open asasine opened 1 year ago

asasine commented 1 year ago

I'm curious and wondering if the maintainers are open to adopting a more standard approach to pico-sdk's CMake integration based on the CMake's guides for package maintainers, including Using Dependencies, Import and Exporting, and Finding Packages. Finding and linking to libraries in a modern CMake project typically utilizes find_package(...) calls, linking to namespaced imported targets, and doesn't require library-specific CMake macro/function calls to "fully initialize" the dependency like pico_sdk_init(). A limited example would look like this:

project(my_project)
find_package(pico-sdk REQUIRED)
add_executable(hello_world src/hello.cpp)
target_include_directories(hello_world include)
target_link_libraries(hello_world pico_sdk::pico_stdlib)

As I see it, only one change is required to get this to work with some lifting on the user's part. Removing the requirement to include pico_sdk_init.cmake before the project(...) call allows users to write their own FindPicoSdk.cmake, thus gaining a similar experience to pico-sdk shipping CMake configuration files. If anyone knows why, I'd love to know what in that CMake file makes this a requirement; nothing obvious stood out to me when I read through it.

Optionally, pico-sdk can fully support CMake conventions with some additional changes:

  1. During the pico-sdk build, create and install(...) a CMake export set and a package configuration file (version files optional but good too).
  2. Provide new installation instructions for adding pico-sdk via CMake config mode, such as cloning to the local administrator path (/usr/local/) or elsewhere (e.g., /opt or ~/.local/); or ship a system package pico-sdk-dev that installs sources to system paths directly (/usr/)

Let me know what you think about this proposal. I have experience writing and maintaining CMake packages and would be happy to jump in to making improvements and fixes.

kilograham commented 9 months ago

We are certainly interested in being more standards compliant.... seems like it might be a bit of a backwards incompatibility so positing a 2.0 SDK release

asasine commented 9 months ago

2.0 seems fine to me. What's the process to get this contributed? There'll need to be some decisions made on where/how installation is performed (e.g., src or binary distribution, installation path).

kilograham commented 3 months ago

A few random thoughts about how things could work better.

  1. we need to set some stuff up before the PROJECT statement (otherwise the user would have to specify a bunch of command line flags to infer stuff we do before setting up the compiler)
  2. Is there any mechanism for the "found" package to perform initialization when it is included via "find-package"... or is that all in the Findpico-sdk.cmake which would be less helpful)
asasine commented 3 months ago

Compiler flags are typically set up on the exported library target that you set up in the the pico-sdk CMakeLists.txt. These flags will continue on with the target when the user imports the target via the find_package(pico-sdk) call. I've never seen a case where find_package(...) is called before project(...) and I'm curious what sort of flags and things need to be inferred before setting up the compiler and before the project(...) call?

Regarding a mechanism to perform initialization when the user find_package(pico-sdk), that's entirely up to the library authors. When creating the package configuration file, you have complete flexibility in what is performed. The general process is you write your own pico-sdk-config.cmake file, which can contain any initialization necessary for the package, install it to the [relocatable] lib/cmake/pico-sdk directory, and find_package(pico-sdk) will search for pico-sdk-config.cmake within several predefined locations on the user's filesystem and invoke whatever is in that pico-sdk-config.cmake file. This is specifically called "config mode" and is set up by packages that natively support CMake, as opposed to "module mode" or "find modules," which is intended for supporting packages without native CMake support.

kilograham commented 3 months ago

I'm curious what sort of flags and things need to be inferred before setting up the compiler and before the project(...) call?

Most (non-capatably?) we do select (default) the toolchain file based on the PICO_PLATFORM and or PICO_BOARD variable... we then do a bunch of other configuration (including the toolchain) but that is maybe fine.

asasine commented 3 months ago

I think it's typical to specify toolchain files when invoking the cmake executable with --toolchain path/to/toolchain.cmake or -DCMAKE_TOOLCHAIN_FILE=path/to/toolchain.cmake.

lurch commented 3 months ago

I think it's typical to specify toolchain files when invoking the cmake executable with --toolchain path/to/toolchain.cmake or -DCMAKE_TOOLCHAIN_FILE=path/to/toolchain.cmake.

Does that mean that everywhere in https://datasheets.raspberrypi.com/pico/getting-started-with-pico.pdf where we do cmake .. we'd need to change it to cmake --toolchain path/to/toolchain.cmake .. ? (which is obviously somewhat less user-friendly)

asasine commented 3 months ago

Or setting the CMAKE_TOOLCHAIN_FILE environment variable once and running CMAKE_TOOLCHAIN_FILE=path/to/file cmake ... This gets persisted to the CMake cache so is only necessary on the first call so subsequent calls can be cmake ..

will-v-pi commented 3 weeks ago

I've had a look into this, and I don't think we'd want to switch to this being the recommended way to do things due to needing to specify the toolchain file, but it should be possible to support doing it this way (with a toolchain file, and then a find_package call after the project). I'll have a go and open up a PR