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

Allow building for pico by specifying a toolchain, same as 32blit #714

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

ali1234
Copy link
Contributor

@ali1234 ali1234 commented Sep 15, 2021

To build a project for pico just do:

cmake -DCMAKE_TOOLCHAIN_FILE=../../32blit-sdk/pico.toolchain ..

You may optionally override PICO_BOARD and PICO_SDK_PATH

To build a project for pico just do:

    cmake -DCMAKE_TOOLCHAIN_FILE=../../32blit-sdk/pico.toolchain ..

You may optionally override PICO_BOARD and PICO_SDK_PATH
@ali1234
Copy link
Contributor Author

ali1234 commented Sep 15, 2021

Guessing this will blow up the checks.

This allows games to be built for picosystem without having to include extra stuff in their local CMakeLists.txt. It works exactly the same as 32blit.toolchain, although the contents is very different.

@ali1234
Copy link
Contributor Author

ali1234 commented Sep 15, 2021

How it works:

32blit-pico/CMakeLists.txt is modified to include those two weird cmake files that set up the SDK, which you'd normally have to add to the project top level. They claim that they must be included before project() however this does not seem to be the case because...

pico.toolchain is pico_arm_gcc.cmake and find_compiler.cmake from the pico-sdk. These normally have to be included before project... because they are actually toolchain files. Using them as such makes all this work. I have simply taken those two files and turned them into one file (ie pasting one into the other where it would normally import it). Then, at the end, I added a couple of defaults for the board and path variables.

# Look for includes and libraries only in the target system prefix.
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
#set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also commented this line because it prevents the 32blit-sdk from being found. This is the same as the problem with emscripten.

@Daft-Freak
Copy link
Collaborator

Hmm, hopefully the Pico SDK doesn't change in a way that breaks this... I guess this is more how CMake is supposed to be used, but also not how the Pico SDK wants you to use it 🤷

Anyway, having to do less special stuff for one platform is nice and this didn't appear to break the old setup.

@ali1234
Copy link
Contributor Author

ali1234 commented Sep 15, 2021

I think the only reason this works for CI is because they have include guards on everything. So it will all get included multiple times if you do it the recommended way, but it is still in mostly the right order.

It probably will break, that's why this is a draft. Maybe we could add it as a submodule or use FetchContent to get a specific revision. (If we do that we can also hard code the path without worrying about it. Currently this will give a weird error if the SDK isn't where I've hard coded it to be, and you don't know to override that.)

@ali1234
Copy link
Contributor Author

ali1234 commented Oct 7, 2021

I've tested this on hardware now and it seems to work fine.

@Gadgetoid Gadgetoid marked this pull request as ready for review October 7, 2021 20:29
@Gadgetoid Gadgetoid merged commit 889ed50 into 32blit:master Oct 11, 2021
@Gadgetoid
Copy link
Contributor

Thank you! This definitely feels more consistent, even if it might spontaneously explode. (maybe?)

@ali1234 ali1234 deleted the pico-as-toolchain branch October 22, 2021 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants