-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Re-enable compiler checks #1097
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
Re-enable compiler checks #1097
Conversation
Add a -nostdlib specifically for the check to ensure that it builds successfully. The -nostdlib isn't needed normally, because linking against the pico SDK will add in definitions for the previously-missing function
moving to 1.5.0 as this requires quite a bit of testing on different platforms |
What's the ideal testing matrix you'd like to see here?
…On Thu, Nov 24, 2022, 10:04 Graham Sanderson ***@***.***> wrote:
moving to 1.5.0 as this requires quite a bit of testing on different
platforms
—
Reply to this email directly, view it on GitHub
<#1097 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABHY2HPYVHNUJRT7RYSYY5LWJ57WPANCNFSM6AAAAAAR4CQEGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Yeah probably want to test on
I mean we do test the platforms b4 the release, but in this case, yes, testing this up front to make sure there aren't any weird issues would be helpful (as 1.4.1 is really just going to be a bug fix release) |
I spent some time trying to test on Windows, and failing |
Can you please post your full console output @peterharperuk ? |
To be clear - I have some config problem that's unrelated to this change. I don't normally develop on Windows.
|
Ah that makes sense. I don't develop on windows either but it looks like it's trying to compile with a Microsoft toolchain against your native environment rather than a gcc targeting ARM. No suggestions here on how to fix it, but I appreciate you taking a look when I haven't had a chance to |
I tested this on Windows with NMake / Ninja and it was happy too, so looks good to me Thanks |
I'm having an issue caused by this change, but I can't figure out exactly why: my project links to an interface library and it uses
That |
It turns out that I just needed to move the |
Add a -nostdlib specifically for the check to ensure that it builds successfully. The -nostdlib isn't needed normally, because linking against the pico SDK will add in definitions for the previously-missing function.
However, when CMake checks to see what features the compiler supports, it compiles a custom program with nothing but the implicit dependencies, like
libc
. Previously, this was worked around by disabling feature detection by settingCMAKE_C_COMPILER_FORCED=TRUE
.I think the best way to fix this is to unconditionally link all programs against
pico_standard_link
in the toolchain. However, this can't be done without precompiling the library (I think--my CMake-fu is quite weak). And we don't actually need an executable that can be installed for compiler feature detection.I've tested this on Arch Linux with
arm-none-eabi-gcc-12.2.0
. I haven't tested on other systems, but this doesn't get super deep into platform-specific stuff.Fixes #1096