-
-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
coreboot-toolchain: remove hardcoded hash of 21.11's x86_64-linux.stdenv
#171223
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit messages missing and problem statement needs to be improved.
Done. Mainly the middle commit; the first commit is a straightforward one-line code change. The last commit fixes the issue described in the middle commit, so the last commit's message refers the reader to the previous commit's message. |
Ping... |
Force-push resolves merge conflict with 7aae279, no other changes. |
Ping... |
x86_64-linux.stdenv
Ping... |
Rebased since #177326 has merged. |
This commit exposes a bug in the coreboot-toolchain expression; if you nix-build that expression at this commit and do not use subsituters, you will get a failure like this: error: output '/nix/store/nvswbzyl39ifpwswfvx132j2mys80ifr-coreboot' is not allowed to refer to the following paths: /nix/store/8ngciqnw8jzvyvbx00arkp05gvn5q6sq-libunistring-1.0 /nix/store/p2r9ynirymj47x5m6y9pnq0lpssn4ahm-bash-5.1-p16 /nix/store/rflgyvwcnmrql5wf8kchynmmq7raggvj-libidn2-2.3.2 /nix/store/rszg7d581z3v3fwrak68ba2wv5lrckx7-glibc-2.34-115 The root cause of the bug is this line in tools/misc/coreboot-toolchain/default.nix: sha256 = "073n8yid3v0l9wgwnrdqrlgzaj9mnhs33a007dgr7xq3z0iw3i52" This hash covers the result of the fetchgit operation, including the postFetch block. The postFetch block runs patchShebangs, which writes the store-path of ${stdenv.shell} into $out/util/crossgcc/buildgcc *before* computing its hash. The next commit after this one fixes the bug by moving patchShebangs out of the postFetch block, so it happens *after* the hash is computed. Note that even without allowedRequisites=[], the state of the code prior to this commit is problematic. Because it hardcodes the store-path of stdenv, the expression will break in these situations: * Building on a platform other than x86_64-linux, since those platforms will have a different boostrap-files store-path and therefore a different stdenv store-path. * Building with an overlay which adds -march= or other compiler flag customizations to stdenv. * Future nixpkgs users if nixpkgs commits any change which influences the store-path of stdenv. So we should fix the problem in any event. The `allowedRequisites=[]` added by this commit ensures that the problem will be noticed immediately if it recurs in the future.
This commit fixes the issue described in detail in the commit message of the previous commit.
Ping... |
To reproduce the issue I recommend simply
|
(@felixsinger, thank you so much for writing the
coreboot-toolchain
expression!)The problem
The following line is found in
tools/misc/coreboot-toolchain/default.nix
:This hash matches the output of the
fetchgit
invocation in the same file; however thatfetchgit
invocation uses apostFetch
block withpatchShebangs
, so the nix store-path ofstdenv.shell
is now hardwired into the hash above.This means that this derivation will fail to build for any user whose
stdenv
store-path does not exactly match the one used at the time the derivation was first built by hydra. For example:Anybody building on a platform other than
x86_64-linux
, since they have a differentboostrap-files
.Anybody with an overlay which adds
-march=...
Any future nixpkgs users, if nixpkgs commits a change which influences the store-path of
stdenv
.See for yourself
To reproduce the problem, check out this PR, revert the last commit, and:
Some of those
--option
s are probably redundant, but I can never seem to get nix's rules for substituters straight.Assuming you didn't already have the outpath in your store (if you did, delete it, start over), you'll see:
The fix
A three-patch series:
fetchgit: allow passing allowedRequisites through to stdenv.mkDerivation (this commit is fetchgit: inherit allowedRequisites in mkDerivation #177326).
coreboot-toolchain: add
allowedRequisites=[]
coreboot-toolchain: move patchShebangs out of fetchgit, update the sha256.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes