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.

Sign up for GitHub

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

Jump to bottom

nixos-rebuild: use remote $PATH in buildHostCmd #107684

Merged
merged 1 commit into from Feb 11, 2021
Merged

nixos-rebuild: use remote $PATH in buildHostCmd #107684

merged 1 commit into from Feb 11, 2021

Conversation

fricklerhandwerk
Copy link
Contributor

Motivation for this change

I was trying to port nixos-rebuild --target-host <target> to Darwin and noticed that the $PATH to run commands on the build host was actually the local one. It should of course be the remote one.

This fixes the issue when using

nixos-rebuild --target-host <target> --use-remote-sudo switch

and the local machine does not have anything in $PATH that would resolve to sudo on the remote machine.

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 (no change)
  • Fits CONTRIBUTING.md.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-run-nixos-rebuild-target-host-from-darwin/9488/3

@cole-h
Copy link
Member

cole-h commented Jan 19, 2021 •
edited

@ofborg eval

@fricklerhandwerk
Copy link
Contributor Author

@zimbatm @lopsided98 @grahamc You've been working on that code in the past ~1 year, can anyone take a quick look?

@zimbatm
Copy link
Member

zimbatm commented Feb 10, 2021 •
edited

Good catch, but I don't think that the solution is working as expected. Did you test this?

$ BAR=3 env FOO='$BAR' bash -c 'echo $FOO'
$BAR

@cole-h
Copy link
Member

cole-h commented Feb 10, 2021

I think the point is that $PATH won't be expanded locally (e.g. on the system executing the ssh), but on the remote system, which is why $PATH is surrounded in single-quotes -- to prevent bash from expanding it to the local host's $PATH.

e.g. the PATH will look like /nix/store/......../bin:$PATH and not e.g. /nix/store/........./bin:/run/wrappers/bin:/home/vin/.nix-profile/bin:/etc/profiles/per-user/vin/bin:/nix/var/nix/profiles/default/bin:/run/current-system/sw/bin (from the host system).

TL;DR: It's expected the single quotes prevent expansion on the local system. Compare ssh <somehost> echo $PATH with ssh <somehost> echo '$PATH'.

@fricklerhandwerk
nixos-rebuild: use remote $PATH in buildHostCmd
50a2503 
this fixes the issue when using

    nixos-rebuild switch --target-host <target> --use-remote-sudo

when the local machine does not have anything in `$PATH` that would
resolve to `sudo` on the remote machine.

the single quotes prevent expansion of `$PATH` on the local machine,
such that the remote machine's value of that variable is used.
@fricklerhandwerk
Copy link
Contributor Author

@cole-h Thanks for the explanation, I have amended the commit message to make it clearer for the future reader.

@zimbatm
Copy link
Member

zimbatm commented Feb 10, 2021

I don't understand what does the path expansion, but it seems to work.

$ ssh build01.nix-community.org env FOO='$HOSTNAME' env | grep FOO
FOO=nix-community-build01

cole-h
cole-h approved these changes Feb 11, 2021
View reviewed changes
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Thanks!

@cole-h cole-h merged commit 283eaaf into NixOS:master Feb 11, 2021
@fricklerhandwerk fricklerhandwerk deleted the patch-1 branch April 14, 2021 21:02
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-run-nixos-rebuild-target-host-from-darwin/9488/4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zimbatm zimbatm zimbatm approved these changes

@cole-h cole-h cole-h approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@fricklerhandwerk @nixos-discourse @cole-h @zimbatm @SuperSandro2000