Skip to content

Fix cmake handling of paths with spaces #164

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

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

TankedThomas
Copy link
Contributor

My Pico SDK filepath uses spaces, which caused problems with the cmake files. I know it's not ideal to have pathname spaces, but putting double quotes around all the variables in the cmake files should fix this.

I'm not aware of any downsides to this change but I'm no expert on the subject. I did test compiling after the changes and it seems to work fine.

There are two commits because I forgot to include the other two cmake files initially.

@lurch
Copy link
Contributor

lurch commented Mar 6, 2025

@TankedThomas
Copy link
Contributor Author

Okay, latest commit should mean this is ready to merge. Only question left is whether we should keep double quotes for PICO_SDK_VERSION_MAJOR and CMAKE_VERSION (would make it more consistent) or remove them (since they're not necessary). I can make another commit to change this before merge if required.
If necessary, I can make pull requests for upstream afterwards.

@P33M
Copy link
Contributor

P33M commented Mar 10, 2025

Please remove the quotes for cmake_version and pico_sdk_version_major - they aren't paths, and shouldn't have spaces in.

After you've done that, please squash your commits and add a signed-off-by line:

git reset --soft HEAD~3
git commit --edit
git commit --amend -s
git push -f <your remote>

The freertos cmake file is from upstream - https://github.com/FreeRTOS/FreeRTOS-Kernel - and the pico-sdk import is from our repo.

Signed-off-by: TankedThomas <[email protected]>
@TankedThomas
Copy link
Contributor Author

TankedThomas commented Mar 11, 2025

Okay, I removed those quotes, also from $ENV{PICO_SDK_FETCH_FROM_GIT} since it's a boolean and we're not quoting every variable.

Commits are squashed and signed off. Should be ready to merge now.
Do you want me to open pull requests for the cmake files on the other two repos?
It doesn't seem like pico_sdk_import.cmake causes problems for pico-sdk as-is though, not that I can reproduce at least.
I'm not sure how to build FreeRTOS to test that one but it has all three files included in the RP2040 directory.

Quick note: upstream has other changes and was updated a month ago vs. 4 months ago in this repo. Should those be merged too? It'll probably require some extra changes to the upstream version too.

@P33M P33M merged commit 1abda4c into raspberrypi:master Mar 11, 2025
@P33M
Copy link
Contributor

P33M commented Mar 11, 2025

Thanks. I only update external cmake files if there's a dependency break - which won't happen if I don't move the submodule pointers. If any changes to those files land upstream and I need to update them, I'll do that as and when necessary.

I've no objection to a different fix landing in upstream repos.

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