Skip to content

nixos/kernel.nix: ensure same kernel is used#87856

Merged
veprbl merged 1 commit intoNixOS:masterfrom
eadwu:kernel/same-kernel
Jan 31, 2021
Merged

nixos/kernel.nix: ensure same kernel is used#87856
veprbl merged 1 commit intoNixOS:masterfrom
eadwu:kernel/same-kernel

Conversation

@eadwu
Copy link
Member

@eadwu eadwu commented May 15, 2020 •
edited
Loading

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 15, 2020
@eadwu eadwu mentioned this pull request May 15, 2020
Closed
10 tasks
@eadwu eadwu marked this pull request as ready for review May 15, 2020 03:08
@eadwu eadwu requested review from bendlas and delroth May 15, 2020 03:08
@eadwu eadwu mentioned this pull request May 15, 2020
Closed
10 tasks
@eadwu eadwu force-pushed the kernel/same-kernel branch from 068f0d7 to 6e2f31b Compare May 23, 2020 15:39
@wizeman
Copy link
Member

wizeman commented Jun 19, 2020

I can't tell if this is doing the right thing because I get confused with the overriding mechanisms.
However, I was having the issue described in #86835, which for me meant NixOS wouldn't boot since I was using ZFS boot and root filesystems.
This PR fixed the issue for me.

@Ekleog
Copy link
Member

Ekleog commented Jun 26, 2020

Would it be possible to git commit --amend to explain why this is required, and how this solves the listed issue?

@eadwu eadwu force-pushed the kernel/same-kernel branch from 6e2f31b to 86456e8 Compare June 27, 2020 16:46
@eadwu
Copy link
Member Author

eadwu commented Jun 27, 2020

Yeah a better fix would probably be fixing the extend override, but I'm not sure it works.

@eadwu eadwu force-pushed the kernel/same-kernel branch from 86456e8 to 12acc62 Compare July 20, 2020 02:18
@eadwu eadwu force-pushed the kernel/same-kernel branch from 12acc62 to a32c412 Compare August 11, 2020 15:55
@eadwu eadwu force-pushed the kernel/same-kernel branch from a32c412 to d8c32c3 Compare September 8, 2020 15:59
@eadwu eadwu force-pushed the kernel/same-kernel branch from d8c32c3 to ce5eec9 Compare September 26, 2020 15:24
@eadwu eadwu force-pushed the kernel/same-kernel branch from ce5eec9 to 33aaae5 Compare October 7, 2020 18:00
@eadwu eadwu force-pushed the kernel/same-kernel branch from 33aaae5 to a34f5a3 Compare October 29, 2020 20:15
@eadwu eadwu force-pushed the kernel/same-kernel branch from a34f5a3 to 9b3dea9 Compare November 16, 2020 19:08
@eadwu eadwu force-pushed the kernel/same-kernel branch from 9b3dea9 to 0d274c3 Compare December 21, 2020 06:07
@eadwu
nixos/kernel.nix: ensure same kernel is used
78f915a 
Originally, changes to the kernel don't propagate to the other
derivation within the same package set. This commit allows for the
changes in the kernel to be propagated.

A distinct example is setting `boot.kernel.randstructSeed` to a non-zero
length string which would result in building 2 kernels, one with the
correct seed and the other with the zero length seed. Then, when using
an out-of-tree kernel driver, it would be built with the zero length
seed which differs from the non-zero length seed used to boot,
contradicting the purpose of the `boot.kernel.randstructSeed`.
@eadwu eadwu force-pushed the kernel/same-kernel branch from 0d274c3 to 78f915a Compare January 11, 2021 18:08
hmenke
hmenke approved these changes Jan 20, 2021
View reviewed changes
Copy link
Member

@hmenke hmenke left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The careless use of super is a common antipattern that you also see a lot in user overlays on the internet. If in an overlay you do

self: super: {
  foobar = super.stdenv.mkDerviaton { ... };
}

this will prevent the user from overriding stdenv for that derivation and it's no longer possible to, e.g. build foobar with a different C compiler. Overrides are simpler and don't have this problem. They compose and you can even do things like this

((nixos-shell.override { nix = nixFlakes; }).override { nix = nixStable; }).override { nix = nixUnstable; }

That's exactly what this PR proposes and therefore I approve!

@veprbl veprbl merged commit 8468a98 into NixOS:master Jan 31, 2021
@lopsided98 lopsided98 mentioned this pull request Feb 4, 2021
Merged
10 tasks
@lopsided98
Copy link
Contributor

lopsided98 commented Feb 4, 2021

This significantly changes the semantics of the boot.kernelPackages option, causing all the attributes of the linuxPackages package set to be ignored except for the kernel attribute. If a user specifies a package set with customization other than to kernel, those extra customizations are just ignored now.

See #111845 for a fix for the original problem with .extends.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@delroth delroth Awaiting requested review from delroth

@bendlas bendlas Awaiting requested review from bendlas

1 more reviewer

@hmenke hmenke hmenke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@eadwu @wizeman @Ekleog @lopsided98 @hmenke @veprbl