From 7130f0a3a6455cf27863f0bbccf0974cfcc82fa2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 Jun 2020 04:04:41 +0000 Subject: [PATCH 01/13] Don't need abstract `struct Derivation` in local-store --- src/libstore/local-store.hh | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index e17cc45ae..872500957 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -23,9 +23,6 @@ namespace nix { const int nixSchemaVersion = 10; -struct Derivation; - - struct OptimiseStats { unsigned long filesLinked = 0; From 18493fd9c48676ab26854739db67ac5d76ff9347 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 Jun 2020 03:56:48 +0000 Subject: [PATCH 02/13] Move some Store functions from derivations.cc to store-api.cc This further continues with the dependency inverstion. Also I just went ahead and exposed `parseDerivation`: it seems like the more proper building block, and not a bad thing to expose if we are trying to be less wedded to drv files on disk anywas. --- src/libexpr/primops.cc | 2 +- src/libstore/derivations.cc | 31 +------------------------------ src/libstore/derivations.hh | 2 +- src/libstore/store-api.cc | 20 +++++++++++++++++++- src/nix/repl.cc | 28 ++++++++++++++++------------ 5 files changed, 38 insertions(+), 45 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 907f15246..0c2371c43 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -103,7 +103,7 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args // FIXME if (state.store->isStorePath(path) && state.store->isValidPath(state.store->parseStorePath(path)) && isDerivation(path)) { - Derivation drv = readDerivation(*state.store, realPath); + Derivation drv = state.store->readDerivation(state.store->parseStorePath(path)); Value & w = *state.allocValue(); state.mkAttrs(w, 3 + drv.outputs.size()); Value * v2 = state.allocAttr(w, state.sDrvPath); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b95f7bfdc..ea30813fa 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -3,7 +3,6 @@ #include "globals.hh" #include "util.hh" #include "worker-protocol.hh" -#include "fs-accessor.hh" #include "istringstream_nocopy.hh" namespace nix { @@ -120,7 +119,7 @@ static StringSet parseStrings(std::istream & str, bool arePaths) } -static Derivation parseDerivation(const Store & store, const string & s) +Derivation parseDerivation(const Store & store, const string & s) { Derivation drv; istringstream_nocopy str(s); @@ -173,34 +172,6 @@ static Derivation parseDerivation(const Store & store, const string & s) } -Derivation readDerivation(const Store & store, const Path & drvPath) -{ - try { - return parseDerivation(store, readFile(drvPath)); - } catch (FormatError & e) { - throw Error("error parsing derivation '%1%': %2%", drvPath, e.msg()); - } -} - - -Derivation Store::derivationFromPath(const StorePath & drvPath) -{ - ensurePath(drvPath); - return readDerivation(drvPath); -} - - -Derivation Store::readDerivation(const StorePath & drvPath) -{ - auto accessor = getFSAccessor(); - try { - return parseDerivation(*this, accessor->readFile(printStorePath(drvPath))); - } catch (FormatError & e) { - throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg()); - } -} - - static void printString(string & res, std::string_view s) { char buf[s.size() * 2 + 2]; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 856aae59f..2adbf6b94 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -77,7 +77,7 @@ StorePath writeDerivation(ref store, const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair); /* Read a derivation from a file. */ -Derivation readDerivation(const Store & store, const Path & drvPath); +Derivation parseDerivation(const Store & store, const string & s); // FIXME: remove bool isDerivation(const string & fileName); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index aae227bae..0147445ff 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1,11 +1,11 @@ #include "crypto.hh" +#include "fs-accessor.hh" #include "globals.hh" #include "store-api.hh" #include "util.hh" #include "nar-info-disk-cache.hh" #include "thread-pool.hh" #include "json.hh" -#include "derivations.hh" #include "url.hh" #include @@ -821,6 +821,24 @@ std::string makeFixedOutputCA(FileIngestionMethod recursive, const Hash & hash) } +Derivation Store::derivationFromPath(const StorePath & drvPath) +{ + ensurePath(drvPath); + return readDerivation(drvPath); +} + + +Derivation Store::readDerivation(const StorePath & drvPath) +{ + auto accessor = getFSAccessor(); + try { + return parseDerivation(*this, accessor->readFile(printStorePath(drvPath))); + } catch (FormatError & e) { + throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg()); + } +} + + } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 4bcaaeebf..b46f7015f 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -66,7 +66,7 @@ struct NixRepl : gc void mainLoop(const std::vector & files); StringSet completePrefix(string prefix); bool getLine(string & input, const std::string &prompt); - Path getDerivationPath(Value & v); + StorePath getDerivationPath(Value & v); bool processLine(string line); void loadFile(const Path & path); void initEnv(); @@ -377,13 +377,16 @@ bool isVarName(const string & s) } -Path NixRepl::getDerivationPath(Value & v) { +StorePath NixRepl::getDerivationPath(Value & v) { auto drvInfo = getDerivation(*state, v, false); if (!drvInfo) throw Error("expression does not evaluate to a derivation, so I can't build it"); - Path drvPath = drvInfo->queryDrvPath(); - if (drvPath == "" || !state->store->isValidPath(state->store->parseStorePath(drvPath))) - throw Error("expression did not evaluate to a valid derivation"); + Path drvPathRaw = drvInfo->queryDrvPath(); + if (drvPathRaw == "") + throw Error("expression did not evaluate to a valid derivation (no drv path)"); + StorePath drvPath = state->store->parseStorePath(drvPathRaw); + if (!state->store->isValidPath(drvPath)) + throw Error("expression did not evaluate to a valid derivation (invalid drv path)"); return drvPath; } @@ -476,29 +479,30 @@ bool NixRepl::processLine(string line) evalString("drv: (import {}).runCommand \"shell\" { buildInputs = [ drv ]; } \"\"", f); state->callFunction(f, v, result, Pos()); - Path drvPath = getDerivationPath(result); - runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPath}); + StorePath drvPath = getDerivationPath(result); + runProgram(settings.nixBinDir + "/nix-shell", Strings{state->store->printStorePath(drvPath)}); } else if (command == ":b" || command == ":i" || command == ":s") { Value v; evalString(arg, v); - Path drvPath = getDerivationPath(v); + StorePath drvPath = getDerivationPath(v); + Path drvPathRaw = state->store->printStorePath(drvPath); if (command == ":b") { /* We could do the build in this process using buildPaths(), but doing it in a child makes it easier to recover from problems / SIGINT. */ - if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) { - auto drv = readDerivation(*state->store, drvPath); + if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPathRaw}) == 0) { + auto drv = state->store->readDerivation(drvPath); std::cout << std::endl << "this derivation produced the following outputs:" << std::endl; for (auto & i : drv.outputs) std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path)); } } else if (command == ":i") { - runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPath}); + runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPathRaw}); } else { - runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPath}); + runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPathRaw}); } } From f1c7746eb407258d77b2f7fffec0e5d4facf516a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 5 Jul 2020 21:39:04 +0000 Subject: [PATCH 03/13] See if setting -std=c++17 for perl bindings helps --- perl/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perl/Makefile b/perl/Makefile index 7ddb0cf69..259ed7dc3 100644 --- a/perl/Makefile +++ b/perl/Makefile @@ -1,6 +1,6 @@ makefiles = local.mk -GLOBAL_CXXFLAGS += -g -Wall +GLOBAL_CXXFLAGS += -g -Wall -std=c++17 -include Makefile.config From e3b394b6e8420795c1976195184921df2c2b3e83 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 16 Jul 2020 09:36:02 -0400 Subject: [PATCH 04/13] Small namespace fix --- src/libstore/derivations.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index d32793b6a..d170d7223 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -76,7 +76,7 @@ StorePath writeDerivation(ref store, const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair); /* Read a derivation from a file. */ -Derivation parseDerivation(const Store & store, string && s); +Derivation parseDerivation(const Store & store, std::string && s); // FIXME: remove bool isDerivation(const string & fileName); From 6ccfdb79c7779f9eedeea73a21df694551c9893e Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 21 Jul 2020 23:38:18 +0200 Subject: [PATCH 05/13] libutil/logging: extend `internal-json` logger to make it more machine-readable The new error-format is pretty nice from a UX point-of-view, however it's fairly hard to parse the output e.g. for editor plugins such as vim-ale[1] that use `nix-instantiate --parse` to determine syntax errors in Nix expression files. This patch extends the `internal-json` logger by adding the fields `line`, `column` and `file` to easily locate an error in a file and the field `raw_msg` which contains the error-message itself without code-lines and additional helpers. An exemplary output may look like this: ``` [nix-shell]$ ./inst/bin/nix-instantiate ~/test.nix --log-format minimal {"action":"msg","column":1,"file":"/home/ma27/test.nix","level":0,"line":4,"raw_msg":"syntax error, unexpected IF, expecting $end","msg":""} ``` [1] https://github.com/dense-analysis/ale --- doc/manual/command-ref/opt-common.xml | 9 ++++++++- src/libutil/logging.cc | 27 +++++++++++++++++++++++++++ src/libutil/tests/logging.cc | 18 ++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/doc/manual/command-ref/opt-common.xml b/doc/manual/command-ref/opt-common.xml index a68eef1d0..093bc2526 100644 --- a/doc/manual/command-ref/opt-common.xml +++ b/doc/manual/command-ref/opt-common.xml @@ -106,7 +106,14 @@ internal-json - Outputs the logs in a structured manner. NOTE: the json schema is not guarantees to be stable between releases. + + Outputs the logs in a structured manner. + + + While the schema itself is relatively stable, the format of the error-messages (namely of the msg-field) can change between several releases. + + + bar diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 832aee783..cbbf64395 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -184,6 +184,33 @@ struct JSONLogger : Logger { json["action"] = "msg"; json["level"] = ei.level; json["msg"] = oss.str(); + json["raw_msg"] = ei.hint->str(); + + if (ei.errPos.has_value() && (*ei.errPos)) { + json["line"] = ei.errPos->line; + json["column"] = ei.errPos->column; + json["file"] = ei.errPos->file; + } else { + json["line"] = nullptr; + json["column"] = nullptr; + json["file"] = nullptr; + } + + if (loggerSettings.showTrace.get() && !ei.traces.empty()) { + nlohmann::json traces = nlohmann::json::array(); + for (auto iter = ei.traces.rbegin(); iter != ei.traces.rend(); ++iter) { + nlohmann::json stackFrame; + stackFrame["raw_msg"] = iter->hint.str(); + if (iter->pos.has_value() && (*iter->pos)) { + stackFrame["line"] = iter->pos->line; + stackFrame["column"] = iter->pos->column; + stackFrame["file"] = iter->pos->file; + } + traces.push_back(stackFrame); + } + + json["trace"] = traces; + } write(json); } diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index ad588055f..7e53f17c6 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -34,6 +34,24 @@ namespace nix { } } + TEST(logEI, jsonOutput) { + SymbolTable testTable; + auto problem_file = testTable.create("random.nix"); + testing::internal::CaptureStderr(); + + makeJSONLogger(*logger)->logEI({ + .name = "error name", + .description = "error without any code lines.", + .hint = hintfmt("this hint has %1% templated %2%!!", + "yellow", + "values"), + .errPos = Pos(foFile, problem_file, 02, 13) + }); + + auto str = testing::internal::GetCapturedStderr(); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1mrandom.nix\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n@nix {\"action\":\"msg\",\"column\":13,\"file\":\"random.nix\",\"level\":0,\"line\":2,\"msg\":\"\\u001b[31;1merror:\\u001b[0m\\u001b[34;1m --- error name --- error-unit-test\\u001b[0m\\n\\u001b[34;1mat: \\u001b[33;1m(2:13)\\u001b[34;1m in file: \\u001b[0mrandom.nix\\n\\nerror without any code lines.\\n\\nthis hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\",\"raw_msg\":\"this hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\"}\n"); + } + TEST(logEI, appendingHintsToPreviousError) { MakeError(TestError, Error); From a0f19d9f3a009961869a077922f15986ce05738e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 27 Aug 2020 14:48:08 +0200 Subject: [PATCH 06/13] RemoteStore::addToStore(): Fix race between stderrThread and NAR writer As pointed out by @B4dM4n, the call to to.flush() on stderrThread is unsafe because the NAR writer thread is also writing to 'to'. Fixes #3943. --- src/libstore/remote-store.cc | 13 ++++++++----- src/libstore/remote-store.hh | 2 +- src/libutil/serialise.hh | 6 ++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index adba17c5e..e4a4ef5af 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -284,9 +284,9 @@ struct ConnectionHandle RemoteStore::Connection * operator -> () { return &*handle; } - void processStderr(Sink * sink = 0, Source * source = 0) + void processStderr(Sink * sink = 0, Source * source = 0, bool flush = true) { - auto ex = handle->processStderr(sink, source); + auto ex = handle->processStderr(sink, source, flush); if (ex) { daemonException = true; std::rethrow_exception(ex); @@ -535,6 +535,8 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) { + conn->to.flush(); + std::exception_ptr ex; struct FramedSink : BufferedSink @@ -574,7 +576,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, std::thread stderrThread([&]() { try { - conn.processStderr(); + conn.processStderr(nullptr, nullptr, false); } catch (...) { ex = std::current_exception(); } @@ -884,9 +886,10 @@ static Logger::Fields readFields(Source & from) } -std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * source) +std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * source, bool flush) { - to.flush(); + if (flush) + to.flush(); while (true) { diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index b319e774b..7cf4c4d12 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -114,7 +114,7 @@ protected: virtual ~Connection(); - std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0); + std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0, bool flush = true); }; ref openConnectionWrapper(); diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 69ae0874a..8f17bc34c 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -23,7 +23,8 @@ struct Sink }; -/* A buffered abstract sink. */ +/* A buffered abstract sink. Warning: a BufferedSink should not be + used from multiple threads concurrently. */ struct BufferedSink : virtual Sink { size_t bufSize, bufPos; @@ -66,7 +67,8 @@ struct Source }; -/* A buffered abstract source. */ +/* A buffered abstract source. Warning: a BufferedSink should not be + used from multiple threads concurrently. */ struct BufferedSource : Source { size_t bufSize, bufPosIn, bufPosOut; From e915fd6d2afb0299bcb77069503698faabe5f233 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 27 Aug 2020 14:51:50 +0200 Subject: [PATCH 07/13] Typo --- src/libutil/serialise.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 8f17bc34c..a6f1c42e9 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -67,7 +67,7 @@ struct Source }; -/* A buffered abstract source. Warning: a BufferedSink should not be +/* A buffered abstract source. Warning: a BufferedSource should not be used from multiple threads concurrently. */ struct BufferedSource : Source { From 626200713bb3cc844a9feb6af583c9b6b42c6dbc Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Thu, 27 Aug 2020 12:28:12 -0400 Subject: [PATCH 08/13] Pass all args when auto-calling a function with an ellipsis The command line options --arg and --argstr that are used by a bunch of CLI commands to pass arguments to top-level functions in files go through the same code-path as auto-calling top-level functions with their default arguments - this, however, was only passing the arguments that were *explicitly* mentioned in the formals of the function - in the case of an as-pattern with an ellipsis (eg args @ { ... }) extra passed arguments would get omitted. This fixes that to instead pass *all* specified auto args in the case that our function has an ellipsis. Fixes #598 --- src/libexpr/eval.cc | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 0123070d1..00191bce0 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1299,12 +1299,23 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) Value * actualArgs = allocValue(); mkAttrs(*actualArgs, fun.lambda.fun->formals->formals.size()); - for (auto & i : fun.lambda.fun->formals->formals) { - Bindings::iterator j = args.find(i.name); - if (j != args.end()) - actualArgs->attrs->push_back(*j); - else if (!i.def) - throwTypeError("cannot auto-call a function that has an argument without a default value ('%1%')", i.name); + if (fun.lambda.fun->formals->ellipsis) { + // If the formals have an ellipsis (eg the function accepts extra args) pass + // all available automatic arguments (which includes arguments specified on + // the command line via --arg/--argstr) + for (auto& v : args) { + actualArgs->attrs->push_back(v); + } + } else { + // Otherwise, only pass the arguments that the function accepts + for (auto & i : fun.lambda.fun->formals->formals) { + Bindings::iterator j = args.find(i.name); + if (j != args.end()) { + actualArgs->attrs->push_back(*j); + } else if (!i.def) { + throwTypeError("cannot auto-call a function that has an argument without a default value ('%1%')", i.name); + } + } } actualArgs->attrs->sort(); From 3156560d413634eea170e8dd88b3c2208149d999 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 28 Aug 2020 18:16:03 +0200 Subject: [PATCH 09/13] nix develop: Set output paths to writable locations Currently, they're set to $(pwd)/outputs/$outputName. This allows commands like 'make install' to work. --- flake.nix | 11 ++----- src/nix/develop.cc | 81 +++++++++++++++++++++++++++++----------------- src/nix/get-env.sh | 17 +++++----- 3 files changed, 63 insertions(+), 46 deletions(-) diff --git a/flake.nix b/flake.nix index a707e90e7..32ebb4936 100644 --- a/flake.nix +++ b/flake.nix @@ -421,6 +421,8 @@ stdenv.mkDerivation { name = "nix"; + outputs = [ "out" "dev" "doc" ]; + buildInputs = buildDeps ++ propagatedDeps ++ perlDeps; inherit configureFlags; @@ -428,15 +430,6 @@ enableParallelBuilding = true; installFlags = "sysconfdir=$(out)/etc"; - - shellHook = - '' - export prefix=$(pwd)/inst - configureFlags+=" --prefix=$prefix" - PKG_CONFIG_PATH=$prefix/lib/pkgconfig:$PKG_CONFIG_PATH - PATH=$prefix/bin:$PATH - unset PYTHONPATH - ''; }); }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 8a14e45c9..20316c477 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -15,7 +15,7 @@ struct Var { bool exported = true; bool associative = false; - std::string value; // quoted string or array + std::string quoted; // quoted string or array }; struct BuildEnvironment @@ -75,12 +75,12 @@ BuildEnvironment readEnvironment(const Path & path) else if (std::regex_search(pos, file.cend(), match, varRegex, std::regex_constants::match_continuous)) { pos = match[0].second; - res.env.insert({match[1], Var { .exported = exported.count(match[1]) > 0, .value = match[2] }}); + res.env.insert({match[1], Var { .exported = exported.count(match[1]) > 0, .quoted = match[2] }}); } else if (std::regex_search(pos, file.cend(), match, assocArrayRegex, std::regex_constants::match_continuous)) { pos = match[0].second; - res.env.insert({match[1], Var { .associative = true, .value = match[2] }}); + res.env.insert({match[1], Var { .associative = true, .quoted = match[2] }}); } else if (std::regex_search(pos, file.cend(), match, functionRegex, std::regex_constants::match_continuous)) { @@ -92,6 +92,8 @@ BuildEnvironment readEnvironment(const Path & path) path, file.substr(pos - file.cbegin(), 60)); } + res.env.erase("__output"); + return res; } @@ -125,27 +127,32 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) /* Rehash and write the derivation. FIXME: would be nice to use 'buildDerivation', but that's privileged. */ drv.name += "-env"; - for (auto & output : drv.outputs) - drv.env.erase(output.first); - drv.outputs = {{"out", DerivationOutput { .output = DerivationOutputInputAddressed { .path = StorePath::dummy }}}}; - drv.env["out"] = ""; - drv.env["_outputs_saved"] = drv.env["outputs"]; - drv.env["outputs"] = "out"; + for (auto & output : drv.outputs) { + output.second = { .output = DerivationOutputInputAddressed { .path = StorePath::dummy } }; + drv.env[output.first] = ""; + } drv.inputSrcs.insert(std::move(getEnvShPath)); Hash h = std::get<0>(hashDerivationModulo(*store, drv, true)); - auto shellOutPath = store->makeOutputPath("out", h, drv.name); - drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputInputAddressed { - .path = shellOutPath - } }); - drv.env["out"] = store->printStorePath(shellOutPath); - auto shellDrvPath2 = writeDerivation(*store, drv); + + for (auto & output : drv.outputs) { + auto outPath = store->makeOutputPath(output.first, h, drv.name); + output.second = { .output = DerivationOutputInputAddressed { .path = outPath } }; + drv.env[output.first] = store->printStorePath(outPath); + } + + auto shellDrvPath = writeDerivation(*store, drv); /* Build the derivation. */ - store->buildPaths({{shellDrvPath2}}); + store->buildPaths({{shellDrvPath}}); - assert(store->isValidPath(shellOutPath)); + for (auto & outPath : drv.outputPaths(*store)) { + assert(store->isValidPath(outPath)); + auto outPathS = store->toRealPath(outPath); + if (lstat(outPathS).st_size) + return outPath; + } - return shellOutPath; + throw Error("get-env.sh failed to produce an environment"); } struct Common : InstallableCommand, MixProfile @@ -171,8 +178,12 @@ struct Common : InstallableCommand, MixProfile "UID", }; - void makeRcScript(const BuildEnvironment & buildEnvironment, std::ostream & out) + std::string makeRcScript( + const BuildEnvironment & buildEnvironment, + const Path & outputsDir = absPath(".") + "/outputs") { + std::ostringstream out; + out << "unset shellHook\n"; out << "nix_saved_PATH=\"$PATH\"\n"; @@ -180,9 +191,9 @@ struct Common : InstallableCommand, MixProfile for (auto & i : buildEnvironment.env) { if (!ignoreVars.count(i.first) && !hasPrefix(i.first, "BASH_")) { if (i.second.associative) - out << fmt("declare -A %s=(%s)\n", i.first, i.second.value); + out << fmt("declare -A %s=(%s)\n", i.first, i.second.quoted); else { - out << fmt("%s=%s\n", i.first, i.second.value); + out << fmt("%s=%s\n", i.first, i.second.quoted); if (i.second.exported) out << fmt("export %s\n", i.first); } @@ -193,13 +204,26 @@ struct Common : InstallableCommand, MixProfile out << buildEnvironment.bashFunctions << "\n"; - // FIXME: set outputs - out << "export NIX_BUILD_TOP=\"$(mktemp -d --tmpdir nix-shell.XXXXXX)\"\n"; for (auto & i : {"TMP", "TMPDIR", "TEMP", "TEMPDIR"}) out << fmt("export %s=\"$NIX_BUILD_TOP\"\n", i); out << "eval \"$shellHook\"\n"; + + /* Substitute occurrences of output paths. */ + auto outputs = buildEnvironment.env.find("outputs"); + assert(outputs != buildEnvironment.env.end()); + + // FIXME: properly unquote 'outputs'. + StringMap rewrites; + for (auto & outputName : tokenizeString>(replaceStrings(outputs->second.quoted, "'", ""))) { + auto from = buildEnvironment.env.find(outputName); + assert(from != buildEnvironment.env.end()); + // FIXME: unquote + rewrites.insert({from->second.quoted, outputsDir + "/" + outputName}); + } + + return rewriteStrings(out.str(), rewrites); } Strings getDefaultFlakeAttrPaths() override @@ -288,19 +312,18 @@ struct CmdDevelop : Common, MixEnvironment auto [rcFileFd, rcFilePath] = createTempFile("nix-shell"); - std::ostringstream ss; - makeRcScript(buildEnvironment, ss); + auto script = makeRcScript(buildEnvironment); - ss << fmt("rm -f '%s'\n", rcFilePath); + script += fmt("rm -f '%s'\n", rcFilePath); if (!command.empty()) { std::vector args; for (auto s : command) args.push_back(shellEscape(s)); - ss << fmt("exec %s\n", concatStringsSep(" ", args)); + script += fmt("exec %s\n", concatStringsSep(" ", args)); } - writeFull(rcFileFd.get(), ss.str()); + writeFull(rcFileFd.get(), script); stopProgressBar(); @@ -362,7 +385,7 @@ struct CmdPrintDevEnv : Common stopProgressBar(); - makeRcScript(buildEnvironment, std::cout); + std::cout << makeRcScript(buildEnvironment); } }; diff --git a/src/nix/get-env.sh b/src/nix/get-env.sh index 2e0e83561..091c0f573 100644 --- a/src/nix/get-env.sh +++ b/src/nix/get-env.sh @@ -1,12 +1,6 @@ set -e if [ -e .attrs.sh ]; then source .attrs.sh; fi -outputs=$_outputs_saved -for __output in $_outputs_saved; do - declare "$__output"="$out" -done -unset _outputs_saved __output - export IN_NIX_SHELL=impure export dontAddDisableDepTrack=1 @@ -14,5 +8,12 @@ if [[ -n $stdenv ]]; then source $stdenv/setup fi -export > $out -set >> $out +for __output in $outputs; do + if [[ -z $__done ]]; then + export > ${!__output} + set >> ${!__output} + __done=1 + else + echo -n >> ${!__output} + fi +done From 50a8710ed14c7a0236a656bb62d5117519ec5813 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 28 Aug 2020 18:43:34 +0200 Subject: [PATCH 10/13] Close stdin while running tests For some reason, the bash shell started by 'nix develop' sometimes reads from stdin, which can hang. --- mk/tests.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mk/tests.mk b/mk/tests.mk index 2e39bb694..c1e140bac 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -8,7 +8,7 @@ define run-install-test .PHONY: $1.test $1.test: $1 $(test-deps) - @env TEST_NAME=$(notdir $(basename $1)) TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 + @env TEST_NAME=$(notdir $(basename $1)) TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 < /dev/null endef From f15651303f8596bf34c67fc8d536b1e9e7843a87 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 28 Aug 2020 19:24:29 +0200 Subject: [PATCH 11/13] nix develop: Add convenience flags for running specific phases For example, for building the Nix flake, you would do: $ nix develop --configure $ nix develop --install $ nix develop --installcheck --- src/nix/develop.cc | 55 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 20316c477..516e7bda9 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -264,6 +264,7 @@ struct Common : InstallableCommand, MixProfile struct CmdDevelop : Common, MixEnvironment { std::vector command; + std::optional phase; CmdDevelop() { @@ -277,6 +278,43 @@ struct CmdDevelop : Common, MixEnvironment command = ss; }} }); + + addFlag({ + .longName = "phase", + .description = "phase to run (e.g. `build` or `configure`)", + .labels = {"phase-name"}, + .handler = {&phase}, + }); + + addFlag({ + .longName = "configure", + .description = "run the configure phase", + .handler = {&phase, {"configure"}}, + }); + + addFlag({ + .longName = "build", + .description = "run the build phase", + .handler = {&phase, {"build"}}, + }); + + addFlag({ + .longName = "check", + .description = "run the check phase", + .handler = {&phase, {"check"}}, + }); + + addFlag({ + .longName = "install", + .description = "run the install phase", + .handler = {&phase, {"install"}}, + }); + + addFlag({ + .longName = "installcheck", + .description = "run the installcheck phase", + .handler = {&phase, {"installCheck"}}, + }); } std::string description() override @@ -314,9 +352,22 @@ struct CmdDevelop : Common, MixEnvironment auto script = makeRcScript(buildEnvironment); - script += fmt("rm -f '%s'\n", rcFilePath); + if (verbosity >= lvlDebug) + script += "set -x\n"; - if (!command.empty()) { + script += fmt("rm -f '%s'\n", rcFilePath); + + if (phase) { + if (!command.empty()) + throw UsageError("you cannot use both '--command' and '--phase'"); + // FIXME: foundMakefile is set by buildPhase, need to get + // rid of that. + script += fmt("foundMakefile=1\n"); + script += fmt("runHook %1%Phase\n", *phase); + script += fmt("exit 0\n", *phase); + } + + else if (!command.empty()) { std::vector args; for (auto s : command) args.push_back(shellEscape(s)); From 8757e7022a9b407b69f173b3793fababf1e8ed84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Sun, 30 Aug 2020 19:41:21 +0200 Subject: [PATCH 12/13] mention how to run a single functional test for faster feedback loop --- doc/manual/hacking.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/manual/hacking.xml b/doc/manual/hacking.xml index d25d4b84a..267386d42 100644 --- a/doc/manual/hacking.xml +++ b/doc/manual/hacking.xml @@ -57,6 +57,12 @@ To install it in $(pwd)/inst and test it: nix (Nix) 2.4 +To run a functional test: + + +make tests/test-name-should-auto-complete.sh.test + + If you have a flakes-enabled nix you can replace: From f38fe24346ca6b0201aed716db9cf47336a708ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Sun, 30 Aug 2020 22:52:34 +0200 Subject: [PATCH 13/13] speed up CI --- .github/workflows/test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e16e6c62d..1f504a8ea 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,5 +13,7 @@ jobs: with: fetch-depth: 0 - uses: cachix/install-nix-action@v10 + with: + skip_adding_nixpkgs_channel: true #- run: nix flake check - run: nix-build -A checks.$(if [[ `uname` = Linux ]]; then echo x86_64-linux; else echo x86_64-darwin; fi)