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

tree-wide: use systemctl of running system #88492

Merged
merged 17 commits into from May 21, 2020

Conversation

@flokli
Copy link
Contributor

flokli commented May 21, 2020

In many different places in nixpkgs, when shelling out to systemctl, we referred to it as ${pkgs.systemd}/bin/systemctl or ${config.systemd.package}/bin/systemctl.

While this provides a systemctl binary, it's not necessarily one that's fully compatible with the one that's currently running. As the systemd of the running configuration is available at /run/current-system/systemd, we can refer to systemctl via /run/current-system/systemd/bin/systemctl.

This should also prevent some service restarts in the case of systemd.package being updated.

Things done
  • 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.
flokli added 17 commits May 21, 2020
Also, make the postRun script refer to that systemctl, and not just rely
on $PATH for consistency.
@flokli flokli requested review from adisbladis, lheckemann, jtojnar and danbst May 21, 2020
flokli referenced this pull request May 21, 2020
…per script context
@ofborg ofborg bot requested review from zimbatm, worldofpeace, globin and romildo May 21, 2020
Copy link
Member

adisbladis left a comment

I haven't tested this, but diff LGTM.

@michaelpj
Copy link
Contributor

michaelpj commented May 21, 2020

This makes a bunch of things depend on the filesystem layout of NixOS. If we want to say "use the ambient systemctl", would it not be reasonable to just take it from PATH and have it be a true runtime dependency?

@flokli
Copy link
Contributor Author

flokli commented May 21, 2020

This makes a bunch of things depend on the filesystem layout of NixOS. If we want to say "use the ambient systemctl", would it not be reasonable to just take it from PATH and have it be a true runtime dependency?

In some of these cases, systemctl isn't available in $PATH. Most of the changes here are inside NixOS module code, writing unit files, so assuming NixOS seems reasonable.

In terms of patching applications people might install with Nix on non-NixOS - I didn't look through all of these. The only 3 changes in this PR don't seem critical:

  • displaylink and autorandr only refer to systemctl in some udev contexts, which aren't managed when using on non-NixOS
  • deepin.dde-control-center refers to systemctl in some dbus .policy file, which also likely isn't used when using outside of NixOS
@flokli flokli merged commit 927b779 into NixOS:master May 21, 2020
18 checks passed
18 checks passed
editorconfig editorconfig
Details
Evaluation Performance Report Evaluator Performance Report
Details
autorandr, autorandr.passthru.tests, deepin.dde-control-center, deepin.dde-control-center.passthru.tests, displaylink, displaylink.passthru.tests, google-compute-engine, google-compute-engine.passthru.tests on aarch64-linux Success
Details
autorandr, autorandr.passthru.tests, deepin.dde-control-center, deepin.dde-control-center.passthru.tests, displaylink, displaylink.passthru.tests, google-compute-engine, google-compute-engine.passthru.tests on x86_64-darwin Success
Details
autorandr, autorandr.passthru.tests, deepin.dde-control-center, deepin.dde-control-center.passthru.tests, displaylink, displaylink.passthru.tests, google-compute-engine, google-compute-engine.passthru.tests 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="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d69f95e"; rev="d69f95e2fe4f7fe157539ad8312c73390d456688"; } ./pkgs/t
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
@flokli flokli deleted the flokli:current-system-systemctl branch May 21, 2020
@vcunat
Copy link
Member

vcunat commented May 22, 2020

Unfortunately this blocks both channels now. There are some checks:

Checking that all programs called by absolute paths in udev rules exist... FAIL
/run/current-system/systemd/bin/systemctl is called in udev rules but is not executable or does not exist

I used

nix build -f nixos/release-small.nix nixos.tests.boot.biosCdrom.x86_64-linux

to bisect to 1955982.

flokli added a commit to flokli/nixpkgs that referenced this pull request May 22, 2020
NixOS#88492 flipped some references to
systemctl from config.systemd.package to /run/current-system/systemd/,
which udevRules obviously isn't able resolve.

If we encounter such references, replace them with
config.systemd.package before doing the check.
@flokli
Copy link
Contributor Author

flokli commented May 22, 2020

Yeah, the check in nixos/modules/services/hardware/udev.nix doesn't know about /run/current-system. I assume other checks might fail too, if more of these patched udev rules land in udev.packages.

I taught udevRules how to resolve these references in #88607, PTAL.

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

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