libutil: don't allow interactive runProgram2
realistically only runProgram is ever called for interactive reasons, and even those calls seem to be rather ill-advised in many cases. the chance of multiple interactive processes interfering with each other, whether for input or for output, must be very low to make this in any way reasonable: if e.g. git calls ssh for multiple fetched inputs and ssh requests passphrases for both we can otherwise not guarantee that *any* input is routed correctly. misrouted output is merely annoying. Change-Id: I794e3fdf0a3238cb9292003a89ac267f0de7a939
This commit is contained in:
@@ -21,6 +21,7 @@
|
||||
|
||||
#include "lix/libfetchers/fetch-settings.hh"
|
||||
|
||||
#include <optional>
|
||||
#include <regex>
|
||||
#include <string.h>
|
||||
#include <sys/time.h>
|
||||
@@ -70,15 +71,13 @@ Path getCachePath(std::string_view key)
|
||||
// ...
|
||||
static kj::Promise<Result<std::optional<std::string>>> readHead(const Path & path)
|
||||
try {
|
||||
auto [status, output] = TRY_AWAIT(runProgram(RunOptions{
|
||||
.program = "git",
|
||||
auto output = TRY_AWAIT(runProgram(
|
||||
"git",
|
||||
true,
|
||||
// FIXME: use 'HEAD' to avoid returning all refs
|
||||
.args = {"ls-remote", "--symref", path},
|
||||
.isInteractive = true,
|
||||
}));
|
||||
if (status != 0) {
|
||||
co_return std::nullopt;
|
||||
}
|
||||
{"ls-remote", "--symref", path},
|
||||
true
|
||||
));
|
||||
|
||||
std::string_view line = output;
|
||||
line = line.substr(0, line.find("\n"));
|
||||
@@ -94,6 +93,8 @@ try {
|
||||
co_return parseResult->target;
|
||||
}
|
||||
co_return std::nullopt;
|
||||
} catch (ExecError &) {
|
||||
co_return std::nullopt;
|
||||
} catch (...) {
|
||||
co_return result::current_exception();
|
||||
}
|
||||
|
||||
+23
-11
@@ -8,6 +8,8 @@
|
||||
#include "lix/libutil/strings.hh"
|
||||
#include "lix/libutil/serialise.hh"
|
||||
#include "lix/libutil/signals.hh"
|
||||
#include "manually-drop.hh"
|
||||
#include "sync.hh"
|
||||
|
||||
#include <cerrno>
|
||||
#include <cstdlib>
|
||||
@@ -19,6 +21,7 @@
|
||||
#include <sys/types.h>
|
||||
#include <sys/wait.h>
|
||||
#include <unistd.h>
|
||||
#include <utility>
|
||||
|
||||
#ifdef __APPLE__
|
||||
# include <sys/syscall.h>
|
||||
@@ -233,8 +236,27 @@ Pid startProcess(std::function<void()> fun, const ProcessOptions & options)
|
||||
kj::Promise<Result<std::string>>
|
||||
runProgram(Path program, bool searchPath, const Strings args, bool isInteractive)
|
||||
try {
|
||||
// allow only one interactive program per unit time so they don't mess with each other.
|
||||
//
|
||||
// see https://git.lix.systems/lix-project/lix/issues/702 for why this is ManuallyDrop.
|
||||
static ManuallyDrop<Sync<int, AsyncMutex>> interactiveMutex{std::in_place, 0};
|
||||
|
||||
std::optional<Sync<int, AsyncMutex>::Lock> interactiveLock;
|
||||
KJ_DEFER({
|
||||
if (interactiveLock) {
|
||||
logger->resume();
|
||||
}
|
||||
});
|
||||
|
||||
if (isInteractive) {
|
||||
interactiveLock = co_await interactiveMutex->lock();
|
||||
logger->pause();
|
||||
}
|
||||
|
||||
auto res = TRY_AWAIT(runProgram(RunOptions{
|
||||
.program = program, .searchPath = searchPath, .args = args, .isInteractive = isInteractive
|
||||
.program = program,
|
||||
.searchPath = searchPath,
|
||||
.args = args,
|
||||
}));
|
||||
|
||||
if (!statusOk(res.first)) {
|
||||
@@ -324,16 +346,6 @@ RunningProgram runProgram2(const RunOptions & options)
|
||||
.dieWithParent = options.dieWithParent,
|
||||
};
|
||||
|
||||
std::optional<Finally<std::function<void()>>> resumeLoggerDefer;
|
||||
if (options.isInteractive) {
|
||||
logger->pause();
|
||||
resumeLoggerDefer.emplace(
|
||||
[]() {
|
||||
logger->resume();
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
printMsg(lvlChatty, "running command: %s", concatMapStringsSep(" ", options.args, shellEscape));
|
||||
|
||||
/* Fork. */
|
||||
|
||||
@@ -96,7 +96,6 @@ struct RunOptions
|
||||
std::optional<std::map<std::string, std::string>> environment = {};
|
||||
bool dieWithParent = true;
|
||||
bool captureStdout = false;
|
||||
bool isInteractive = false;
|
||||
std::vector<Redirection> redirections;
|
||||
#if __linux__
|
||||
std::set<long> caps;
|
||||
|
||||
Reference in New Issue
Block a user