Skip to content
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

Jump to bottom

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

Merged
merged 1 commit into from Jan 31, 2021
Merged

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

merged 1 commit into from Jan 31, 2021

Conversation

eadwu
Copy link
Member

@eadwu eadwu commented May 15, 2020 •
edited

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.

@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 delroth and bendlas May 15, 2020 03:08
@eadwu eadwu mentioned this pull request May 15, 2020
Closed
10 tasks
@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
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
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`.
hmenke
hmenke approved these changes Jan 20, 2021
View reviewed changes
Copy link
Member

@hmenke hmenke left a comment •
edited

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

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

@hmenke hmenke hmenke approved these changes

@delroth delroth Awaiting requested review from delroth

@bendlas bendlas Awaiting requested review from bendlas

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@eadwu @wizeman @Ekleog @lopsided98 @hmenke @veprbl