From d477b34d1df3b471f8132525b0a008bbd03ddb6d Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 23 Jun 2024 15:19:47 +0200 Subject: [PATCH] libutil: remove runProgram2 stdin functionality this was only used in one place, and that place has been rewritten to use a temporary file instead. keeping this around is not very helpful at this time, and in any case we'd be better off rewriting subprocess handling in rust where we not only have a much safer library for such things but also async frameworks necessary for this easily available. Change-Id: I6f8641b756857c84ae2602cdf41f74ee7a1fda02 --- src/libcmd/repl.cc | 4 +-- src/libfetchers/git.cc | 8 +++--- src/libfetchers/mercurial.cc | 3 +- src/libutil/processes.cc | 55 ++---------------------------------- src/libutil/processes.hh | 5 +--- 5 files changed, 10 insertions(+), 65 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 99abbe44b..32cbf8e33 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -242,8 +242,7 @@ NixRepl::NixRepl(const SearchPath & searchPath, nix::ref store, ref & input = {}) +void runNix(Path program, const Strings & args) { auto subprocessEnv = getEnv(); subprocessEnv["NIX_CONFIG"] = globalConfig.toKeyValue(); @@ -252,7 +251,6 @@ void runNix(Path program, const Strings & args, .program = settings.nixBinDir+ "/" + program, .args = args, .environment = subprocessEnv, - .input = input, }); return; diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index f2d577914..a06bc86b2 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -358,7 +358,7 @@ struct GitInputScheme : InputScheme args.push_back(destDir); - runProgram("git", true, args, {}, true); + runProgram("git", true, args, true); } std::optional getSourcePath(const Input & input) const override @@ -589,7 +589,7 @@ struct GitInputScheme : InputScheme : ref == "HEAD" ? *ref : "refs/heads/" + *ref; - runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }, {}, true); + runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }, true); } catch (Error & e) { if (!pathExists(localRefFile)) throw; warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl); @@ -656,7 +656,7 @@ struct GitInputScheme : InputScheme // everything to ensure we get the rev. Activity act(*logger, lvlTalkative, actUnknown, fmt("making temporary clone of '%s'", repoDir)); runProgram("git", true, { "-C", tmpDir, "fetch", "--quiet", "--force", - "--update-head-ok", "--", repoDir, "refs/*:refs/*" }, {}, true); + "--update-head-ok", "--", repoDir, "refs/*:refs/*" }, true); } runProgram("git", true, { "-C", tmpDir, "checkout", "--quiet", input.getRev()->gitRev() }); @@ -683,7 +683,7 @@ struct GitInputScheme : InputScheme { Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching submodules of '%s'", actualUrl)); - runProgram("git", true, { "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" }, {}, true); + runProgram("git", true, { "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" }, true); } filter = isNotDotGitDirectory; diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 6015f8ec7..b4150e9df 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -28,10 +28,9 @@ static RunOptions hgOptions(const Strings & args) } // runProgram wrapper that uses hgOptions instead of stock RunOptions. -static std::string runHg(const Strings & args, const std::optional & input = {}) +static std::string runHg(const Strings & args) { RunOptions opts = hgOptions(args); - opts.input = input; auto res = runProgram(std::move(opts)); diff --git a/src/libutil/processes.cc b/src/libutil/processes.cc index e8af12fbd..250092393 100644 --- a/src/libutil/processes.cc +++ b/src/libutil/processes.cc @@ -230,10 +230,9 @@ Pid startProcess(std::function fun, const ProcessOptions & options) return Pid{pid}; } -std::string runProgram(Path program, bool searchPath, const Strings & args, - const std::optional & input, bool isInteractive) +std::string runProgram(Path program, bool searchPath, const Strings & args, bool isInteractive) { - auto res = runProgram(RunOptions {.program = program, .searchPath = searchPath, .args = args, .input = input, .isInteractive = isInteractive}); + auto res = runProgram(RunOptions {.program = program, .searchPath = searchPath, .args = args, .isInteractive = isInteractive}); if (!statusOk(res.first)) throw ExecError(res.first, "program '%1%' %2%", program, statusToString(res.first)); @@ -262,20 +261,9 @@ void runProgram2(const RunOptions & options) { checkInterrupt(); - assert(!(options.standardIn && options.input)); - - std::unique_ptr source_; - Source * source = options.standardIn; - - if (options.input) { - source_ = std::make_unique(*options.input); - source = source_.get(); - } - /* Create a pipe. */ - Pipe out, in; + Pipe out; if (options.standardOut) out.create(); - if (source) in.create(); ProcessOptions processOptions; @@ -298,8 +286,6 @@ void runProgram2(const RunOptions & options) if (options.mergeStderrToStdout) if (dup2(STDOUT_FILENO, STDERR_FILENO) == -1) throw SysError("cannot dup stdout into stderr"); - if (source && dup2(in.readSide.get(), STDIN_FILENO) == -1) - throw SysError("dupping stdin"); if (options.chdir && chdir((*options.chdir).c_str()) == -1) throw SysError("chdir failed"); @@ -328,47 +314,12 @@ void runProgram2(const RunOptions & options) out.writeSide.close(); - std::thread writerThread; - - std::promise promise; - - Finally doJoin([&]() { - if (writerThread.joinable()) - writerThread.join(); - }); - - - if (source) { - in.readSide.close(); - writerThread = std::thread([&]() { - try { - std::vector buf(8 * 1024); - while (true) { - size_t n; - try { - n = source->read(buf.data(), buf.size()); - } catch (EndOfFile &) { - break; - } - writeFull(in.writeSide.get(), {buf.data(), n}); - } - promise.set_value(); - } catch (...) { - promise.set_exception(std::current_exception()); - } - in.writeSide.close(); - }); - } - if (options.standardOut) drainFD(out.readSide.get(), *options.standardOut); /* Wait for the child to finish. */ int status = pid.wait(); - /* Wait for the writer thread to finish. */ - if (source) promise.get_future().get(); - if (status) throw ExecError(status, "program '%1%' %2%", options.program, statusToString(status)); } diff --git a/src/libutil/processes.hh b/src/libutil/processes.hh index 001e1fa12..48a195481 100644 --- a/src/libutil/processes.hh +++ b/src/libutil/processes.hh @@ -71,8 +71,7 @@ Pid startProcess(std::function fun, const ProcessOptions & options = Pro * shell backtick operator). */ std::string runProgram(Path program, bool searchPath = false, - const Strings & args = Strings(), - const std::optional & input = {}, bool isInteractive = false); + const Strings & args = Strings(), bool isInteractive = false); struct RunOptions { @@ -83,8 +82,6 @@ struct RunOptions std::optional gid; std::optional chdir; std::optional> environment; - std::optional input; - Source * standardIn = nullptr; Sink * standardOut = nullptr; bool mergeStderrToStdout = false; bool isInteractive = false;