WIP fwupd: 1.0.5 -> 1.1.0 #39792

Moredread · 2018-05-01T04:41:44Z

We override the ESP mount point in the config file /etc/fwupd/uefi.conf
(available since version 1.0.6), as it is set to a path in the nix store
during build time.

Current issues: See issue #39386

Also:

Tests are disabled as it needs /etc/os-release, which is not available
when building with sandboxing enabled.

I also get a lot of bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) during the build, but am not sure if this is an issue with my system configuration.

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.

Mic92 · 2018-05-01T09:29:47Z

You are running into this problem regarding locales: #38991

Mic92 · 2018-05-01T09:30:23Z

cc @jtojnar

jtojnar · 2018-05-01T11:34:59Z

nixos/modules/services/hardware/fwupd.nix

@@ -71,6 +71,13 @@ in {
          BlacklistPlugins=${lib.concatStringsSep ";" cfg.blacklistPlugins}
        '';
      };
+      "fwupd/uefi.conf" = {
+        source = pkgs.writeText "daemon.conf" ''


Why do the names of the files mismatch?

jtojnar · 2018-05-01T11:47:55Z

From the changelog, the only thing that caught my eye were bash completions, but they use an exact path so it works for us. Upstream should probably switch to something like the following, as per https://www.bassi.io/articles/2018/03/15/pkg-config-and-paths/.

bash_completion_dep = dependency('bash_completion')
completionsdir = bash_completion_dep.get_pkgconfig_variable(
  'completionsdir',
  define_variable: [ 'prefix', prefix ],
)

@Mic92 I do not think that is #38991, the packages are built in sandbox so there should be no conflict with system archive. I have always seen the warning when setting LC_ALL.

@Moredread Regarding /etc/os-release, that should be reported upstream, it was already fixed once (by skipping the test IIRC).

Could you add a comment saying that /etc/fwupd/uefi.conf is created manually in the NixOS module, next to filesInstalledToEtc, so that we have the mismatch documented?

@GrahamcOfBorg test fwupd

GrahamcOfBorg · 2018-05-01T11:48:31Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: tests.fwupd

Partial log (click to expand)


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

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

GrahamcOfBorg · 2018-05-01T11:53:46Z

Success on x86_64-linux (full log)

Attempted: tests.fwupd

Partial log (click to expand)

machine: exit status 1
syncing
machine: running command: sync
machine: exit status 0
test script finished in 22.88s
cleaning up
killing machine (pid 593)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/zcwgkamrvyjw071qxafl11sz8jgpsbvf-vm-test-run-fwupd

Moredread · 2018-05-04T05:55:40Z

@jtojnar I'll try to get to it later today but I'm on vacation the next week or so, so I am not sure when I have the time.

Moredread · 2018-05-04T11:50:01Z

@jtojnar have added the comment and created an upstream issue for the failing test. But might it be a better idea to provide a os-release file in the sandbox, or should that be as minimal as possible? We have several packages that patch out os-release relevant code and tests after all.

peterhoeg · 2018-05-31T05:25:15Z

nixos/modules/services/hardware/fwupd.nix

+      "fwupd/uefi.conf" = {
+        source = pkgs.writeText "uefi.conf" ''
+          [uefi]
+          OverrideESPMountPoint=/boot


The mount point is configurable on nixos so instead of being statically set, it should be:

OverrideESPMountPoint=${config.boot.loader.efi.efiSysMountPoint}

Moredread · 2018-06-02T15:03:27Z

@jtojnar the issue for the missing os-release file has been closed upstream as nofix

Moredread · 2018-06-02T15:59:34Z

Besides the test issue the PR seems to be working and could be merged IMHO.

Edit: I forgot about the rights issue in issue #39386. In short I need to mount the /boot partition with fmask=0002,dmask=0002, i.e. the group needs write access. As this already likely an issue older fwupd version, I rather create a new PR than block this one, as this needs further investigating. I'm not even sure if the permission changes reliably fixes the issue.

jtojnar · 2018-06-02T16:36:14Z

Could you use the fix from #40872?

Moredread · 2018-06-02T17:11:09Z

In principle that would work (although I'd prefere to add the /tmp/os-release path to the list).

This would expose the os-release parser to a potential attacker with arbitrary rights on a misconfigured system without the file in the default location. I'm not sure how much that would be an issue.

jtojnar · 2018-06-11T02:20:39Z

Sounds reasonable.

By the way, 1.0.8 is out: NEWS

Moredread · 2018-06-18T17:41:15Z

@jtonar The patch we apply doesn't work there anymore. I'm not sure when I'll have time to look at the issue, but will leave a comment when I work on that.

jtojnar · 2018-06-18T18:15:29Z

Looks like the patch needs to update the context of po/make-images.sh @@ -7,6 +7,7 @@ from gzip -f ${x} to gzip -fn9 ${x}.

dtzWill · 2018-08-07T18:22:16Z

Looks like 1.1.0 is out, for what it's worth: https://github.com/hughsie/fwupd/blob/1.1.0/NEWS

Anyone know the status re:NixOS+fwupd? Is it expected to work if /boot is writable to the right groups/users?

jtojnar · 2018-08-07T18:56:15Z

I will try to update it. The 1.1.0 changes look interesting. Edit: Never mind. Apparently, it was just ESP location auto-detection, we are setting the path in module anyway.

jtojnar · 2018-08-07T20:01:44Z

Pushed. It would be great if anyone with relevant hardware could test this.

We override the ESP mount point in the config file /etc/fwupd/uefi.conf (available since version 1.0.6), as it is set to a path in the nix store during build time. Tests are disabled as it needs /etc/os-release, which is not available when building with sandboxing enabled.

jtojnar · 2018-08-07T21:27:55Z

Since this is a huge improvement we should probably merge it even if the UEFI update is still broken.

peterhoeg · 2018-08-08T00:19:27Z

Anyone know the status re:NixOS+fwupd? Is it expected to work if /boot is writable to the right groups/users?

I have used it to successfully update the firmware on a logitech receiver but not the machine firmware.

Moredread · 2018-08-08T10:59:50Z

@peterhoeg I had a few problems with permission, sometimes it worked, sometimes it didn't. (See #39386)

You also have to copy over the UEFI files for fwupd manually, and remove them after the update has finished.

I did a successful FW upgrade on an older version on my Dell laptop a few months ago so it should work in principal.

dtzWill · 2018-08-08T11:28:38Z

Thanks all! Gave this a go, generally looks good and UEFI situation same as @Moredread reports above-- best I could figure out was to just copy the things over (and remove after). Finally updated my laptop's BIOS xD.

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels May 1, 2018

jtojnar reviewed May 1, 2018

View reviewed changes

Moredread force-pushed the fwupd branch 2 times, most recently from 1551d4c to 5ed6380 Compare May 4, 2018 11:35

Moredread mentioned this pull request May 4, 2018

Test fail when /etc/os-release isn't available fwupd/fwupd#492

Closed

xeji added the 2.status: work-in-progress label May 9, 2018

jtojnar mentioned this pull request May 21, 2018

fwupd: 1.0.5 -> 1.0.7 #40872

Closed

8 tasks

peterhoeg reviewed May 31, 2018

View reviewed changes

jtojnar mentioned this pull request May 31, 2018

fwupd: 1.0.5 update & cleanups #35065

Merged

8 tasks

Moredread force-pushed the fwupd branch 2 times, most recently from d1efe1a to 9f394fb Compare June 2, 2018 15:56

jtojnar force-pushed the fwupd branch from 9f394fb to 6ea4ab0 Compare August 7, 2018 19:59

jtojnar changed the title ~~WIP fwupd: 1.0.5 -> 1.0.7~~ WIP fwupd: 1.0.5 -> 1.1.0 Aug 7, 2018

GrahamcOfBorg added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Aug 7, 2018

jtojnar force-pushed the fwupd branch from 0f77e42 to c1e6689 Compare August 7, 2018 20:11

Moredread and others added 2 commits August 7, 2018 23:25

gnome3.gnome-software: enable fwupd

82bf0a2

jtojnar force-pushed the fwupd branch from c1e6689 to 82bf0a2 Compare August 7, 2018 21:25

jtojnar merged commit 83a18e1 into NixOS:master Aug 8, 2018

Moredread deleted the fwupd branch November 18, 2018 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP fwupd: 1.0.5 -> 1.1.0 #39792

WIP fwupd: 1.0.5 -> 1.1.0 #39792

Moredread commented May 1, 2018 •

edited

Mic92 commented May 1, 2018

Mic92 commented May 1, 2018

jtojnar May 1, 2018

jtojnar commented May 1, 2018

GrahamcOfBorg commented May 1, 2018

GrahamcOfBorg commented May 1, 2018

Moredread commented May 4, 2018

Moredread commented May 4, 2018 •

edited

peterhoeg May 31, 2018

Moredread Jun 2, 2018

Moredread commented Jun 2, 2018

Moredread commented Jun 2, 2018 •

edited

jtojnar commented Jun 2, 2018

Moredread commented Jun 2, 2018 •

edited

jtojnar commented Jun 11, 2018

Moredread commented Jun 18, 2018

jtojnar commented Jun 18, 2018

dtzWill commented Aug 7, 2018

jtojnar commented Aug 7, 2018 •

edited

jtojnar commented Aug 7, 2018

jtojnar commented Aug 7, 2018

peterhoeg commented Aug 8, 2018

Moredread commented Aug 8, 2018

dtzWill commented Aug 8, 2018

WIP fwupd: 1.0.5 -> 1.1.0 #39792

WIP fwupd: 1.0.5 -> 1.1.0 #39792

Conversation

Moredread commented May 1, 2018 • edited

Things done

Mic92 commented May 1, 2018

Mic92 commented May 1, 2018

jtojnar May 1, 2018

Choose a reason for hiding this comment

jtojnar commented May 1, 2018

GrahamcOfBorg commented May 1, 2018

GrahamcOfBorg commented May 1, 2018

Moredread commented May 4, 2018

Moredread commented May 4, 2018 • edited

peterhoeg May 31, 2018

Choose a reason for hiding this comment

Moredread Jun 2, 2018

Choose a reason for hiding this comment

Moredread commented Jun 2, 2018

Moredread commented Jun 2, 2018 • edited

jtojnar commented Jun 2, 2018

Moredread commented Jun 2, 2018 • edited

jtojnar commented Jun 11, 2018

Moredread commented Jun 18, 2018

jtojnar commented Jun 18, 2018

dtzWill commented Aug 7, 2018

jtojnar commented Aug 7, 2018 • edited

jtojnar commented Aug 7, 2018

jtojnar commented Aug 7, 2018

peterhoeg commented Aug 8, 2018

Moredread commented Aug 8, 2018

dtzWill commented Aug 8, 2018

Moredread commented May 1, 2018 •

edited

Moredread commented May 4, 2018 •

edited

Moredread commented Jun 2, 2018 •

edited

Moredread commented Jun 2, 2018 •

edited

jtojnar commented Aug 7, 2018 •

edited