-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Qt 5: Fix debug flags #34047
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
Qt 5: Fix debug flags #34047
Conversation
NIX_CFLAGS_COMPILE = | ||
let arg = args.NIX_CFLAGS_COMPILE or []; in | ||
optional (debug == true) "-DQT_NO_DEBUG" | ||
++ (if builtins.isList arg then arg else [arg]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there is lib.toList
for this purpose.
Do we want to reset the commits that we have created to circumvent the bug? |
else "-DCMAKE_BUILD_TYPE=Release"); | ||
++ [ | ||
"-DBUILD_TESTING=OFF" | ||
''-DCMAKE_BUILD_TYPE=${if debug then "Debug" else "Release"}'' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use apostrophes instead of double quotes?
I don't think that's necessary. The only practical difference between |
If ‘mkDerivation’ is passed ‘NIX_CFLAGS_COMPILE’, we should include those flags along with the common flags. See also: NixOS#34039 NixOS#34038 NixOS#33935 NixOS#33933 NixOS#33930 NixOS#33927
9a0e724
to
4a39533
Compare
Motivation
Setting debug flags for Qt 5-based builds incorrectly overrides
NIX_CFLAGS_COMPILE
.Incidentally I decided that
debug
should betrue
orfalse
, but nevernull
. (What would that even mean?)Testing
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)