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

nixos-rebuild: Remove ambient systemctl dependency #108879

Merged
merged 1 commit into from Mar 7, 2021

Conversation

kaii-zen
Copy link
Contributor

@kaii-zen kaii-zen commented Jan 9, 2021

Motivation for this change

We wanted to use nixos-rebuild --target-host to deploy NixOS configurations from a flake to remote NixOS machines.
However, this fails from both Darwin and non-systemd distros due to the code below which relies on an ambient systemctl being available:

# If the Nix daemon is running, then use it.  This allows us to use
# the latest Nix from Nixpkgs (below) for expression evaluation, while
# still using the old Nix (via the daemon) for actual store access.
# This matters if the new Nix in Nixpkgs has a schema change.  It
# would upgrade the schema, which should only happen once we actually
# switch to the new configuration.
# If --repair is given, don't try to use the Nix daemon, because the
# flag can only be used directly.
if [ -z "$repair" ] && systemctl show nix-daemon.socket nix-daemon.service | grep -q ActiveState=active; then
    export NIX_REMOTE=${NIX_REMOTE-daemon}
fi

As far as I could gather, the NIX_REMOTE environment variable is pretty much unnecessary nowadays (it's not even set in any of my environments, NixOS or otherwise) as nix uses other heuristics to determine whether to use the daemon or not. Simply removing this part seems to have resolved our issue without breaking anything else and this seems like a valid use-case.

Things done

Remove aforementioned par

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

cc @manveru

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Please change the title and commit message to nixos-rebuild: Remove ambient systemctl dependency

@kaii-zen kaii-zen changed the title Remove ambient systemctl dependency from nixos-rebuild nixos-rebuild: Remove ambient systemctl dependency from nixos-rebuild Jan 9, 2021
@kaii-zen kaii-zen changed the title nixos-rebuild: Remove ambient systemctl dependency from nixos-rebuild nixos-rebuild: Remove ambient systemctl dependency Jan 9, 2021
The use of systemctl makes this incompatible with darwin even though
building/deploying a nixos closure from darwin is a perfectly valid use case.

NIX_DAEMON is pretty much unnecessary nowadays as nix uses other
indicators for deciding whether to use the daemon or not.
@Mic92 Mic92 requested a review from edolstra March 4, 2021 06:15
@Mic92
Copy link
Member

Mic92 commented Mar 4, 2021

Do you know when this switch to heuristic happened? Just to make sure it does not affect people that downgrade nix.

@kaii-zen
Copy link
Contributor Author

kaii-zen commented Mar 4, 2021

Do you know when this switch to heuristic happened? Just to make sure it does not affect people that downgrade nix.

I just trusted @matthewbauer: NixOS/nix@92f461e

@Mic92
Copy link
Member

Mic92 commented Mar 7, 2021

Ok. Three years sounds fair enough.

@Mic92
Copy link
Member

Mic92 commented Mar 7, 2021

I think it does not actually matter because nixos-rebuild builds its own nix anyway.

@Mic92 Mic92 merged commit e7c8a73 into NixOS:master Mar 7, 2021
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

5 participants