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

boot.kernelPackages: check for conflicts #58207

Merged
merged 3 commits into from Aug 20, 2019
Merged

boot.kernelPackages: check for conflicts #58207

merged 3 commits into from Aug 20, 2019

Conversation

teto
Copy link
Member

@teto teto commented Mar 24, 2019 •
edited

It's currently possible to set conflicting boot.kernelPackages several times
which can prove confusing.
This is an attempt to warn for this.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@teto teto requested a review from infinisil as a code owner March 24, 2019 13:12
@teto teto changed the title boot.kernelPackages: check for conflicts [WIP] boot.kernelPackages: check for conflicts Mar 24, 2019
@teto teto changed the title [WIP] boot.kernelPackages: check for conflicts [RFC] boot.kernelPackages: check for conflicts Mar 24, 2019
@teto
Copy link
Member Author

teto commented Apr 11, 2019

This helped me catch yet another conflict when deploying VMs via nixops, thus I think it's useful.

@teto
Copy link
Member Author

teto commented May 14, 2019

not sure who to ask review from. This saved me a few reboot with the wrong kernel already.
@dtzWill @Ericson2314 ?

@ofborg ofborg bot removed the 6.topic: kernel label May 17, 2019
@infinisil
Copy link
Member

This does break ofborg's evaluation :/

@teto
Copy link
Member Author

teto commented May 18, 2019

hopefully that solved evaluation

@ofborg ofborg bot requested a review from maggesi August 9, 2019 07:37
@teto
boot.kernelPackages: check for conflicts
2da1ad6 
It's currently possible to set conflicting `boot.kernelPackages` several times
which can prove confusing.
This is an attempt to warn for this.
@teto
Addressing review
4e30a28
@teto
Conditionally include nvidia modules
31c3a53
@teto teto changed the title [RFC] boot.kernelPackages: check for conflicts boot.kernelPackages: check for conflicts Aug 13, 2019
@teto
Copy link
Member Author

teto commented Aug 13, 2019

Looks like it's working here.

maggesi
maggesi approved these changes Aug 19, 2019
View reviewed changes
Copy link
Contributor

@maggesi maggesi left a comment

Choose a reason for hiding this comment

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

LGTM

A comment on BLCR: it is now very old and unsupported for modern linux kernels. I suggest to remove it from nixos and nixpkgs. I can do it in a subsequent PR.

@teto
Copy link
Member Author

teto commented Aug 20, 2019

yep BLCR seems like a good candidate for removal: # BLCR version 0.8.6 should works with linux kernel up to version 3.17.x.
Merging hopefully it doesn't break stuff :s

@teto teto merged commit 3b205ad into NixOS:master Aug 20, 2019
@teto teto deleted the kernelPackages_check branch August 20, 2019 05:15
@teto teto mentioned this pull request Aug 21, 2019
Closed
@teto teto mentioned this pull request Oct 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtzWill dtzWill dtzWill left review comments

@infinisil infinisil infinisil left review comments

@Izorkin Izorkin Izorkin left review comments

@maggesi maggesi maggesi approved these changes

@abbradar abbradar Awaiting requested review from abbradar

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

Successfully merging this pull request may close these issues.

None yet
7 participants
@teto @infinisil @dtzWill @maggesi @Izorkin @lheckemann @GrahamcOfBorg