stdenv: fix withCFlags

[andresilva]

Some derivations set NIX_CFLAGS_COMPILE directly as args (i.e. not in env) which results in:

error: The ‘env’ attribute set cannot contain any attributes passed to derivation. The following attributes are overlapping: NIX_CFLAGS_COMPILE

(There are more adapters that need the same handling but probably makes sense to come up with a more general approach of merging args and env.args).

Related #306953.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[emilazy]

Perhaps it is better to do this in shell and use concatTo or similar? cc @wolfgangwalther @ShamrockLee

[wolfgangwalther]

I remember reading a discussion somewhere, but can't find the source right now, where it was argued that env should only contain strings, not lists etc. - the "caller" should make sure to turn lists into strings already.

In general, we should analyze the case of NIX_CFLAGS_COMPILE and decide whether it really is supposed to be an environment variable (because it needs to be passed on to other processes than stdenv) or whether it is actually an argument to stdenv / hooks only. Then we should standardize on one way to call it.

I suspect that it should not really be an environment variable, but people think it is, because of its capitalized spelling.

[andresilva]

I remember reading a discussion somewhere, but can't find the source right now, where it was argued that env should only contain strings, not lists etc. - the "caller" should make sure to turn lists into strings already.

I briefly looked at usages of NIX_CFLAGS_COMPILE and it was treated as a string, but there are hundreds of usages and I didn't check all of them.

[ShamrockLee]

I suspect that it should not really be an environment variable, but people think it is, because of its capitalized spelling.

I thought NIX_CFLAGS_COMPILE were an environment variable taken by the patched/wrapped CC. Is it?

[wolfgangwalther]

I thought NIX_CFLAGS_COMPILE were an environment variable taken by the patched/wrapped. Is it?

Yes, it is currently used as an environment variable by the CC wrapper. That's because it needs to pass the boundary between stdenv and the wrapper - different process, so just a bash variable won't work. This means, that every NIX_CFLAGS_COMPILE set outside env will not work with __structuredAttrs.

Now, imho, we have two ways out:

• Move all those to env.NIX_CFLAGS_COMPILE and only support that.
• Make NIX_CFLAGS_COMPILE a non-environment variable, i.e. somehow make sure it's passed to CC wrapper through a different channel than environment variables. This would then make it a "regular" drv arg, which would be supported with both structuredAttrs and without. Although.. that would require the reverse migration, i.e. all env.NIX_CFLAGS_COMPILE would need to be come regular arguments again. And the capitalized naming would also not be very good anymore...

I don't have a concrete proposal for the latter and while the former is some migration effort, it's probably the more sensible option.

In any case, that means that we should not do what is proposed in this PR, i.e. we should not support both NIX_CFLAGS_COMPILE with and without env..

---

Once we agree that NIX_CFLAGS_COMPILE should always be in env, we need to discuss the format of env. Should you only be able to pass strings to stuff in env - or should you have more "structured" options available as well, like lists? The current PR explicitly supports lists for env.NIX_CFLAGS_COMPILE as well - and I think we should not do that either.

The whole mess around structuredAttrs originated exactly in this: Without structuredAttrs, structured nix-level drv args were passed as environment variables in bash. This caused all the problems! With the invention of both structuredAttrs (for drv args) and env for environment variables, which are by definition only string, the mess was will eventually be cleaned up.

Thus, I think we should be very explicit and only accept pure string in env.NIX_CFLAGS_COMPILE (and all other env.*).

TLDR: I don't think we should do what is proposed here at all.

---

The problem statement is this:

Some derivations set NIX_CFLAGS_COMPILE directly as args (i.e. not in env) which results in

Which is exactly the thing that we need to clean up, also for structuredAttrs support, as mentioned above. Let's just get rid of that and withCFlags will work as well.

[wolfgangwalther]

Also related: #217206 - a migration to env.NIX_CFLAGS_COMPILE had already been done once, but I guess we'll need to enforce this.

[andresilva]

I'm closing this as there doesn't seem to be any consensus (yet) on how this should be done. Someone else can pick this up if they're interested.