nixos-rebuild: fix issues when using --target-host

[Enzime]

Description of changes

This PR fixes deploying to hosts that require entering a password for sudo using --target-host.

This PR changes --use-remote-sudo to use sudo as little as possible.

This PR also fixes systemd-run hanging when used with --target-host.

Replaces #109046

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

cc @FRidh @zimbatm @oxalica @bjornfor

---

Add a 👍 reaction to pull requests you find important.

[Enzime]

cc @thiagokokada @duament @roberth

[roberth]

Unfortunately, if you don't add a test, it will keep breaking. Please add a test.

[duament]

I tried this pr. But it didn't fix #262686.

My first switch (https://github.com/duament/flakes/actions/runs/7363818666/job/20043632674) succeeded, although there's a warning Pseudo-terminal will not be allocated because stdin is not a terminal. and the output of switch-to-configuration was not printed.

My second switch (https://github.com/duament/flakes/actions/runs/7363898397/job/20043786127) failed, but the exit code was 0 and github treated it as a success. Here's the error message: Failed to start transient service unit: Unit nixos-rebuild-switch-to-configuration.service was already loaded or has a fragment file.

[Enzime]

@duament could you test this PR again? I've changed the flag from -t to -tt which should hopefully fix it

[Enzime]

-tt breaks the tests so I'm going to switch back to using -t, my last commit only fixes a systemd-run hang regression that was introduced by adding -t, I don't think it fixes #262686

[roberth]

If we really need a new behavior (do we?), maybe fix the inconsistency of "remote" vs "target"?
Not sure I like "target" because of cross compilation terminology, but target does become less relevant to users as compilers and their Nix expressions improve. "Remote" isn't great either, because both machines may be equally remote, as demonstrated by the test.
Also if you have an office with NixOS machines and you make the on site server responsible for deploying updates, arguably the deployer is more remote.

So I think I prefer target?

Suggested change

	.Op Fl -use-remote-sudo-always
	.Op Fl -target-use-sudo

Using target as a prefix reinforces nice grouping, and helps with discovery through shell completions.

[Enzime]

I've removed the --use-remote-sudo-always flag, opting to just make this PR an improvement on the existing --use-remote-sudo flag

[hannesg]

Hi @Enzime, Hi @roberth

I just tested the change with my nixos server and I observed a regression. When using --build-host and flakes, nixos-rebuild fails with a weird error. I added +x logging to bash and found that pathToConfig ( here ) is set to the tty output of ssh which is in my case:

'warning: you did not specify '\''--add-root'\''; the result might be removed by the garbage collector
'nix/store/ph132c4ihigcfyi86pwlmn4q12qmwyz7-nixos-system-granogayo-24.05.20240113.e0d20bf

All subsequent commands then fail with weird errors. Reverting 09fd207 fixes the issue.

[roberth]

Found an even worse problem as well, which was already caught by the tests, but not reported well by ofborg. I did check the list, but missed the single grey status this time.

Fixes available in #280842; will merge as soon as tests pass locally.

If any other issues come up, consider reverting both PRs unless trivially fixable.

TODO:

• Add regression test for --build-host. I suggest extending the target host test for this; seems more efficient that way. Maybe rename it to -remote-hosts? Keep the existing test case in the testScript as is.

[roberth]

Made a start here. Would appreciate some help.

• [WIP] Test nixos-rebuild --build-host #281130