coreboot-toolchain: remove hardcoded hash of 21.11's x86_64-linux.stdenv#171223
Conversation
13 tasks
felixsinger
reviewed
May 2, 2022
ghost
commented
May 2, 2022
ghost
commented
May 2, 2022
felixsinger
suggested changes
May 2, 2022
Member
felixsinger
left a comment
There was a problem hiding this comment.
Commit messages missing and problem statement needs to be improved.
Author
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. |
4 tasks
Author
|
Ping... |
Author
|
Force-push resolves merge conflict with 7aae279, no other changes. |
Author
|
Ping... |
4 tasks
x86_64-linux.stdenv
13 tasks
Author
|
Ping... |
Author
|
Rebased since #177326 has merged. |
added 2 commits
June 23, 2022 22:53
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.
Author
|
Ping... |
Member
|
To reproduce the issue I recommend simply |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(@felixsinger, thank you so much for writing the
coreboot-toolchainexpression!)The problem
The following line is found in
tools/misc/coreboot-toolchain/default.nix:This hash matches the output of the
fetchgitinvocation in the same file; however thatfetchgitinvocation uses apostFetchblock withpatchShebangs, so the nix store-path ofstdenv.shellis now hardwired into the hash above.This means that this derivation will fail to build for any user whose
stdenvstore-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
--options 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 = trueset 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.shto update generated release notes