{nixos-container,lsb-release}: use substitute rather than substituteAll, fix cross-compilation

[philiptaron]

Description of changes

This change fixes cross-compilation of the lsb-release and nixos-container scripts, all of which used the stdenv environment variable shell instead of runtimeShell string as the shebang... leading to a cross-compilation mess.

There's no other change in produced output. Rationale for using substitute instead of substituteAll is here:

• substituteAll is a terrible function and we need to stop using it / fix it respectively #237216

Previously:

• nixos-rebuild: use substitute rather than substituteAll #300308

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• 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/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 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]

The shell substitutions look like the value would come from buildPlatform, not hostPlatform (the default) like the other substitutions. Does it really exist? hello.drvPath doesn't have it. Otherwise I wouldn't be surprised if these packages were broken when cross-compiled.

[philiptaron]

The shell substitutions look like the value would come from buildPlatform, not hostPlatform (the default) like the other substitutions. Does it really exist? hello.drvPath doesn't have it. Otherwise I wouldn't be surprised if these packages were broken when cross-compiled.

Right you are. I did the following on master:

nix-build -A pkgsCross.aarch64-multiplatform-musl.deterministic-uname

Then looked at the shebang at the top of the result (which is where @shell@ expands):

/nix/store/a1s263pmsci9zykm5xcdf7x9rv26w6d5-bash-5.2p26/bin/bash

Then used file:

$ file /nix/store/a1s263pmsci9zykm5xcdf7x9rv26w6d5-bash-5.2p26/bin/bash
/nix/store/a1s263pmsci9zykm5xcdf7x9rv26w6d5-bash-5.2p26/bin/bash: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /nix/store/ddwyrxif62r8n6xclvskjyy6szdhvj60-glibc-2.39-5/lib/ld-linux-x86-64.so.2, BuildID[sha1]=1098c4d83aff81fd6ddd5bc06fb9442388f4f51d, for GNU/Linux 3.10.0, not stripped

Definitely wrong.

It looks like the correct thing to do is use runtimeShell.

[ghost]

while substituteAll implementation may be terrible, the interface is much nicer than using substitute. couldn't substitueAll be re implemented so that the key / value pairs just passed to substitute using the --subst-var-by flag? like:

# some nix code 
substituteAllish {
 src = ./the/file;
 substitutions = {
   inherit coreutils;
   stuff = someComputation;
   etc.
 };
};

# substituteAllish mock implementation
{ lib, src, substitutions):
let
subs =  lib.flatten ( map (pair:  [ "--subst-var-by" pair.name pair.value ]) (lib.attrsets.attrsToList substitutions ));
in
substitute { inherit src; substitutions = subs; etc. }

?

[philiptaron]

I've been cooking exactly that patch @a-n-n-a-l-e-e. My hope is that I'll get through enough of these substituteAll refactorings to get a sense of how often there's a defect embedded in them to recommend a secondary approach. So far, there's basically a defect per patch.

Edit: made a PR with the cooked patch: #301350

[philiptaron]

Let's collaborate on what better would look like on #237216.