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.

Sign up for GitHub

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

grub: add grub.useOsprober option #22558

Merged
merged 2 commits into from
Feb 13, 2017
Merged

grub: add grub.useOsprober option #22558

merged 2 commits into from
Feb 13, 2017

Conversation

symphorien
Copy link
Member

Motivation for this change

Fix #7406

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

I only tested on one machine (BIOS, x86_64 and grub2).

@mention-bot
Copy link

@symphorien, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @wkennington and @obadz to be potential reviewers.

@globin globin requested a review from edolstra February 13, 2017 13:34
edolstra
edolstra reviewed Feb 13, 2017
View reviewed changes
nixos/modules/system/boot/loader/grub/grub.nix Outdated
@@ -265,6 +267,14 @@ in
'';
};

useOsprober = mkOption {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rename this to useOSProber?

@edolstra
Copy link
Member

Looks good to me.

@globin globin merged commit 0b87efa into NixOS:master Feb 13, 2017
@symphorien
Copy link
Member Author

@globin : you didn't rename the option in https://github.com/NixOS/nixpkgs/pull/22558/files#diff-3f0e9813c2baa6a4e7f8dbe5fd38ea56R435
Maybe get is case insensitive; I can't test on my machine it would entail a few hours of compilation and I can't reopen the PR anyway.

@globin
Copy link
Member

globin commented Feb 13, 2017

I fixed that in an extra commit :) on my phone right now but should be the commit right after the merge

@vcunat
Copy link
Member

vcunat commented Feb 13, 2017

Nix is case-sensitive.

@florianjacob
Copy link
Contributor

@symphorien or anybody else, I'm trying to test this, I added useOSProber = true; to my grub config, the output of nixos-rebuild seems like it probes for other OSes, but the resulting /boot/grub/grub.cfg is the same as without that option:

[florian@nausikaa:/]$ sudo nixos-rebuild switch
building Nix...
building the system configuration...
updating GRUB 2 menu...
lsblk: /dev/mapper/no*[0-9]: not a block-oriented device
lsblk: /dev/mapper/raid*[0-9]: not a block-oriented device
lsblk: /dev/mapper/disks*[0-9]: not a block-oriented device
activating the configuration...
setting up /etc...

I have an Arch Linux laying around on that hard disk as well. Is there a possibility to run the prober script alone to see what's its problem, or another way to debug this?

@symphorien
Copy link
Member Author

Just run os-prober as root. It should output one line by OS detected.

@florianjacob
Copy link
Contributor

@symphorien Thanks! Now I was able to see the problem is detecting the other OS, not the grub.cfg generation. 👍

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

@edolstra edolstra edolstra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@symphorien @mention-bot @edolstra @globin @vcunat @florianjacob