Remove NIX_CFLAGS_COMPILE where possible

[nh2]

Problem

NIX_CFLAGS_COMPILE is a low-level hack that sneaks compiler arguments past packages' build systems.

It has various drawbacks:

• the build system cannot see the flag, thus intended upstream logic in the build system is potentially not activated
• generated files, such as pkg-config .pc files, may lack the necessary entries, making problems for downstream packages
• the flag does not show up in the build logs, making it unclear what's happening

Because many packages use NIX_CFLAGS_COMPILE, more packages get it added when people copy this style.

Solution

It is better to pass CFLAGS, CXXFLAGS and so on directly to the build system via the method intended to pass compiler flags, for example (feel free to edit this to add more build systems):

• Autoconf: By setting environment variables like CFLAGS before the ./configure invocation (e.g. by writing CFLAGS = "..."; into mkDerivation)
• CMake: By setting environment variables like CFLAGS before the cmake invocation (see also cmake setup hook: Allow passing environment variables to cmake invocation #79302)
• Meson: -Dc_args (see also cinnamon.nemo: init at 4.4.1 #77979 (comment) and Report undeclared gio-unix-2.0 upstream #36468 (comment))

Delivery

I think we should run a nixpkgs-wide sprint (like #79064) to do this switch for as many packages as possible.

We should also update the documentation to recommend better alternatives to this hack (like above).

[jtojnar]

Note that many of these are actually upstream bugs so in addition to changing the expressions, we should report the issues. See, for example, #36468

[blaggacao]

I was just about to copy one such instance of NIX_CFLAGS_COMPILE it is so absolutely widespread that it appears to be a "dominant" pattern.

$ rg 'NIX_CFLAGS_COMPILE' | wc -l
705

[jtojnar]

@blaggacao Yes, it is a very common hack. It would still be better if maintainers went upstream for their packages and opened issues or merge requests with a proper fix, like we do with #36468.

[nh2]

Often it's also not an upstream problem but straight-out unnecessarily done in nixpkgs, as described in the Solution section in the issue description.

[blaggacao]

@nh2 Would you want to do a regex mass commit (quickly) that at least prepends the ocurrances with a comment and link to this issue? That would at least stop further propagation. Good/bad idea?

[nh2]

@blaggacao That sounds like a great idea to me!

[edolstra]

I'm not convinced getting rid of NIX_CFLAGS_COMPILE is a good idea. NIX_CFLAGS_COMPILE is consistent and reliable, unlike figuring out the magic incantation (often silently ignored) to tell the build system to pass a flag to the C compiler.

[blaggacao]

@edolstra Is there a way a consistent policy can be drafted that does justice to the motivations of the author and also to the inherent values of the flag? Maybe such draft could be the comment to be regex-commited and further if not avoidance, but so awareness. (Blindly copy in my opinion is something that is happening — it happend to me)

[nh2]

NIX_CFLAGS_COMPILE is consistent and reliable

@edolstra Well, my argument in the issue description is exactly the opposite: By sneaking flags past the build system, things actually stop working as expected.

I've lost many working days figuring out why some programs don't link, only to find out that one of my dependencies snuck an -L flag past the build system with NIX_CFLAGS_COMPILE. This happens especially when using C/C++ dependencies from other languages.

(Also, if we followed your argument all the way through, we might as well get rid of buildInputs, the pkgconfig hook and so on, and sneak all the dependencies past the build systems whose job it is to manage them.)

figuring out the magic incantation (often silently ignored) to tell the build system to pass a flag to the C compiler

Is that really a practical problem? Most build systems make it straightforward to do this today (and for the big ones like Autoconf, CMake, and Meson I've written in the description what the straightforward way is). It's similarly easy for non-C toolchains that may invoke C compilers on the way, like Haskell.

Also, this issue is not suggesting to outright forbid the use of NIX_CFLAGS_COMPILE (there may still be cases where it's unavoidable), but to do the grunt work of getting rid of hacks where there's a clean way to do it.

[blaggacao]

@nh2 Maybe you would be able to use unsafeGetAttrPosition for a mass edit, like exemplified here: https://paper.dropbox.com/doc/nixpkgs-Merge-Queue-Easy-Mark-Broken--A~tOgAlNhmvXMIdnK2nqZok_Ag-A232tyO0zLDbUZ7zhAefX for me, personally, it would be interesting to learn more about how to use this tool, so I'd be happy to watch.

[7c6f434c]

(Also, if we followed your argument all the way through, we might as well get rid of `buildInputs`, the `pkgconfig` hook and so on, and sneak _all_ the dependencies past the build systems whose job it is to manage them.)

Not buildInputs, because the main thing it does is setting NIX_CFLAGS_COMPILE and NIX_LDFLAGS. Which it ususally does correctly and it is enough, but some build systems fail at noticing the dependencies are already available and need pkgconfig hooks.

[Enteee]

I am actually completely with @edolstra on this subject. I just spent half a day trying to build sqlite DSO's with -fsanitize=address and -shared-libasan. The latter is silently dropped by libtool when linking. And I have absolutely no clue why. Probably some globbing or sed issue in the 12556 lines bash madness this build system consist of. Thanks to the power of NIX_CFLAGS_COMPILE and especially NIX_LDFLAGS this was rather easy to resolve.

Don't get me wrong. Using the capabilities of the build system first is a good thing. And I am all-in for removing NIX_CFLAGS_COMPILE and NIX_LDFLAGS whenever possible. But sometimes we just need a sledgehammer to make an uncooperative build system obey.

[edolstra]

Yup. Last week I had to pass a #define to a glibc source file. I could have spent a few hours figuring out glibc's build system, patch the makefiles and configure scripts, etc. - or just add a line to set NIX_CFLAGS_COMPILE.

[AleXoundOS]

I haven't decided if NIX_CFLAGS_COMPILE good or not. But another case where it causes confusion is generating compilation database aka compile_commands.json for IDE. Build systems (e.g. cmake) write explicitly passed compiler flags to compile_commands.json and miss NIX_CFLAGS_COMPILE. At least, a documented workaround should be provided for major build systems (avoiding patching sources). Indirectly related to #79302.

[AleXoundOS]

Interestingly, NIX_CFLAGS_COMPILE carries some -frandom-seed=0czq9rsaa1 value which hints for the sake of reproducibility we cannot remove NIX_CFLAGS_COMPILE (fully).

[AleXoundOS]

As for compile_commands.json compilation database generation, the workaround is to add export CFLAGS="$NIX_CFLAGS_COMPILE" to shellHook for nix shell. At least, works for cmake.

[danielbarter]

I've been thinking about this a little recently, and I personally don't have any issue with the NIX_CFLAGS_COMPILE pattern. Yes, it is quite different to how things work on say, debian, but its kind of a drop in the bucket. I do feel like we could do some work to improve how we are setting it though. For example, given a shell.nix like

with (import <nixpkgs> {});

(mkShell.override {stdenv = llvmPackages.stdenv;}) {
  buildInputs = [
    cmake
    gtest
    python3
    python3Packages.pybind11
  ];
}

the resulting NIX_CFLAGS_COMPILE is

-frandom-seed=546fwp0217 
-isystem /nix/store/34x96xyjic8dwq51bnvm4x844pp5x01w-compiler-rt-libc-11.1.0-dev/include
-isystem /nix/store/d1mprylqwl2lgjg7ng7svz3ryvf5m563-gtest-1.11.0-dev/include 
-isystem /nix/store/pn7863n7s2p66b0gazcylm6cccdwpzaf-python3-3.9.13/include 
-isystem /nix/store/1b3x0mc9n5fjl5lwnp55r7lhkllyyrda-python3.9-pybind11-2.9.2/include 
-isystem /nix/store/34x96xyjic8dwq51bnvm4x844pp5x01w-compiler-rt-libc-11.1.0-dev/include
-isystem /nix/store/d1mprylqwl2lgjg7ng7svz3ryvf5m563-gtest-1.11.0-dev/include 
-isystem /nix/store/pn7863n7s2p66b0gazcylm6cccdwpzaf-python3-3.9.13/include
-isystem /nix/store/1b3x0mc9n5fjl5lwnp55r7lhkllyyrda-python3.9-pybind11-2.9.2/include

As you can see, we are getting all the -isystem includes twice. Moreover, I feel like there are a bunch of standard header locations which I often end up passing into build systems and clangd via my own environment variable:

  CLANGD_PATH = builtins.concatStringsSep ":" [
    # C++ stdlib headers
    "${llvmPackages.libcxx.dev}/include/c++/v1"
    # libc headers
    "${clang.libc_dev}/include"
    # compiler specific headers like AVX headers
    "${clang}/resource-root/include"
  ];

which evaluates to

$ echo $CLANGD_PATH
/nix/store/i3zvlj1ig56ifdh805j2b7vk2cxd6bwy-libcxx-11.1.0-dev/include/c++/v1:
/nix/store/k3d8wqlsnmm5270zd19cbs26g7wifxj6-glibc-2.34-210-dev/include:
/nix/store/dk8bcp4cn373yci93xp94nl66pvsy0ap-clang-wrapper-11.1.0/resource-root/include

On my current system. It would be nice to add these to NIX_CFLAGS_COMPILE

[danielbarter]

OK, so adding these to NIX_CFLAGS_COMPILE doesn't expose them to clangd. Adding them to CPATH does expose them to clangd as expected. Can someone point me towards where we are wrapping clangd so that it reads NIX_CFLAGS_COMPILE as described in https://nixos.wiki/wiki/C#Editor.2FIDE_integration?

[nekoniaow]

From: chaincodelabs/libmultiprocess#75 (comment)

The issue is that util.cpp depends on capnp headers, but the current build doesn't explicitly add the capnp include directory. On my system, the bug was hidden because I'm using a nix environment, and just adding the capnproto dependency to it seems to automatically set CMAKE_INCLUDE_PATH and NIX_CFLAGS_COMPILE to include the capnp include directory. But if I manually override these I see the same error you reported, and the fix above is necessary.

In light of this example I fail to see how the use of NIX_CFLAGS_COMPILE is not a problem since it can hide the fact that our own build setup is incorrectly specified.

Using it locally to solve temporary problems is completely understandable but those changes, once submitted, compromise the environment of depending packages which seems to go against the Nix philosophy.

[7c6f434c]

Well, Nixpkgs is about building the software even if upstream build system assumes too much, not about making the packagers suffer because upstream developers can't be bothered.

Also it looks like CMAKE_INCLUDE_PATH could have solved the problem on its own. Nixpkgs should assume that if you add packages to buildInputs you want them available to the compiler, otherwise nothing will work with an amount of effort that is anywhere near acceptable.

[zopsicle]

Nixpkgs should assume that if you add packages to buildInputs you want them available to the compiler.

Nixpkgs is not only used for packaging software for Nixpkgs, it is also used for obtaining development environments with nix-shell, for developing software that might be used on non-Nix platforms. In this case, the compiler implicitly finding headers due to -isystem flags in NIX_CFLAGS_COMPILE can mask a poorly configured build system (such as a find_package call missing in CMakeLists.txt). In fact, in the case of nix-shell (apart from the other use case of nix-shell that is debugging Nixpkgs packages), poorly configured build systems should cause the build to fail, instead of Nixpkgs “helpfully” inserting workarounds for such misconfigurations, so that the developer can notice and correct them.

[7c6f434c]

nix-shell, as a buildsystem-universal tool, provides an environment intended to be comfortable for developing the software no matter what buildsystem it uses, including the case it uses a makefile of «just install things globally» variety, and including the case of a shell script launching different build systems in different subdirectories. This is useful and important, and in conflict with the goals you describe.

An environment tuned to a specific choice of buildsystem is a useful thing, but it probably should be defined separately per-build-system, and it is a different thing from nix-shell.

To go to an extreme example, we suppress some -Werror flags in builds in Nixpkgs instead of patching the warning reasons. Maybe we should have «DeathStation 9000» mode environments easily available, but we will not use them by default.

[nh2]

I don't think the distinction in the above 2 posts matters for what I'm proposing:

That if there's a cleaner way to pass something to the build system, use that instead of the NIX_CFLAGS_COMPILE hack.

In cases where there is no other way, using the NIX_CFLAGS_COMPILE hack is fine, but it should be commented in packages why the hack is needed (e.g. # Upstream build system does not have a configure flag for library X).

So there should be no difference in buildability.

The point of my issue description is that there are many cases in nixpkgs where the hack is used unnecessarily, and that that leads to the hack being copy-pasted around too much.

[7c6f434c]

I don't think the distinction in the above 2 posts matters for what I'm proposing:

That if there's a cleaner way to pass something to the build system, use that instead of the NIX_CFLAGS_COMPILE hack.

It does. Given that build systems are complicated, messy, and when they change they change more dramatically than the actual code, NIX_CFLAGS_COMPILE is actually the clean option. That's why buildInputs first of all does that, and then maybe possibly some hooks pass stuff for build systems.

[nekoniaow]

I don't think the distinction in the above 2 posts matters for what I'm proposing:
That if there's a cleaner way to pass something to the build system, use that instead of the NIX_CFLAGS_COMPILE hack.

It does. Given that build systems are complicated, messy, and when they change they change more dramatically than the actual code, NIX_CFLAGS_COMPILE is actually the clean option. That's why buildInputs first of all does that, and then maybe possibly some hooks pass stuff for build systems.

This misses @nh2 's point which is that those usages should be removed when they are not necessary.

Your point seems to be that NIX_CFLAGS_COMPILE should never be removed in any case because it's the only way that works.

[7c6f434c]

This misses @nh2 's point which is that those usages should be removed when they are not necessary.

No it doesn't. Even if it is strictly speaking not necessary, but making things work without it takes two hours of debugging instead of ten minutes, and this exercise will need to be repeated once a year instead of once in five years — let's not jump the hoops when we have a tool to just demolish them.

Your point seems to be that NIX_CFLAGS_COMPILE should never be removed in any case because it's the only way that works.

Well, if the most obvious thing that comes to mind literally in the first minute of looking at the expression works fine from at most the second attempt, sure, let's use it instead of NIX_CFLAGS_COMPILE. Otherwise, let's use the more predictable approach for Nixpkgs packages.

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/single-cmakebuildtype-shared-between-derivation-and-dependency/35184/5

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/single-cmakebuildtype-shared-between-derivation-and-dependency/35184/6

[allsey87]

While I understand the arguments for not using NIX_CFLAGS_COMPILE in recipes, I do not think it should be removed since it provides a way of consistently applying flags across all libraries and binaries during cross compilation (for example, march etc).

[nh2]

While I understand the arguments for not using NIX_CFLAGS_COMPILE in recipes, I do not think it should be removed

I don't think anybody is suggesting for this feature to be removed.

The idea is that if there's a clean way to pass an option to the build system, that should be the default.

[LunNova]

Is there a correct way to disable includes getting added to NIX_CFLAGS_COMPILE for buildInputs in a stdenv or in a derivation?

I'm working on packaging rocm 6.2.2. I'd like to turn off the automatic include paths since cmake and pkg-config should be handling all that anyway - in practice we just get a giant pile of duplicate includes passed to clang.

So far I didn't find the correct way to turn it off. I'm assuming there is one since nix hook stuff is pretty flexible and I'm just doing it wrong.

Adding this to the compiler wrapper allows turning it off for that specific wrapper but it's super jank :(

substituteInPlace $out/nix-support/setup-hook --replace-fail '1/include"' '1/include-DONOTUSE"'