From f6fd01bd19e14b62a8326712928f77904d097f17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20M=C3=B6st?= Date: Thu, 20 Feb 2020 07:52:14 +0100 Subject: [PATCH 1/6] .dir-locals.el: Set additional lambda indentation to zero --- .dir-locals.el | 1 + 1 file changed, 1 insertion(+) diff --git a/.dir-locals.el b/.dir-locals.el index 2d1117f4b..8b21d4e40 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -13,4 +13,5 @@ (eval . (c-set-offset 'arglist-cont-nonempty '+)) (eval . (c-set-offset 'substatement-open 0)) (eval . (c-set-offset 'access-label '-)) + (eval . (c-set-offset 'inlambda 0)) ))) From 22a754c091f765061f59bef5ce091268493bb138 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 28 Feb 2020 18:07:10 +0100 Subject: [PATCH 2/6] Fix GC failures on bad store path names It failed on names like '/nix/store/9ip48nkc9rfy0a4yaw98lp6gipqlib1a-'. --- src/libstore/gc.cc | 40 +++++++++++++++++++-------------------- src/libstore/path.cc | 14 ++++++++++++++ src/libstore/store-api.cc | 8 -------- src/libstore/store-api.hh | 2 ++ 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 690febc5b..0c3d89611 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -255,12 +255,11 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor) void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) { auto foundRoot = [&](const Path & path, const Path & target) { - Path storePath = toStorePath(target); - // FIXME - if (isStorePath(storePath) && isValidPath(parseStorePath(storePath))) - roots[parseStorePath(storePath)].emplace(path); + auto storePath = maybeParseStorePath(toStorePath(target)); + if (storePath && isValidPath(*storePath)) + roots[std::move(*storePath)].emplace(path); else - printInfo("skipping invalid root from '%1%' to '%2%'", path, storePath); + printInfo("skipping invalid root from '%1%' to '%2%'", path, target); }; try { @@ -296,10 +295,9 @@ void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) } else if (type == DT_REG) { - Path storePath = storeDir + "/" + std::string(baseNameOf(path)); - // FIXME - if (isStorePath(storePath) && isValidPath(parseStorePath(storePath))) - roots[parseStorePath(storePath)].emplace(path); + auto storePath = maybeParseStorePath(storeDir + "/" + std::string(baseNameOf(path))); + if (storePath && isValidPath(*storePath)) + roots[std::move(*storePath)].emplace(path); } } @@ -523,14 +521,14 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path) unsigned long long size = 0; - // FIXME - if (isStorePath(path) && isValidPath(parseStorePath(path))) { + auto storePath = maybeParseStorePath(path); + if (storePath && isValidPath(*storePath)) { StorePathSet referrers; - queryReferrers(parseStorePath(path), referrers); + queryReferrers(*storePath, referrers); for (auto & i : referrers) if (printStorePath(i) != path) deletePathRecursive(state, printStorePath(i)); - size = queryPathInfo(parseStorePath(path))->narSize; - invalidatePathChecked(parseStorePath(path)); + size = queryPathInfo(*storePath)->narSize; + invalidatePathChecked(*storePath); } Path realPath = realStoreDir + "/" + std::string(baseNameOf(path)); @@ -593,8 +591,7 @@ bool LocalStore::canReachRoot(GCState & state, StorePathSet & visited, const Sto visited.insert(path.clone()); - //FIXME - if (!isStorePath(printStorePath(path)) || !isValidPath(path)) return false; + if (!isValidPath(path)) return false; StorePathSet incoming; @@ -637,8 +634,9 @@ void LocalStore::tryToDelete(GCState & state, const Path & path) //Activity act(*logger, lvlDebug, format("considering whether to delete '%1%'") % path); - // FIXME - if (!isStorePath(path) || !isValidPath(parseStorePath(path))) { + auto storePath = maybeParseStorePath(path); + + if (!storePath || !isValidPath(*storePath)) { /* A lock file belonging to a path that we're building right now isn't garbage. */ if (isActiveTempFile(state, path, ".lock")) return; @@ -655,7 +653,7 @@ void LocalStore::tryToDelete(GCState & state, const Path & path) StorePathSet visited; - if (canReachRoot(state, visited, parseStorePath(path))) { + if (storePath && canReachRoot(state, visited, *storePath)) { debug("cannot delete '%s' because it's still reachable", path); } else { /* No path we visited was a root, so everything is garbage. @@ -818,8 +816,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) string name = dirent->d_name; if (name == "." || name == "..") continue; Path path = storeDir + "/" + name; - // FIXME - if (isStorePath(path) && isValidPath(parseStorePath(path))) + auto storePath = maybeParseStorePath(path); + if (storePath && isValidPath(*storePath)) entries.push_back(path); else tryToDelete(state, path); diff --git a/src/libstore/path.cc b/src/libstore/path.cc index a33bec3ed..70b919adc 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -55,6 +55,20 @@ StorePath Store::parseStorePath(std::string_view path) const return StorePath::make(path, storeDir); } +std::optional Store::maybeParseStorePath(std::string_view path) const +{ + try { + return parseStorePath(path); + } catch (Error &) { + return {}; + } +} + +bool Store::isStorePath(const Path & path) const +{ + return (bool) maybeParseStorePath(path); +} + StorePathSet Store::parseStorePathSet(const PathSet & paths) const { StorePathSet res; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index d8f6c22bc..75fa5d1e6 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -19,14 +19,6 @@ bool Store::isInStore(const Path & path) const } -bool Store::isStorePath(const Path & path) const -{ - return isInStore(path) - && path.size() >= storeDir.size() + 1 + storePathHashLen - && path.find('/', storeDir.size() + 1) == Path::npos; -} - - Path Store::toStorePath(const Path & path) const { if (!isInStore(path)) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 861b96930..a1cfb314a 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -281,6 +281,8 @@ public: StorePath parseStorePath(std::string_view path) const; + std::optional maybeParseStorePath(std::string_view path) const; + std::string printStorePath(const StorePath & path) const; // FIXME: remove From d700eecea9a274c1b45549141f40180ac74454ce Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 2 Mar 2020 17:27:48 +0100 Subject: [PATCH 3/6] Add test for foldl' --- tests/lang/eval-okay-foldlStrict.exp | 1 + tests/lang/eval-okay-foldlStrict.nix | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 tests/lang/eval-okay-foldlStrict.exp create mode 100644 tests/lang/eval-okay-foldlStrict.nix diff --git a/tests/lang/eval-okay-foldlStrict.exp b/tests/lang/eval-okay-foldlStrict.exp new file mode 100644 index 000000000..837e12b40 --- /dev/null +++ b/tests/lang/eval-okay-foldlStrict.exp @@ -0,0 +1 @@ +500500 diff --git a/tests/lang/eval-okay-foldlStrict.nix b/tests/lang/eval-okay-foldlStrict.nix new file mode 100644 index 000000000..3b87188d2 --- /dev/null +++ b/tests/lang/eval-okay-foldlStrict.nix @@ -0,0 +1,3 @@ +with import ./lib.nix; + +builtins.foldl' (x: y: x + y) 0 (range 1 1000) From 401b5bc5418f3eb6d57da9d9e66df055f8bce122 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 21 Feb 2020 19:25:49 +0100 Subject: [PATCH 4/6] builtins.cache: Cache regular expressions The evaluator was spending about 1% of its time compiling a small number of regexes over and over again. --- src/libexpr/eval.hh | 4 ++++ src/libexpr/primops.cc | 8 +++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index cabc92d15..34a212aa4 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -114,6 +114,9 @@ private: /* Cache used by checkSourcePath(). */ std::unordered_map resolvedPaths; + /* Cache used by prim_match(). */ + std::unordered_map regexCache; + public: EvalState(const Strings & _searchPath, ref store); @@ -314,6 +317,7 @@ private: friend struct ExprOpConcatLists; friend struct ExprSelect; friend void prim_getAttr(EvalState & state, const Pos & pos, Value * * args, Value & v); + friend void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v); }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 29302c9b6..4cd28698c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1811,19 +1811,21 @@ static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, /* Match a regular expression against a string and return either ‘null’ or a list containing substring matches. */ -static void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) +void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) { auto re = state.forceStringNoCtx(*args[0], pos); try { - std::regex regex(re, std::regex::extended); + auto regex = state.regexCache.find(re); + if (regex == state.regexCache.end()) + regex = state.regexCache.emplace(re, std::regex(re, std::regex::extended)).first; PathSet context; const std::string str = state.forceString(*args[1], context, pos); std::smatch match; - if (!std::regex_match(str, match, regex)) { + if (!std::regex_match(str, match, regex->second)) { mkNull(v); return; } From 75db069f927ffaf38ac6ef2d8143926b724ca935 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 23 Feb 2020 16:36:19 +0100 Subject: [PATCH 5/6] Optimise Derivation::unparse() In nix-instantiate --dry-run '' -A nixos.tests.simple.x86_64-linux this reduces time spent in unparse() from 9.15% to 4.31%. The main culprit was appending characters one at a time to the destination string. Even though the string has enough capacity, push_back() still needs to check this on every call. --- src/libstore/derivations.cc | 58 ++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index d9da8769c..205b90e55 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -213,15 +213,26 @@ Derivation Store::derivationFromPath(const StorePath & drvPath) } -static void printString(string & res, const string & s) +static void printString(string & res, std::string_view s) +{ + char buf[s.size() * 2 + 2]; + char * p = buf; + *p++ = '"'; + for (auto c : s) + if (c == '\"' || c == '\\') { *p++ = '\\'; *p++ = c; } + else if (c == '\n') { *p++ = '\\'; *p++ = 'n'; } + else if (c == '\r') { *p++ = '\\'; *p++ = 'r'; } + else if (c == '\t') { *p++ = '\\'; *p++ = 't'; } + else *p++ = c; + *p++ = '"'; + res.append(buf, p - buf); +} + + +static void printUnquotedString(string & res, std::string_view s) { res += '"'; - for (const char * i = s.c_str(); *i; i++) - if (*i == '\"' || *i == '\\') { res += "\\"; res += *i; } - else if (*i == '\n') res += "\\n"; - else if (*i == '\r') res += "\\r"; - else if (*i == '\t') res += "\\t"; - else res += *i; + res.append(s); res += '"'; } @@ -239,6 +250,19 @@ static void printStrings(string & res, ForwardIterator i, ForwardIterator j) } +template +static void printUnquotedStrings(string & res, ForwardIterator i, ForwardIterator j) +{ + res += '['; + bool first = true; + for ( ; i != j; ++i) { + if (first) first = false; else res += ','; + printUnquotedString(res, *i); + } + res += ']'; +} + + string Derivation::unparse(const Store & store, bool maskOutputs, std::map * actualInputs) const { @@ -249,10 +273,10 @@ string Derivation::unparse(const Store & store, bool maskOutputs, bool first = true; for (auto & i : outputs) { if (first) first = false; else s += ','; - s += '('; printString(s, i.first); - s += ','; printString(s, maskOutputs ? "" : store.printStorePath(i.second.path)); - s += ','; printString(s, i.second.hashAlgo); - s += ','; printString(s, i.second.hash); + s += '('; printUnquotedString(s, i.first); + s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path)); + s += ','; printUnquotedString(s, i.second.hashAlgo); + s += ','; printUnquotedString(s, i.second.hash); s += ')'; } @@ -261,24 +285,24 @@ string Derivation::unparse(const Store & store, bool maskOutputs, if (actualInputs) { for (auto & i : *actualInputs) { if (first) first = false; else s += ','; - s += '('; printString(s, i.first); - s += ','; printStrings(s, i.second.begin(), i.second.end()); + s += '('; printUnquotedString(s, i.first); + s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end()); s += ')'; } } else { for (auto & i : inputDrvs) { if (first) first = false; else s += ','; - s += '('; printString(s, store.printStorePath(i.first)); - s += ','; printStrings(s, i.second.begin(), i.second.end()); + s += '('; printUnquotedString(s, store.printStorePath(i.first)); + s += ','; printUnquotedStrings(s, i.second.begin(), i.second.end()); s += ')'; } } s += "],"; auto paths = store.printStorePathSet(inputSrcs); // FIXME: slow - printStrings(s, paths.begin(), paths.end()); + printUnquotedStrings(s, paths.begin(), paths.end()); - s += ','; printString(s, platform); + s += ','; printUnquotedString(s, platform); s += ','; printString(s, builder); s += ','; printStrings(s, args.begin(), args.end()); From d37dc71e3cf077fa5d24a9bf8395deae21cc4410 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 4 Mar 2020 13:55:15 +0100 Subject: [PATCH 6/6] nix-build: Fix ! handling This was broken by 22a754c091f765061f59bef5ce091268493bb138. https://hydra.nixos.org/eval/1573669 --- src/libstore/path.cc | 2 +- src/libstore/store-api.hh | 2 +- src/nix-build/nix-build.cc | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libstore/path.cc b/src/libstore/path.cc index 70b919adc..9a28aa96a 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -64,7 +64,7 @@ std::optional Store::maybeParseStorePath(std::string_view path) const } } -bool Store::isStorePath(const Path & path) const +bool Store::isStorePath(std::string_view path) const { return (bool) maybeParseStorePath(path); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a1cfb314a..a000757fb 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -305,7 +305,7 @@ public: /* Return true if ‘path’ is a store path, i.e. a direct child of the Nix store. */ - bool isStorePath(const Path & path) const; + bool isStorePath(std::string_view path) const; /* Chop off the parts after the top-level store name, e.g., /nix/store/abcd-foo/bar => /nix/store/abcd-foo. */ diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index ff95ad787..27ec7d0fe 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -295,7 +295,8 @@ static void _main(int argc, char * * argv) try { absolute = canonPath(absPath(i), true); } catch (Error & e) {}; - if (store->isStorePath(absolute) && std::regex_match(absolute, std::regex(".*\\.drv(!.*)?"))) + auto [path, outputNames] = parsePathWithOutputs(absolute); + if (store->isStorePath(path) && hasSuffix(path, ".drv")) drvs.push_back(DrvInfo(*state, store, absolute)); else /* If we're in a #! script, interpret filenames