From 4ed66735b67b785efad7e23599a1777343c264b5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Sep 2021 23:22:09 +0200 Subject: [PATCH] RunOptions: Use designated initializers Also get rid of _killStderr because it wasn't actually checked anywhere. --- src/libfetchers/git.cc | 25 ++++++++++----------- src/libfetchers/mercurial.cc | 25 +++++++++------------ src/libstore/build/derivation-goal.cc | 12 +++++----- src/libstore/build/local-derivation-goal.cc | 14 +++++++----- src/libutil/util.cc | 12 ++-------- src/libutil/util.hh | 14 ++++-------- src/nix/repl.cc | 10 +++++---- 7 files changed, 48 insertions(+), 64 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 6b1057c63..9e895904e 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -413,17 +413,14 @@ struct GitInputScheme : InputScheme AutoDelete delTmpDir(tmpDir, true); PathFilter filter = defaultPathFilter; - RunOptions checkCommitOpts( - "git", - { "-C", repoDir, "cat-file", "commit", input.getRev()->gitRev() } - ); - checkCommitOpts.searchPath = true; - checkCommitOpts.mergeStderrToStdout = true; - - auto result = runProgram(checkCommitOpts); + auto result = runProgram({ + .program = "git", + .args = { "-C", repoDir, "cat-file", "commit", input.getRev()->gitRev() }, + .mergeStderrToStdout = true + }); if (WEXITSTATUS(result.first) == 128 - && result.second.find("bad file") != std::string::npos - ) { + && result.second.find("bad file") != std::string::npos) + { throw Error( "Cannot find Git revision '%s' in ref '%s' of repository '%s'! " "Please make sure that the " ANSI_BOLD "rev" ANSI_NORMAL " exists on the " @@ -455,9 +452,11 @@ struct GitInputScheme : InputScheme // FIXME: should pipe this, or find some better way to extract a // revision. auto source = sinkToSource([&](Sink & sink) { - RunOptions gitOptions("git", { "-C", repoDir, "archive", input.getRev()->gitRev() }); - gitOptions.standardOut = &sink; - runProgram2(gitOptions); + runProgram2({ + .program = "git", + .args = { "-C", repoDir, "archive", input.getRev()->gitRev() }, + .standardOut = &sink + }); }); unpackTarfile(*source, tmpDir); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 191c5af54..d52d4641b 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -11,28 +11,27 @@ using namespace std::string_literals; namespace nix::fetchers { -namespace { - -RunOptions hgOptions(const Strings & args) +static RunOptions hgOptions(const Strings & args) { - RunOptions opts("hg", args); - opts.searchPath = true; - auto env = getEnv(); // Set HGPLAIN: this means we get consistent output from hg and avoids leakage from a user or system .hgrc. env["HGPLAIN"] = ""; - opts.environment = env; - return opts; + return { + .program = "hg", + .searchPath = true, + .args = args, + .environment = env + }; } // runProgram wrapper that uses hgOptions instead of stock RunOptions. -string runHg(const Strings & args, const std::optional & input = {}) +static string runHg(const Strings & args, const std::optional & input = {}) { RunOptions opts = hgOptions(args); opts.input = input; - auto res = runProgram(opts); + auto res = runProgram(std::move(opts)); if (!statusOk(res.first)) throw ExecError(res.first, fmt("hg %1%", statusToString(res.first))); @@ -40,8 +39,6 @@ string runHg(const Strings & args, const std::optional & input = {} return res.second; } -} - struct MercurialInputScheme : InputScheme { std::optional inputFromURL(const ParsedURL & url) override @@ -254,9 +251,7 @@ struct MercurialInputScheme : InputScheme have to pull again. */ if (!(input.getRev() && pathExists(cacheDir) - && runProgram( - hgOptions({ "log", "-R", cacheDir, "-r", input.getRev()->gitRev(), "--template", "1" }) - .killStderr(true)).second == "1")) + && runProgram(hgOptions({ "log", "-R", cacheDir, "-r", input.getRev()->gitRev(), "--template", "1" })).second == "1")) { Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching Mercurial repository '%s'", actualUrl)); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b94accb73..b58d04c05 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -774,9 +774,6 @@ void runPostBuildHook( hookEnvironment.emplace("OUT_PATHS", chomp(concatStringsSep(" ", store.printStorePathSet(outputPaths)))); hookEnvironment.emplace("NIX_CONFIG", globalConfig.toKeyValue()); - RunOptions opts(settings.postBuildHook, {}); - opts.environment = hookEnvironment; - struct LogSink : Sink { Activity & act; std::string currentLine; @@ -807,9 +804,12 @@ void runPostBuildHook( }; LogSink sink(act); - opts.standardOut = &sink; - opts.mergeStderrToStdout = true; - runProgram2(opts); + runProgram2({ + .program = settings.postBuildHook, + .environment = hookEnvironment, + .standardOut = &sink, + .mergeStderrToStdout = true, + }); } void DerivationGoal::buildDone() diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 5cc0691e8..446c19ae0 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -66,12 +66,14 @@ void handleDiffHook( auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { try { - RunOptions diffHookOptions(diffHook,{tryA, tryB, drvPath, tmpDir}); - diffHookOptions.searchPath = true; - diffHookOptions.uid = uid; - diffHookOptions.gid = gid; - diffHookOptions.chdir = "/"; - auto diffRes = runProgram(diffHookOptions); + auto diffRes = runProgram({ + .program = diffHook, + .searchPath = true, + .args = {tryA, tryB, drvPath, tmpDir}, + .uid = uid, + .gid = gid, + .chdir = "/" + }); if (!statusOk(diffRes.first)) throw ExecError(diffRes.first, "diff-hook program '%1%' %2%", diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 72405ac83..d2660a5d3 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1034,17 +1034,10 @@ std::vector stringsToCharPtrs(const Strings & ss) return res; } -// Output = "standard out" output stream string runProgram(Path program, bool searchPath, const Strings & args, const std::optional & input) { - RunOptions opts(program, args); - opts.searchPath = searchPath; - // This allows you to refer to a program with a pathname relative to the - // PATH variable. - opts.input = input; - - auto res = runProgram(opts); + auto res = runProgram({.program = program, .searchPath = searchPath, .args = args, .input = input}); if (!statusOk(res.first)) throw ExecError(res.first, fmt("program '%1%' %2%", program, statusToString(res.first))); @@ -1053,9 +1046,8 @@ string runProgram(Path program, bool searchPath, const Strings & args, } // Output = error code + "standard out" output stream -std::pair runProgram(const RunOptions & options_) +std::pair runProgram(RunOptions && options) { - RunOptions options(options_); StringSink sink; options.standardOut = &sink; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index a8dd4bd47..bee77b53f 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -276,26 +276,20 @@ string runProgram(Path program, bool searchPath = false, struct RunOptions { + Path program; + bool searchPath = true; + Strings args; std::optional uid; std::optional gid; std::optional chdir; std::optional> environment; - Path program; - bool searchPath = true; - Strings args; std::optional input; Source * standardIn = nullptr; Sink * standardOut = nullptr; bool mergeStderrToStdout = false; - bool _killStderr = false; - - RunOptions(const Path & program, const Strings & args) - : program(program), args(args) { }; - - RunOptions & killStderr(bool v) { _killStderr = true; return *this; } }; -std::pair runProgram(const RunOptions & options); +std::pair runProgram(RunOptions && options); void runProgram2(const RunOptions & options); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index b711f4163..abb10f674 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -110,11 +110,13 @@ string runNix(Path program, const Strings & args, { auto subprocessEnv = getEnv(); subprocessEnv["NIX_CONFIG"] = globalConfig.toKeyValue(); - RunOptions opts(settings.nixBinDir+ "/" + program, args); - opts.input = input; - opts.environment = subprocessEnv; - auto res = runProgram(opts); + auto res = runProgram({ + .program = settings.nixBinDir+ "/" + program, + .args = args, + .environment = subprocessEnv, + .input = input, + }); if (!statusOk(res.first)) throw ExecError(res.first, fmt("program '%1%' %2%", program, statusToString(res.first)));