Setting -u in stdenv to catch invalid variables

[peterhoeg]

Issue description

I propose we do set -u in stdenv to ensure that we catch references to unset variables and blow up accordingly.

One example is this fix: 3f56114

Previously the derivation referenced $pname which should have been ${pname}. In this case, the impact was rather limited admittedly, but there is in all likelihood a number of bugs lurking because files end up in the wrong directory.

Changing this will probably cause a lot of breakage (assuming this is a widespread issue).

Cc: @Ericson2314 @Profpatsch @NeQuissimus @vcunat @LnL7

[joachifm]

As I recall this was already proposed but one argument against was the amount of noise required to handle potentially uninitialized variables (i.e., ${foo:-} all over the place).

[Ericson2314]

Actually we do. I did merge #28057. Careful use of set +u prevents package's hooks from also being set -u'd, as that would be a huge breaking change.

I would be for doing that too, but this will be a significant undertaking. This is probably best paired with Nix defining env vars (it doesn't right now), so that more variables are always defined.

[joachifm]

@Ericson2314 ah, that's the one I was thinking of, I hadn't noticed it ended up being merged, so much has happened with stdenv lately :)

[Profpatsch]

A first step would be for somebody to find out how many packages do break when set -u is enabled globally.

[peterhoeg]

I wasn't aware of the earlier PR.

This is probably best paired with Nix defining env vars (it doesn't right now), so that more variables are always defined.

Such as?

[Ericson2314]

A few from memory: configureFlags, nativeBuildInputs, buildInputs when empty; dontConfigure dontBuild doCheck, doInstallCheck when false.

[copumpkin]

We need a set -u for everything but a set of variables that are allowed to be unset 😄

[peterhoeg]

Surely it wouldn't be that noisy to change references to configureFlags and the likes to ${configureFlags:-""}. Or am I grossly underestimating this?

[Ericson2314]

@peterhoeg you aren't---I think it's worth it either way.

[peterhoeg]

Another option would be to use an approach similar to what the hardening guys did so that each derivation can simply disable the "pedanticHook" in case it causes problems. Of course, talk is cheap.... ;-)

[Ericson2314]

Somebody that isn't me should take the lead on this :). Grep for set +u and remove them all, then see what happens!

[stale]

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
2. Ask on the NixOS Discourse.
3. Ask on the #nixos channel on irc.freenode.net.

[peterhoeg]

Closing as @Ericson2314 is the man!

Ref #72347.