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: split daemon again #79371

Merged
merged 2 commits into from Feb 6, 2020
Merged

fwupd: split daemon again #79371

merged 2 commits into from Feb 6, 2020

Conversation

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 6, 2020

In 1.3.5, fwupdprivate library was made into a shared fwupdplugin library. This library is considered semi-private and is used by fwupd daemon and fwupd plug-ins and now possibly third party plug-ins.

The fwupdplugin library refers to the plug-in directory in fwupd.out causing a dependency cycle. For that reason we need to move it to out.

  • tests.fwupd succeeded.
  • Rebuilt system with this.
    • Daemon starts
      • no weird journal entries
    • fwupdmgr get-devices prints devices
@jtojnar jtojnar requested review from dtzWill and worldofpeace Feb 6, 2020
@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Feb 6, 2020

Looks like we need to disable the invalid plug-in.

@jtojnar jtojnar force-pushed the jtojnar:hughsie-pkgs branch from 6a2c360 to b20f8b7 Feb 6, 2020
@jtojnar jtojnar added this to the 20.03 milestone Feb 6, 2020
@jtojnar
Copy link
Contributor Author

@jtojnar jtojnar commented Feb 6, 2020

Marked as a blocker because I forgot to disable the invalid plug-in in the previous PR.

# We do not want to place the daemon into lib
"--libexecdir=${placeholder "out"}/libexec"
# Our builder only adds $lib/lib into rpath
"-Dc_link_args=-Wl,-rpath,${placeholder "out"}/lib"

This comment has been minimized.

@worldofpeace

worldofpeace Feb 6, 2020
Member

Everything else I think I understand here, but would you mind explaining this for me?

This comment has been minimized.

@jtojnar

jtojnar Feb 6, 2020
Author Contributor

Since Nix does not have /usr/lib, every library a program depends on must have path for finding it listed in DT_RUNPATH entry in the ELF file. Our generic builder populates the entry for us but apparently only with lib outputs. Because the libfwupdplugin is in out its location is not included so we need to add it manually.

I believe the builder adds every /lib directory to entry of every[citation needed] ELF file but superfluous paths (those not containing libraries listed in DT_NEEDED entries) are stripped by[citation needed] strip setup-hook so there is no cycle.

This comment has been minimized.

@jtojnar

jtojnar Feb 6, 2020
Author Contributor

I improved the comments slightly.

This comment has been minimized.

@worldofpeace

worldofpeace Feb 6, 2020
Member

Thanks a lot 👍 All clear here.

@jtojnar jtojnar force-pushed the jtojnar:hughsie-pkgs branch from b20f8b7 to 6890c56 Feb 6, 2020
jtojnar added 2 commits Feb 6, 2020
In 1.3.5, fwupdprivate library was made into a shared fwupdplugin library.
This library is considered semi-private and is used by fwupd daemon and
fwupd plug-ins and now possibly third party plug-ins.

The fwupdplugin library refers to the plug-in directory in fwupd.out
causing a dependency cycle. For that reason we need to move it to out.
invalid test was introduced in fwupd/fwupd@297d159
and it is disabled in the shipped daemon.conf.

I forgot to reflect that in the module, which caused the daemon to print the following on start-up:

    FuEngine             invalid has incorrect built version invalid

and the command to warn:

    WARNING: The daemon has loaded 3rd party code and is no longer supported by the upstream developers!

To reduce the change of this happening in the future, I moved the list of default disabled plug-ins to the package expression.

I also set the value of the NixOS module option in the config section of the module instead of the default value used previously,
which will allow users to not care about these plug-ins.
@jtojnar jtojnar force-pushed the jtojnar:hughsie-pkgs branch from 6890c56 to e5f7dac Feb 6, 2020
@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Feb 6, 2020

@GrahamcOfBorg test installed-tests.fwupd

@jtojnar jtojnar merged commit 07281f2 into NixOS:master Feb 6, 2020
19 of 20 checks passed
19 of 20 checks passed
fwupd on x86_64-darwin
Details
fwupd on aarch64-linux Failure
Details
tests.fwupd on aarch64-linux No attempt
Details
tests.fwupd on x86_64-linux No attempt
Details
tests.installed-tests.fwupd on aarch64-linux Failure
Details
Evaluation Performance Report Evaluator Performance Report
Details
fwupd on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
tests.installed-tests.fwupd on x86_64-linux Success
Details
@jtojnar jtojnar deleted the jtojnar:hughsie-pkgs branch Feb 6, 2020
dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request Feb 7, 2020
fwupd: split daemon again
(cherry picked from commit 07281f2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.