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

nixos/modules-closure.sh: don't fail if firmware is missing #109030

Merged
merged 1 commit into from Jan 12, 2021
Merged

nixos/modules-closure.sh: don't fail if firmware is missing #109030

merged 1 commit into from Jan 12, 2021

Conversation

xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Jan 11, 2021 •
edited

Motivation for this change

Since fdf3215, we no longer allow missing modules in the initrd. Unfortunately atm the modules-closure script also fails on missing firmware, which is a very common case (e.g. xhci-pci.ko.xz lists renesas_usb_fw.mem as dependent firmware). Fix this by only issuing a warning instead.

Theoretically this change could beak things for somebody if they explicitly have allowMissing set and rely on the current behavior (that it reports missing firmware as well). I have a hard time imagining such a setup though, so it should be fine I think.

Things done

Rebuilding my system from this branch currently. The initrd builds now (the system closure contains gpgme, which has other issues currently, so I did not boot from this yet).

  • 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.

@xaverdh
nixos/modules-closure.sh: don't fail if firmware is missing
49130f9 
Since fdf3215, we no longer allow
missing modules in the initrd. Unfortunately since before this commit,
the modules-closure script would also fail on missing firmware, which
is a very common case (e.g. xhci-pci.ko.xz lists renesas_usb_fw.mem as
dependent firmware). Fix this by only issuing a warning instead.
@xaverdh xaverdh mentioned this pull request Jan 11, 2021
Merged
10 tasks
@xaverdh
Copy link
Contributor Author

xaverdh commented Jan 11, 2021

By the way:
Do we have a way for builders to properly report warnings like this short of aborting the build (maybe logging to stderr or something)?
Would be cool if they would show up more prominently (colors or some sort of markup) in the build logs.

@eadwu
Copy link
Member

eadwu commented Jan 11, 2021 •
edited

Looks like better solution would be to set CONFIG_USB_XHCI_PCI_RENESAS to no or to include the firmware.

Nevermind this doesn't work.

@xaverdh
Copy link
Contributor Author

xaverdh commented Jan 12, 2021

Looks like better solution would be to set CONFIG_USB_XHCI_PCI_RENESAS to no or to include the firmware.

Nevermind this doesn't work.

Ok, maybe my description sounded like this is the only occurance, that is far from true I think. i915 for example lists two firmware blobs as dependencies on my system.
The Renesas firmware is also quite special. Its packaged in aur though, but with a custom license written in the proprietary rtf format.. I somewhat doubt everybody who uses usb wants that on their system.
It also says "You may not sublicense, rent, distribute, assign, transfer or otherwise dispose of Software and its license to any third party", so I am not quite sure if its legal to package this at all.

@K900
Copy link
Contributor

K900 commented Jan 12, 2021

Can confirm applying this makes my VM build again.

@xaverdh xaverdh mentioned this pull request Jan 12, 2021
Draft
10 tasks
@delroth
Copy link
Contributor

delroth commented Jan 12, 2021

Hit this issue today as well, thanks for the fix.

@Mic92 Mic92 merged commit ba09100 into NixOS:master Jan 12, 2021
@xaverdh xaverdh deleted the modules-closure-ignore-missing-firmware branch January 12, 2021 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@xaverdh @eadwu @K900 @delroth @Mic92