Skip to content

Conversation

@adsnaider
Copy link

Not a problem when building with cargo proper but when we are using rules_rust in bazel, it uses our C toolchain which we use with -Wundef and -Werror, causing a compilation failure. This fixes the issue by using the defined preprocessor statement (https://gcc.gnu.org/onlinedocs/cpp/Defined.html)

@nagisa
Copy link
Member

nagisa commented Jan 27, 2026

Thank you for contributing. This change still needs test coverage.

@adsnaider
Copy link
Author

Sure. @nagisa what are you hoping to test exactly?

@nagisa
Copy link
Member

nagisa commented Jan 27, 2026

Well, if there are conditions under which the build fails, then a test added here would have to be failing before this change and passing after it. This probably can be as simple as somehow forcing -Wundef -Werror in one/all of the gh workflows, possibly via a CFLAGS or similar env var that the cc crate looks at.

@adsnaider adsnaider force-pushed the psm_cfgs_compliant_cpp branch from 077d206 to 299ef97 Compare January 28, 2026 15:48
Adam Snaider added 2 commits January 28, 2026 11:01
Change-Id: I558ac060a03eb96b63f5e1697d21263c6a6a6964
@adsnaider adsnaider force-pushed the psm_cfgs_compliant_cpp branch from 299ef97 to cfe7a41 Compare January 28, 2026 16:01
@adsnaider
Copy link
Author

Done. First commit triggers the failure in the test and second commit fixes it. Unfortunately it only tests the failure is corrected in x86_64-unknown-linux-gnu because I couldn't figure out a way to set CFLAGS more broadly on all non msvc builds (aside from copying the CFLAGS_* line for a bunch of targets). I could modify it in the build.rs but that seems somewhat worse and if I put the flag in CI then the local vs. CI test behavior diverges. Happy to change to a different approach though

@adsnaider
Copy link
Author

@nagisa anything else you need here?

@nagisa
Copy link
Member

nagisa commented Feb 2, 2026

Just to find a little free time to work on this crate.

I couldn't figure out a way to set CFLAGS more broadly on all non msvc builds (aside from copying the CFLAGS_* line for a bunch of targets

Would setting the CFLAGS environment variable in .github/worklows not set the flag universally? I guess for Windows the flags might use a different format, but then we have matrices for different targets there.

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