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

fwupd: 0.9.6 → 1.0.1 #30252

Merged
merged 3 commits into from Nov 24, 2017
Merged

fwupd: 0.9.6 → 1.0.1 #30252

merged 3 commits into from Nov 24, 2017

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Oct 9, 2017

Motivation for this change

I cherry-picked from GNOME 3.26, where it builds sandboxed. Then I updated again (to 1.0.0), added a nixos service and hopefully fixed the problems from #24376. The new version contains some backwards incompatible changes but we do not depend on it anywhere.

There are several issues:

  • mesonFlags variable does not interpolate variables so I had to modify it in preConfigureHook.
  • The sysconfdir and localstatedir are inside the nix store, i.e. not writeable, making any operation impossible. I also had to remove ReadWritePaths from systemd service file to prevent “Failed at step NAMESPACE spawning” error. Solved by NixOS module.
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
    • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

cc @shlevy

@jtojnar jtojnar changed the title fwupd: 0.9.6 → 1.0.0 [wip] fwupd: 0.9.6 → 1.0.0 Oct 9, 2017
@jtojnar jtojnar force-pushed the fwupd branch 2 times, most recently from 7f7d25e to 4783dd7 Compare October 9, 2017 15:12
@jtojnar jtojnar mentioned this pull request Oct 11, 2017
8 tasks
mesonFlags = [
"-Denable-colorhug=false"
"-Denable-man=false"
"-Denable-tests=false"
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason to disable tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just porting the configureFlags from before. I will try to make the tests run later but first we need to make /var and /etc work.

@jtojnar jtojnar force-pushed the fwupd branch 2 times, most recently from 9c00bfa to 2fe87c6 Compare November 7, 2017 14:00
@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 7, 2017

The service now builds and runs but there are some errors in the journal:

Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: failed to load remote fwupd: Failed to parse /nix/store/8q2ip5d58h1nwvq5dda3qrppdlhg44hg-fwupd-1.0.0/share/fwupd/remotes.d/fwupd/metadata.xml file: cannot process file of type text/plain
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: disabling plugin because: failed to startup dell: Firmware updating not supported
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: disabling plugin because: failed to startup amt: Unable to find a ME interface
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: disabling plugin because: failed to coldplug synapticsmst: MST firmware updating not supported by OEM
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: disabling plugin because: failed to coldplug raspberrypi: Raspberry PI firmware updating not supported, no /boot/start.elf
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: disabling plugin because: failed to coldplug thunderbolt_power: missing kernel support for intel-wmi-thunderbolt
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: disabling plugin because: failed to coldplug uefi: UEFI firmware updating not supported
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: using plugins: steelseries, ebitdo, altos, unifying, udev, upower, thunderbolt, dfu
Nov 07 13:57:59 nixos .fwupd-wrapped[1064]: Daemon ready for requests

@jtojnar jtojnar changed the title [wip] fwupd: 0.9.6 → 1.0.0 fwupd: 0.9.6 → 1.0.0 Nov 7, 2017
@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 7, 2017

Not sure what else I can do about it.

@thblt
Copy link
Contributor

thblt commented Nov 7, 2017

Not sure, but couldn't these messages just mean that some hardware or vendor-specific plugins can't be loaded because they don't correspond to the actual hardware? The only oddity is the first line, which may just mean that fwupdmgr is trying to load a database from a wrong location.

On a related topic, I'd like to test this PR, but I don't know how. Do I need to fork nixpkgs, merge jtojnar's branch and somehow instruct NixOS to get nixpkgs from my fork? I'd appreciate pointers to relevant documentations, I don't know what to search for. Thanks!

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 7, 2017

Yes, just fetch the branch and rebuild from the directory.

cd nixpkgs
git remote add jtojnar git@github.com:jtojnar/nixpkgs.git
git fetch jtojnar
git checkout jtojnar/fwupd
sudo nixos-rebuild switch -I nixpkgs=.

@thblt
Copy link
Contributor

thblt commented Nov 7, 2017

Thanks @jtojnar ! I've just tried your branch at 3fb5702, fwupdmgr refresh works well, but fwupdmgr update didn't seem to do anything (Dell XPS 9560 4K+touch). Please tell me if I can help testing.

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 7, 2017

@thblt Are there any updates when you list fwupdmgr get-updates? Maybe there is not anything to do? Not sure – I am only updating this because it is a GNOME blocker.

@thblt
Copy link
Contributor

thblt commented Nov 7, 2017

I haven't tested this. There was at least one possible updates, firmware to 1.5, which I ended up doing directly from the BIOS (sorry, it was a bit urgent, I would have waited otherwise). It appears in /var/lib/fwupd/remotes.d/metadata.xml.gz:

<component type="firmware">
    <id>com.dell.uefi34578c72.firmware</id>
    <name>XPS 15 9560/Precision 5520 System Update</name>
    <summary>Firmware for the Dell XPS 15 9560/Precision 5520</summary>
    <developer_name>Dell Inc.</developer_name>
    <project_license>proprietary</project_license>
    <description><p>Updating the system firmware improves performance.</p></description>
    <url type="homepage">http://support.dell.com/</url>
    <releases>
      <release version="66816" timestamp="1504150320">
        <size type="installed">10334313</size>
        <size type="download">10051765</size>
        <location>https://fwupd.org/downloads/e69073e5403ee433ddfdd40b53781e91074b07b2-firmware_XPS_9560_1_5_0.wu.cab</location>
        <checksum filename="e69073e5403ee433ddfdd40b53781e91074b07b2-firmware_XPS_9560_1_5_0.wu.cab" target="container" type="sha1">dd472551e4c6f80c5cfd0ffcb3f477ef120fe358</checksum>
        <checksum filename="firmware.bin" target="content" type="sha1">e05410c1cd3c96e0f5cafa5e29c59ab1f88f601b</checksum>
      </release>

@c0bw3b c0bw3b mentioned this pull request Nov 11, 2017
8 tasks
@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 20, 2017

Is @thblt test conclusive enough?

@jtojnar jtojnar changed the title fwupd: 0.9.6 → 1.0.0 fwupd: 0.9.6 → 1.0.1 Nov 20, 2017
@ixmatus
Copy link
Contributor

ixmatus commented Nov 22, 2017

I needed to use @jtojnar's work here and can confirm it has worked well for me.

@jtojnar
Copy link
Contributor Author

jtojnar commented Nov 23, 2017

@GrahamcOfBorg build fwupd

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Failure for system: x86_64-darwin

error: Package ‘fwupd-1.0.1’ in /tmp/nix-ofborg/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/30252/pkgs/os-specific/linux/firmware/fwupd/default.nix:10 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-linux

shrinking /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib/fwupd-plugins-3/libfu_plugin_steelseries.so
shrinking /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib/fwupd-plugins-3/libfu_plugin_amt.so
shrinking /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib/fwupd-plugins-3/libfu_plugin_colorhug.so
shrinking /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib/fwupd-plugins-3/libfu_plugin_thunderbolt_power.so
shrinking /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib/fwupd-plugins-3/libfu_plugin_unifying.so
shrinking /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib/libfwupd.so.2.0.0
stripping (with flags -S) in /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/lib  /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/libexec  /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1/bin 
patching script interpreter paths in /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1
checking for references to /tmp/nix-build-fwupd-1.0.1.drv-0 in /nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1...
/nix/store/2v57k7bvhy3i11w3vb9q1j2275mw0kf6-fwupd-1.0.1

@jtojnar jtojnar merged commit 193c4c5 into NixOS:master Nov 24, 2017
@jtojnar jtojnar deleted the fwupd branch November 24, 2017 15:54
originalEtc =
let
isRegular = v: v == "regular";
listFiles = d: builtins.attrNames (filterAttrs (const isRegular) (builtins.readDir d));
Copy link
Member

Choose a reason for hiding this comment

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

Please, not this! It causes building during evaluation of your system, which breaks various assumptions, like Hydra rebuilding stdenv when it's only supposed to get a list of jobs. Especially as this service is enabled by default.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, we apparently had some other places like this, but they most likely haven't been tracked down yet. #29774

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, this should be fixed or reverted. And why is the service enabled by default?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have disabled it by default in 2f1a818

Copy link
Contributor

@orivej orivej Nov 25, 2017

Choose a reason for hiding this comment

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

fwupd increases the size of the smallContainer from 400 to 800 MiB: https://hydra.nixos.org/job/nixos/trunk-combined/nixos.closures.smallContainer.x86_64-linux#tabs-charts

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, not suitable to be on by default in this state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about enabling it by default, that was not intended.

Is there any better way to install the files to etc other than hardcoding the list of files?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know of any. You often don't even need to put anything into /etc/ and you just pass paths to some /nix/store/... as config. But I'm not really good at nixos module best practices.

Copy link
Contributor

@orivej orivej Nov 26, 2017

Choose a reason for hiding this comment

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

Is there any better way to install the files to etc other than hardcoding the list of files?

I see three options:

I am not sure which is better: the first is fine if fwupd is the only package that needs it (you may list the files in a passthru attribute of the fwupd definition to keep them closer to the source), the second is hard to justify, and the third is more work and may be hard to get right, but is the right approach to support more dynamic /etc.

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

Successfully merging this pull request may close these issues.

None yet

8 participants