From 1290411c2d0c62dd1761485f55292dc944eae55d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Apr 2020 14:00:12 +0200 Subject: [PATCH 1/7] fetchMercurial: Use inputFromAttrs() --- src/libexpr/primops/fetchMercurial.cc | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index f18351646..0a1ba49d5 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -54,15 +54,14 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar if (evalSettings.pureEval && !rev) throw Error("in pure evaluation mode, 'fetchMercurial' requires a Mercurial revision"); - auto parsedUrl = parseURL( - url.find("://") != std::string::npos - ? "hg+" + url - : "hg+file://" + url); - if (rev) parsedUrl.query.insert_or_assign("rev", rev->gitRev()); - if (ref) parsedUrl.query.insert_or_assign("ref", *ref); - // FIXME: use name - auto input = fetchers::inputFromURL(parsedUrl); + fetchers::Attrs attrs; + attrs.insert_or_assign("type", "hg"); + attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url); + if (ref) attrs.insert_or_assign("ref", *ref); + if (rev) attrs.insert_or_assign("rev", rev->gitRev()); + auto input = fetchers::inputFromAttrs(attrs); + // FIXME: use name auto [tree, input2] = input->fetchTree(state.store); state.mkAttrs(v, 8); From b3e5eea4a91400fb2a12aba4b07a94d03ba54605 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Apr 2020 16:28:07 +0200 Subject: [PATCH 2/7] Add function to allocate a Value in traceable memory --- src/libexpr/eval.cc | 8 ++++++++ src/libexpr/value.hh | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f963a42ca..2f5b1db47 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -22,6 +22,8 @@ #if HAVE_BOEHMGC +#define GC_INCLUDE_NEW + #include #include @@ -56,6 +58,12 @@ static char * dupStringWithLen(const char * s, size_t size) } +RootValue allocRootValue(Value * v) +{ + return std::allocate_shared(traceable_allocator(), v); +} + + static void printValue(std::ostream & str, std::set & active, const Value & v) { checkInterrupt(); diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 689373873..9cf2bf06a 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -261,4 +261,9 @@ typedef std::map ValueMap; #endif +/* A value allocated in traceable memory. */ +typedef std::shared_ptr RootValue; + +RootValue allocRootValue(Value * v); + } From 10e17eaa5802a3c368eee8408821828072e76ba7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Apr 2020 17:24:28 +0200 Subject: [PATCH 3/7] ValueMap, VectorVector: Use traceable_allocator We want to *trace* the 'Value *' arrays, not garbage-collect them! Otherwise the vectors/maps can end up pointing to nowhere. Fixes #3377. Closes #3384. --- src/libexpr/value.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 9cf2bf06a..71025824e 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -253,8 +253,8 @@ void mkPath(Value & v, const char * s); #if HAVE_BOEHMGC -typedef std::vector > ValueVector; -typedef std::map, gc_allocator > > ValueMap; +typedef std::vector > ValueVector; +typedef std::map, traceable_allocator > > ValueMap; #else typedef std::vector ValueVector; typedef std::map ValueMap; From 9f46f54de4e55267df492456fc0393f74616366b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Apr 2020 17:28:32 +0200 Subject: [PATCH 4/7] JSONSax: Use a RootValue More #3377. --- src/libexpr/json-to-value.cc | 63 ++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 1fdce1983..76e1a26bf 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -4,7 +4,6 @@ #include using json = nlohmann::json; -using std::unique_ptr; namespace nix { @@ -13,69 +12,69 @@ namespace nix { class JSONSax : nlohmann::json_sax { class JSONState { protected: - unique_ptr parent; - Value * v; + std::unique_ptr parent; + RootValue v; public: - virtual unique_ptr resolve(EvalState &) + virtual std::unique_ptr resolve(EvalState &) { throw std::logic_error("tried to close toplevel json parser state"); - }; - explicit JSONState(unique_ptr&& p) : parent(std::move(p)), v(nullptr) {}; - explicit JSONState(Value* v) : v(v) {}; - JSONState(JSONState& p) = delete; - Value& value(EvalState & state) + } + explicit JSONState(std::unique_ptr && p) : parent(std::move(p)) {} + explicit JSONState(Value * v) : v(allocRootValue(v)) {} + JSONState(JSONState & p) = delete; + Value & value(EvalState & state) { - if (v == nullptr) - v = state.allocValue(); - return *v; - }; - virtual ~JSONState() {}; - virtual void add() {}; + if (!v) + v = allocRootValue(state.allocValue()); + return **v; + } + virtual ~JSONState() {} + virtual void add() {} }; class JSONObjectState : public JSONState { using JSONState::JSONState; - ValueMap attrs = ValueMap(); - virtual unique_ptr resolve(EvalState & state) override + ValueMap attrs; + std::unique_ptr resolve(EvalState & state) override { - Value& v = parent->value(state); + Value & v = parent->value(state); state.mkAttrs(v, attrs.size()); for (auto & i : attrs) v.attrs->push_back(Attr(i.first, i.second)); return std::move(parent); } - virtual void add() override { v = nullptr; }; + void add() override { v = nullptr; } public: - void key(string_t& name, EvalState & state) + void key(string_t & name, EvalState & state) { - attrs[state.symbols.create(name)] = &value(state); + attrs.insert_or_assign(state.symbols.create(name), &value(state)); } }; class JSONListState : public JSONState { - ValueVector values = ValueVector(); - virtual unique_ptr resolve(EvalState & state) override + ValueVector values; + std::unique_ptr resolve(EvalState & state) override { - Value& v = parent->value(state); + Value & v = parent->value(state); state.mkList(v, values.size()); for (size_t n = 0; n < values.size(); ++n) { v.listElems()[n] = values[n]; } return std::move(parent); } - virtual void add() override { - values.push_back(v); + void add() override { + values.push_back(*v); v = nullptr; - }; + } public: - JSONListState(unique_ptr&& p, std::size_t reserve) : JSONState(std::move(p)) + JSONListState(std::unique_ptr && p, std::size_t reserve) : JSONState(std::move(p)) { values.reserve(reserve); } }; EvalState & state; - unique_ptr rs; + std::unique_ptr rs; template inline bool handle_value(T f, Args... args) { @@ -107,12 +106,12 @@ public: return handle_value(mkInt, val); } - bool number_float(number_float_t val, const string_t& s) + bool number_float(number_float_t val, const string_t & s) { return handle_value(mkFloat, val); } - bool string(string_t& val) + bool string(string_t & val) { return handle_value(mkString, val.c_str()); } @@ -123,7 +122,7 @@ public: return true; } - bool key(string_t& name) + bool key(string_t & name) { dynamic_cast(rs.get())->key(name, state); return true; From fcd048a526bd239fa615457e77d61d69d679bf03 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Apr 2020 16:54:34 +0200 Subject: [PATCH 5/7] Use RootValue --- src/libexpr/primops.cc | 10 +++++----- src/nix/command.hh | 2 +- src/nix/installables.cc | 21 ++++++++------------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index fc6c8296b..a3f2b92ce 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -121,16 +121,16 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args } w.attrs->sort(); - static Value * fun = nullptr; + static RootValue fun; if (!fun) { - fun = state.allocValue(); + fun = allocRootValue(state.allocValue()); state.eval(state.parseExprFromString( #include "imported-drv-to-derivation.nix.gen.hh" - , "/"), *fun); + , "/"), **fun); } - state.forceFunction(*fun, pos); - mkApp(v, *fun, w); + state.forceFunction(**fun, pos); + mkApp(v, **fun, w); state.forceAttrs(v, pos); } else { state.forceAttrs(*args[0]); diff --git a/src/nix/command.hh b/src/nix/command.hh index 2c2303208..bf43d950f 100644 --- a/src/nix/command.hh +++ b/src/nix/command.hh @@ -41,7 +41,7 @@ private: std::shared_ptr evalState; - std::shared_ptr vSourceExpr; + RootValue vSourceExpr; }; enum RealiseMode { Build, NoBuild, DryRun }; diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 902383bff..1d70ad3d5 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -8,8 +8,6 @@ #include "store-api.hh" #include "shared.hh" -#include - #include namespace nix { @@ -27,17 +25,14 @@ SourceExprCommand::SourceExprCommand() Value * SourceExprCommand::getSourceExpr(EvalState & state) { - if (vSourceExpr) return vSourceExpr.get(); + if (vSourceExpr) return *vSourceExpr; auto sToplevel = state.symbols.create("_toplevel"); - // Allocate the vSourceExpr Value as uncollectable. Boehm GC doesn't - // consider the member variable "alive" during execution causing it to be - // GC'ed in the middle of evaluation. - vSourceExpr = std::allocate_shared(traceable_allocator()); + vSourceExpr = allocRootValue(state.allocValue()); if (file != "") - state.evalFile(lookupFileArg(state, file), *vSourceExpr); + state.evalFile(lookupFileArg(state, file), **vSourceExpr); else { @@ -45,9 +40,9 @@ Value * SourceExprCommand::getSourceExpr(EvalState & state) auto searchPath = state.getSearchPath(); - state.mkAttrs(*vSourceExpr, 1024); + state.mkAttrs(**vSourceExpr, 1024); - mkBool(*state.allocAttr(*vSourceExpr, sToplevel), true); + mkBool(*state.allocAttr(**vSourceExpr, sToplevel), true); std::unordered_set seen; @@ -58,7 +53,7 @@ Value * SourceExprCommand::getSourceExpr(EvalState & state) mkPrimOpApp(*v1, state.getBuiltin("findFile"), state.getBuiltin("nixPath")); Value * v2 = state.allocValue(); mkApp(*v2, *v1, mkString(*state.allocValue(), name)); - mkApp(*state.allocAttr(*vSourceExpr, state.symbols.create(name)), + mkApp(*state.allocAttr(**vSourceExpr, state.symbols.create(name)), state.getBuiltin("import"), *v2); }; @@ -72,10 +67,10 @@ Value * SourceExprCommand::getSourceExpr(EvalState & state) } else addEntry(i.first); - vSourceExpr->attrs->sort(); + (*vSourceExpr)->attrs->sort(); } - return vSourceExpr.get(); + return *vSourceExpr; } ref SourceExprCommand::getEvalState() From 67a5941472ab56a798556ee2d5afc62fc38a799a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Apr 2020 13:12:58 +0200 Subject: [PATCH 6/7] Logger: Add method for writing to stdout Usually this just writes to stdout, but for ProgressBar, we need to clear the current line, write the line to stdout, and then redraw the progress bar. (cherry picked from commit 696c026006a6ac46adc990ed5cb0f31535bac076) --- src/libutil/logging.cc | 6 ++++++ src/libutil/logging.hh | 10 ++++++++++ src/nix/progress-bar.cc | 13 +++++++++++++ 3 files changed, 29 insertions(+) diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index bb437cf1c..3cc4ef8f1 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -3,6 +3,7 @@ #include #include +#include namespace nix { @@ -24,6 +25,11 @@ void Logger::warn(const std::string & msg) log(lvlWarn, ANSI_YELLOW "warning:" ANSI_NORMAL " " + msg); } +void Logger::writeToStdout(std::string_view s) +{ + std::cout << s << "\n"; +} + class SimpleLogger : public Logger { public: diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 108b5dcb0..18c24d508 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -78,6 +78,16 @@ public: virtual void stopActivity(ActivityId act) { }; virtual void result(ActivityId act, ResultType type, const Fields & fields) { }; + + virtual void writeToStdout(std::string_view s); + + template + inline void stdout(const std::string & fs, const Args & ... args) + { + boost::format f(fs); + formatHelper(f, args...); + writeToStdout(f.str()); + } }; ActivityId getCurActivity(); diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc index adc9b9a5d..8e7ba95a3 100644 --- a/src/nix/progress-bar.cc +++ b/src/nix/progress-bar.cc @@ -7,6 +7,7 @@ #include #include #include +#include namespace nix { @@ -442,6 +443,18 @@ public: return res; } + + void writeToStdout(std::string_view s) override + { + auto state(state_.lock()); + if (state->active) { + std::cerr << "\r\e[K"; + Logger::writeToStdout(s); + draw(*state); + } else { + Logger::writeToStdout(s); + } + } }; void startProgressBar(bool printBuildLogs) From efaffaa9d1de38efecb718aa7a99ba1f2e342ade Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Apr 2020 13:46:37 +0200 Subject: [PATCH 7/7] Use Logger::stdout() (cherry picked from commit 8f41847394524fcac40d3b5620139ca7e94a18e3) --- src/nix/add-to-store.cc | 2 +- src/nix/eval.cc | 5 ++--- src/nix/hash.cc | 5 ++--- src/nix/ls.cc | 10 ++++------ src/nix/show-config.cc | 2 +- src/nix/why-depends.cc | 2 +- 6 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 139db3657..ed02227db 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -50,7 +50,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand if (!dryRun) store->addToStore(info, sink.s); - std::cout << fmt("%s\n", store->printStorePath(info.path)); + logger->stdout("%s", store->printStorePath(info.path)); } }; diff --git a/src/nix/eval.cc b/src/nix/eval.cc index 6398fc58e..86a1e8b68 100644 --- a/src/nix/eval.cc +++ b/src/nix/eval.cc @@ -55,16 +55,15 @@ struct CmdEval : MixJSON, InstallableCommand auto v = installable->toValue(*state).first; PathSet context; - stopProgressBar(); - if (raw) { + stopProgressBar(); std::cout << state->coerceToString(noPos, *v, context); } else if (json) { JSONPlaceholder jsonOut(std::cout); printValueAsJSON(*state, true, *v, jsonOut, context); } else { state->forceValueDeep(*v); - std::cout << *v << "\n"; + logger->stdout("%s", *v); } } }; diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 0cc523f50..01628cf6c 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -60,8 +60,7 @@ struct CmdHash : Command Hash h = hashSink->finish().first; if (truncate && h.hashSize > 20) h = compressHash(h, 20); - std::cout << format("%1%\n") % - h.to_string(base, base == SRI); + logger->stdout(h.to_string(base, base == SRI)); } } }; @@ -95,7 +94,7 @@ struct CmdToBase : Command void run() override { for (auto s : args) - std::cout << fmt("%s\n", Hash(s, ht).to_string(base, base == SRI)); + logger->stdout(Hash(s, ht).to_string(base, base == SRI)); } }; diff --git a/src/nix/ls.cc b/src/nix/ls.cc index 3ef1f2750..8590199d7 100644 --- a/src/nix/ls.cc +++ b/src/nix/ls.cc @@ -34,16 +34,14 @@ struct MixLs : virtual Args, MixJSON (st.isExecutable ? "-r-xr-xr-x" : "-r--r--r--") : st.type == FSAccessor::Type::tSymlink ? "lrwxrwxrwx" : "dr-xr-xr-x"; - std::cout << - (format("%s %20d %s") % tp % st.fileSize % relPath); + auto line = fmt("%s %20d %s", tp, st.fileSize, relPath); if (st.type == FSAccessor::Type::tSymlink) - std::cout << " -> " << accessor->readLink(curPath) - ; - std::cout << "\n"; + line += " -> " + accessor->readLink(curPath); + logger->stdout(line); if (recursive && st.type == FSAccessor::Type::tDirectory) doPath(st, curPath, relPath, false); } else { - std::cout << relPath << "\n"; + logger->stdout(relPath); if (recursive) { auto st = accessor->stat(curPath); if (st.type == FSAccessor::Type::tDirectory) diff --git a/src/nix/show-config.cc b/src/nix/show-config.cc index 87544f937..6104b10bc 100644 --- a/src/nix/show-config.cc +++ b/src/nix/show-config.cc @@ -23,7 +23,7 @@ struct CmdShowConfig : Command, MixJSON std::map settings; globalConfig.getSettings(settings); for (auto & s : settings) - std::cout << s.first + " = " + s.second.value + "\n"; + logger->stdout("%s = %s", s.first, s.second.value); } } }; diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index d3b7a674a..f9acc7f13 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -149,7 +149,7 @@ struct CmdWhyDepends : SourceExprCommand auto pathS = store->printStorePath(node.path); assert(node.dist != inf); - std::cout << fmt("%s%s%s%s" ANSI_NORMAL "\n", + logger->stdout("%s%s%s%s" ANSI_NORMAL, firstPad, node.visited ? "\e[38;5;244m" : "", firstPad != "" ? "→ " : "",