substitute-all.nix: Define in terms of the substitute bash function

[raphaelr]

Description of changes

The substituteAll bash function that was previously used substitutes all environment variables. This can cause surprising behaviour, because some environment variables, like system, and not under control of the expression author that calls substituteAll.

This commit changes the nixpkgs substituteAll function to use substitute with an explicit variable list instead. This ensures that the only variables that will be substituted are those that the caller specified, and not any that stdenv.mkDerivation or Nix itself added/modified.

Partially addresses #237216.

TODO:

• Should this change be added to the release notes? This change can break users that depend on being able to substitute e.g. @system@ without explicitly passing it to substituteAll
• This function is now very similar to the substitute nixpkgs function, but more ergonomic to use (substitute expects a list of arguments to the bash substitute function, while substituteAll accepts an attrset of variables to replace). Should one of them be removed?

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• 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/)
• 23.11 Release Notes (or backporting 23.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.

[roberth]

removeAttrs is still kinda terrible.
It'd be nicer to first have a function that does the right thing for all possible key value combinations, and then define substituteAll in terms of that function for compatibility.

The interface could be something like this when called

substituteStrings {
  nativeBuildInputs = [ foolint ];
  checkPhase = ''
    runHook preCheck;
    barlint $out;
    runHook postCheck;
  '';
  replacements = {
    "@foo@" = lib.getExe foo;
  };
};

(note that I'm also making the @s explicit for better grepping)

[roberth]

The above could be done with --replace and escapeShellArg I think.

[thiagokokada]

My 2c: I think we could have a new interface but at the same time having a better substituteAll that does what it is supposed to do would be much better.

[raphaelr]

I do not currently have time to work on this. Closing this.