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

fix cmake syntax for finding credentials.cmake #13

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

SebKuzminsky
Copy link
Contributor

When building this project from another cmake project, I get an error even though I have credentials.cmake (this is on Debian Bookworm using cmake 3.25):

CMake Warning at pico-w-webserver-example/CMakeLists.txt:24 (message):
  Credentials file not found, using default values!

Per the cmake docs, if EXISTS requires full/absolute paths; relative paths do not have well-defined behavior:

<https://cmake.org/cmake/help/latest/command/if.html#exists>

When building this project from another cmake project, I get an error
even though I have credentials.cmake (this is on Debian Bookworm using
cmake 3.25):

    CMake Warning at pico-w-webserver-example/CMakeLists.txt:24 (message):
      Credentials file not found, using default values!

Per the cmake docs, `if EXISTS` requires full/absolute paths; relative
paths do not have well-defined behavior:

    <https://cmake.org/cmake/help/latest/command/if.html#exists>
@SebKuzminsky
Copy link
Contributor Author

Hi there, thanks for this super helpful example :-)

I realize that my use case of having a project that includes pico-w-webserver-example in a subdirectory and builds it using add_directory(pico-w-webserver-example) in the top-level cmake file is unusual, so feel free to discard this PR if you don't find it relevant or useful.

@SebKuzminsky
Copy link
Contributor Author

Hmm, the build failed with this error:

-- Build files have been written to: D:/a/pico-w-webserver-example/pico-w-webserver-example/build
mingw32-make: *** No targets specified and no makefile found.  Stop.

I'm sorry I don't know enough about Windows to debug this :-(

@krzmaz
Copy link
Owner

krzmaz commented Oct 9, 2023

Hi, sorry for the late response, I was on vacation for the last three weeks.
Thanks for the contribution! 😄
I will try to reproduce the issue using act and see if I can find a workaround

@krzmaz
Copy link
Owner

krzmaz commented Oct 9, 2023

Update - sadly act doesn't support Windows, but I've ran a quick check and the mainline build seems to be broken (#14) on Windows so I'm just going to merge your change and deal with windows in #15

@krzmaz krzmaz merged commit 3727cb2 into krzmaz:main Oct 9, 2023
@SebKuzminsky SebKuzminsky deleted the find-credentials branch October 9, 2023 18:32
@SebKuzminsky
Copy link
Contributor Author

Thanks @krzmaz :-)

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.

2 participants