Make SSHMaster::startCommand work on an args list

This avoids split-on-whitespace errors:

- No more `bash -c` needed

- No more `shellEscape` needed

- `remote-program` ssh store setting also cleanly supports args (e.g.
  `nix daemon`)

- `ssh` uses `--` to separate args for SSH from args for the command to
  run.

and will help with Hydra dedup.

Some code taken from #6628.

Co-Authored-By: Alexander Bantyev <balsoft@balsoft.ru>
This commit is contained in:
John Ericson 2024-01-22 15:50:00 -05:00
parent 74534829f2
commit b71673109c
5 changed files with 36 additions and 19 deletions

View File

@ -55,9 +55,14 @@ LegacySSHStore::LegacySSHStore(const std::string & scheme, const std::string & h
ref<LegacySSHStore::Connection> LegacySSHStore::openConnection() ref<LegacySSHStore::Connection> LegacySSHStore::openConnection()
{ {
auto conn = make_ref<Connection>(); auto conn = make_ref<Connection>();
conn->sshConn = master.startCommand( Strings command = remoteProgram.get();
fmt("%s --serve --write", remoteProgram) command.push_back("--serve");
+ (remoteStore.get() == "" ? "" : " --store " + shellEscape(remoteStore.get()))); command.push_back("--write");
if (remoteStore.get() != "") {
command.push_back("--store");
command.push_back(remoteStore.get());
}
conn->sshConn = master.startCommand(std::move(command));
conn->to = FdSink(conn->sshConn->in.get()); conn->to = FdSink(conn->sshConn->in.get());
conn->from = FdSource(conn->sshConn->out.get()); conn->from = FdSource(conn->sshConn->out.get());

View File

@ -13,7 +13,7 @@ struct LegacySSHStoreConfig : virtual CommonSSHStoreConfig
{ {
using CommonSSHStoreConfig::CommonSSHStoreConfig; using CommonSSHStoreConfig::CommonSSHStoreConfig;
const Setting<Path> remoteProgram{this, "nix-store", "remote-program", const Setting<Strings> remoteProgram{this, {"nix-store"}, "remote-program",
"Path to the `nix-store` executable on the remote machine."}; "Path to the `nix-store` executable on the remote machine."};
const Setting<int> maxConnections{this, 1, "max-connections", const Setting<int> maxConnections{this, 1, "max-connections",

View File

@ -17,7 +17,7 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
using RemoteStoreConfig::RemoteStoreConfig; using RemoteStoreConfig::RemoteStoreConfig;
using CommonSSHStoreConfig::CommonSSHStoreConfig; using CommonSSHStoreConfig::CommonSSHStoreConfig;
const Setting<Path> remoteProgram{this, "nix-daemon", "remote-program", const Setting<Strings> remoteProgram{this, {"nix-daemon"}, "remote-program",
"Path to the `nix-daemon` executable on the remote machine."}; "Path to the `nix-daemon` executable on the remote machine."};
const std::string name() override { return "Experimental SSH Store"; } const std::string name() override { return "Experimental SSH Store"; }
@ -212,14 +212,15 @@ public:
ref<RemoteStore::Connection> SSHStore::openConnection() ref<RemoteStore::Connection> SSHStore::openConnection()
{ {
auto conn = make_ref<Connection>(); auto conn = make_ref<Connection>();
Strings command = remoteProgram.get();
std::string command = remoteProgram + " --stdio"; command.push_back("--stdio");
if (remoteStore.get() != "") if (remoteStore.get() != "") {
command += " --store " + shellEscape(remoteStore.get()); command.push_back("--store");
for (auto & arg : extraRemoteProgramArgs) command.push_back(remoteStore.get());
command += " " + shellEscape(arg); }
command.insert(command.end(),
conn->sshConn = master.startCommand(command); extraRemoteProgramArgs.begin(), extraRemoteProgramArgs.end());
conn->sshConn = master.startCommand(std::move(command));
conn->to = FdSink(conn->sshConn->in.get()); conn->to = FdSink(conn->sshConn->in.get());
conn->from = FdSource(conn->sshConn->out.get()); conn->from = FdSource(conn->sshConn->out.get());
return conn; return conn;

View File

@ -52,7 +52,8 @@ bool SSHMaster::isMasterRunning() {
return res.first == 0; return res.first == 0;
} }
std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string & command) std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(
Strings && command, Strings && extraSshArgs)
{ {
Path socketPath = startMaster(); Path socketPath = startMaster();
@ -84,18 +85,19 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string
Strings args; Strings args;
if (fakeSSH) { if (!fakeSSH) {
args = { "bash", "-c" };
} else {
args = { "ssh", host.c_str(), "-x" }; args = { "ssh", host.c_str(), "-x" };
addCommonSSHOpts(args); addCommonSSHOpts(args);
if (socketPath != "") if (socketPath != "")
args.insert(args.end(), {"-S", socketPath}); args.insert(args.end(), {"-S", socketPath});
if (verbosity >= lvlChatty) if (verbosity >= lvlChatty)
args.push_back("-v"); args.push_back("-v");
args.splice(args.end(), std::move(extraSshArgs));
args.push_back("--");
} }
args.push_back(command); args.splice(args.end(), std::move(command));
execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); execvp(args.begin()->c_str(), stringsToCharPtrs(args).data());
// could not exec ssh/bash // could not exec ssh/bash

View File

@ -41,7 +41,16 @@ public:
AutoCloseFD out, in; AutoCloseFD out, in;
}; };
std::unique_ptr<Connection> startCommand(const std::string & command); /**
* @param command The command (arg vector) to execute.
*
* @param extraSShArgs Extra args to pass to SSH (not the command to
* execute). Will not be used when "fake SSHing" to the local
* machine.
*/
std::unique_ptr<Connection> startCommand(
Strings && command,
Strings && extraSshArgs = {});
Path startMaster(); Path startMaster();
}; };