From 1ddabe1a0120787ff5bbdba5383222a6eb59c219 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 20 Apr 2022 16:39:47 +0200 Subject: [PATCH 01/21] nix: Respect meta.outputsToInstall, and use all outputs by default 'nix profile install' will now install all outputs listed in the package's meta.outputsToInstall attribute, or all outputs if that attribute doesn't exist. This makes it behave consistently with nix-env. Fixes #6385. Furthermore, for consistency, all other 'nix' commands do this as well. E.g. 'nix build' will build and symlink the outputs in meta.outputsToInstall, defaulting to all outputs. Previously, it only built/symlinked the first output. Note that this means that selecting a specific output using attrpath selection (e.g. 'nix build nixpkgs#libxml2.dev') no longer works. A subsequent PR will add a way to specify the desired outputs explicitly. --- src/libcmd/installables.cc | 33 ++++++++++++++++----- src/libcmd/installables.hh | 2 +- src/libexpr/eval-cache.cc | 54 +++++++++++++++++++++++++++++++++- src/libexpr/eval-cache.hh | 6 +++- src/nix/app.cc | 2 +- src/nix/flake.cc | 4 +-- src/nix/search.cc | 6 ++-- tests/build.sh | 11 ++----- tests/ca/content-addressed.nix | 2 +- tests/ca/substitute.sh | 3 +- 10 files changed, 96 insertions(+), 27 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index e3210d18d..e2ee47dea 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -440,10 +440,8 @@ DerivedPaths InstallableValue::toDerivedPaths() // Group by derivation, helps with .all in particular for (auto & drv : toDerivations()) { - auto outputName = drv.outputName; - if (outputName == "") - throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(drv.drvPath)); - drvsToOutputs[drv.drvPath].insert(outputName); + for (auto & outputName : drv.outputsToInstall) + drvsToOutputs[drv.drvPath].insert(outputName); drvsToCopy.insert(drv.drvPath); } @@ -497,7 +495,13 @@ std::vector InstallableAttrPath::toDerivations auto drvPath = drvInfo.queryDrvPath(); if (!drvPath) throw Error("'%s' is not a derivation", what()); - res.push_back({ *drvPath, drvInfo.queryOutputName() }); + std::set outputsToInstall; + for (auto & output : drvInfo.queryOutputs(false, true)) + outputsToInstall.insert(output.first); + res.push_back(DerivationInfo { + .drvPath = *drvPath, + .outputsToInstall = std::move(outputsToInstall) + }); } return res; @@ -598,9 +602,24 @@ std::tuple InstallableF auto drvPath = attr->forceDerivation(); + std::set outputsToInstall; + + if (auto aMeta = attr->maybeGetAttr(state->sMeta)) + if (auto aOutputsToInstall = aMeta->maybeGetAttr("outputsToInstall")) + for (auto & s : aOutputsToInstall->getListOfStrings()) + outputsToInstall.insert(s); + + if (outputsToInstall.empty()) + if (auto aOutputs = attr->maybeGetAttr(state->sOutputs)) + for (auto & s : aOutputs->getListOfStrings()) + outputsToInstall.insert(s); + + if (outputsToInstall.empty()) + outputsToInstall.insert("out"); + auto drvInfo = DerivationInfo { - std::move(drvPath), - attr->getAttr(state->sOutputName)->getString() + .drvPath = std::move(drvPath), + .outputsToInstall = std::move(outputsToInstall), }; return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo)}; diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index de8b08525..3c2c33549 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -141,7 +141,7 @@ struct InstallableValue : Installable struct DerivationInfo { StorePath drvPath; - std::string outputName; + std::set outputsToInstall; }; virtual std::vector toDerivations() = 0; diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 6fb4bf266..3a92f2815 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -175,6 +175,24 @@ struct AttrDb }); } + AttrId setListOfStrings( + AttrKey key, + const std::vector & l) + { + return doSQLite([&]() + { + auto state(_state->lock()); + + state->insertAttribute.use() + (key.first) + (symbols[key.second]) + (AttrType::ListOfStrings) + (concatStringsSep("\t", l)).exec(); + + return state->db.getLastInsertedRowId(); + }); + } + AttrId setPlaceholder(AttrKey key) { return doSQLite([&]() @@ -269,6 +287,8 @@ struct AttrDb } case AttrType::Bool: return {{rowId, queryAttribute.getInt(2) != 0}}; + case AttrType::ListOfStrings: + return {{rowId, tokenizeString>(queryAttribute.getStr(2), "\t")}}; case AttrType::Missing: return {{rowId, missing_t()}}; case AttrType::Misc: @@ -385,7 +405,7 @@ std::string AttrCursor::getAttrPathStr(Symbol name) const Value & AttrCursor::forceValue() { - debug("evaluating uncached attribute %s", getAttrPathStr()); + debug("evaluating uncached attribute '%s'", getAttrPathStr()); auto & v = getValue(); @@ -601,6 +621,38 @@ bool AttrCursor::getBool() return v.boolean; } +std::vector AttrCursor::getListOfStrings() +{ + if (root->db) { + if (!cachedValue) + cachedValue = root->db->getAttr(getKey()); + if (cachedValue && !std::get_if(&cachedValue->second)) { + if (auto l = std::get_if>(&cachedValue->second)) { + debug("using cached list of strings attribute '%s'", getAttrPathStr()); + return *l; + } else + throw TypeError("'%s' is not a list of strings", getAttrPathStr()); + } + } + + debug("evaluating uncached attribute '%s'", getAttrPathStr()); + + auto & v = getValue(); + root->state.forceValue(v, noPos); + + if (v.type() != nList) + throw TypeError("'%s' is not a list", getAttrPathStr()); + + std::vector res; + + for (auto & elem : v.listItems()) + res.push_back(std::string(root->state.forceStringNoCtx(*elem))); + + cachedValue = {root->db->setListOfStrings(getKey(), res), res}; + + return res; +} + std::vector AttrCursor::getAttrs() { if (root->db) { diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index b0709ebc2..636e293ad 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -44,6 +44,7 @@ enum AttrType { Misc = 4, Failed = 5, Bool = 6, + ListOfStrings = 7, }; struct placeholder_t {}; @@ -61,7 +62,8 @@ typedef std::variant< missing_t, misc_t, failed_t, - bool + bool, + std::vector > AttrValue; class AttrCursor : public std::enable_shared_from_this @@ -114,6 +116,8 @@ public: bool getBool(); + std::vector getListOfStrings(); + std::vector getAttrs(); bool isDerivation(); diff --git a/src/nix/app.cc b/src/nix/app.cc index cce84d026..821964f86 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -89,7 +89,7 @@ UnresolvedApp Installable::toApp(EvalState & state) auto outputName = cursor->getAttr(state.sOutputName)->getString(); auto name = cursor->getAttr(state.sName)->getString(); auto aPname = cursor->maybeGetAttr("pname"); - auto aMeta = cursor->maybeGetAttr("meta"); + auto aMeta = cursor->maybeGetAttr(state.sMeta); auto aMainProgram = aMeta ? aMeta->maybeGetAttr("mainProgram") : nullptr; auto mainProgram = aMainProgram diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 040c1c7af..6a34ca67b 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1015,8 +1015,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON auto name = visitor.getAttr(state->sName)->getString(); if (json) { std::optional description; - if (auto aMeta = visitor.maybeGetAttr("meta")) { - if (auto aDescription = aMeta->maybeGetAttr("description")) + if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) { + if (auto aDescription = aMeta->maybeGetAttr(state->sDescription)) description = aDescription->getString(); } j.emplace("type", "derivation"); diff --git a/src/nix/search.cc b/src/nix/search.cc index 76451f810..87dc1c0de 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -93,10 +93,10 @@ struct CmdSearch : InstallableCommand, MixJSON }; if (cursor.isDerivation()) { - DrvName name(cursor.getAttr("name")->getString()); + DrvName name(cursor.getAttr(state->sName)->getString()); - auto aMeta = cursor.maybeGetAttr("meta"); - auto aDescription = aMeta ? aMeta->maybeGetAttr("description") : nullptr; + auto aMeta = cursor.maybeGetAttr(state->sMeta); + auto aDescription = aMeta ? aMeta->maybeGetAttr(state->sDescription) : nullptr; auto description = aDescription ? aDescription->getString() : ""; std::replace(description.begin(), description.end(), '\n', ' '); auto attrPath2 = concatStringsSep(".", attrPathS); diff --git a/tests/build.sh b/tests/build.sh index 13a0f42be..339155991 100644 --- a/tests/build.sh +++ b/tests/build.sh @@ -2,15 +2,8 @@ source common.sh clearStore -# Make sure that 'nix build' only returns the outputs we asked for. -nix build -f multiple-outputs.nix --json a --no-link | jq --exit-status ' - (.[0] | - (.drvPath | match(".*multiple-outputs-a.drv")) and - (.outputs | keys | length == 1) and - (.outputs.first | match(".*multiple-outputs-a-first"))) -' - -nix build -f multiple-outputs.nix --json a.all b.all --no-link | jq --exit-status ' +# Make sure that 'nix build' returns all outputs by default. +nix build -f multiple-outputs.nix --json a b --no-link | jq --exit-status ' (.[0] | (.drvPath | match(".*multiple-outputs-a.drv")) and (.outputs | keys | length == 2) and diff --git a/tests/ca/content-addressed.nix b/tests/ca/content-addressed.nix index 1be3eeb6e..31c144ae0 100644 --- a/tests/ca/content-addressed.nix +++ b/tests/ca/content-addressed.nix @@ -23,7 +23,7 @@ rec { }; rootCA = mkCADerivation { name = "rootCA"; - outputs = [ "out" "dev" "foo"]; + outputs = [ "out" "dev" "foo" ]; buildCommand = '' echo "building a CA derivation" echo "The seed is ${toString seed}" diff --git a/tests/ca/substitute.sh b/tests/ca/substitute.sh index 3d9001bb8..819f3fd85 100644 --- a/tests/ca/substitute.sh +++ b/tests/ca/substitute.sh @@ -25,7 +25,8 @@ buildDrvs --substitute --substituters $REMOTE_STORE --no-require-sigs -j0 transi # Check that the thing we’ve just substituted has its realisation stored nix realisation info --file ./content-addressed.nix transitivelyDependentCA # Check that its dependencies have it too -nix realisation info --file ./content-addressed.nix dependentCA rootCA +nix realisation info --file ./content-addressed.nix dependentCA +# nix realisation info --file ./content-addressed.nix rootCA --outputs out # Same thing, but # 1. With non-ca derivations From 6e0a2b971bfe75a581a91e5092553f2ecc9c0b6f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 20 Apr 2022 16:50:24 +0200 Subject: [PATCH 02/21] Add a test for outputsToInstall --- tests/nix-profile.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh index fad62b993..814192252 100644 --- a/tests/nix-profile.sh +++ b/tests/nix-profile.sh @@ -17,6 +17,7 @@ cat > $flake1Dir/flake.nix < $flake1Dir/flake.nix < 1.0" nix profile diff-closures | grep 'env-manifest.nix: ε → ∅' @@ -55,7 +61,7 @@ printf NixOS > $flake1Dir/who printf 2.0 > $flake1Dir/version nix profile upgrade 1 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello NixOS" ]] -nix profile history | grep "packages.$system.default: 1.0 -> 2.0" +nix profile history | grep "packages.$system.default: 1.0, 1.0-man -> 2.0, 2.0-man" # Test 'history', 'diff-closures'. nix profile diff-closures @@ -86,7 +92,7 @@ nix profile wipe-history printf true > $flake1Dir/ca.nix printf 3.0 > $flake1Dir/version nix profile upgrade 0 -nix profile history | grep "packages.$system.default: 1.0 -> 3.0" +nix profile history | grep "packages.$system.default: 1.0, 1.0-man -> 3.0, 3.0-man" # Test new install of CA package. nix profile remove 0 From 13d8400ac511dd36dcfd74c35737d093cf52bba3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 20 Apr 2022 16:51:12 +0200 Subject: [PATCH 03/21] Remove obsolete FIXME --- src/nix/profile.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index b151e48d6..52c918016 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -67,7 +67,6 @@ struct ProfileElement ref store, const BuiltPaths & builtPaths) { - // FIXME: respect meta.outputsToInstall storePaths.clear(); for (auto & buildable : builtPaths) { std::visit(overloaded { From 717298c7491af6ee0e2544258bc7267c8328be58 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 26 Apr 2022 17:17:51 +0200 Subject: [PATCH 04/21] Bump eval cache schema version --- src/libexpr/eval-cache.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 3a92f2815..a1cb162ee 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -47,7 +47,7 @@ struct AttrDb { auto state(_state->lock()); - Path cacheDir = getCacheDir() + "/nix/eval-cache-v2"; + Path cacheDir = getCacheDir() + "/nix/eval-cache-v3"; createDirs(cacheDir); Path dbPath = cacheDir + "/" + fingerprint.to_string(Base16, false) + ".sqlite"; From 143b73f52dabb35cd56551c24caef95466bce488 Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Fri, 29 Apr 2022 11:14:08 +0200 Subject: [PATCH 05/21] tests: Distinguish crashes from expected failures --- tests/lang.sh | 58 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/tests/lang.sh b/tests/lang.sh index 61bb444ba..6cb4b6e2b 100644 --- a/tests/lang.sh +++ b/tests/lang.sh @@ -10,32 +10,49 @@ nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" (throw set +x fail=0 +just_failed=0 + +safe_expect () { + expected=$1 name=$2 msg=$3 cmd=$4 + just_failed=0 + res=0 + eval "$cmd" || res=$? + + if [ $res -ge 2 ]; then + [ $res -le 128 ] || { echo "FAIL: $name returned unexpected code '$res' (should be 0 or 1)"; just_failed=1; } + [ $res -gt 128 ] || { echo "FAIL: $name crashed with $res ($(kill -l $(($res - 128))))"; just_failed=1; } + elif [ $expected = "fail" ]; then + [ $res -ne 0 ] || { echo "FAIL: $name $msg"; just_failed=1; } + elif [ $expected = "pass" ]; then + [ $res -eq 0 ] || { echo "FAIL: $name $msg"; just_failed=1; } + else + echo "usage: safe_expect [pass|fail] name msg command" + exit 1 + fi + + [ $just_failed -eq 0 ] || fail=1 +} + for i in lang/parse-fail-*.nix; do echo "parsing $i (should fail)"; i=$(basename $i .nix) - if nix-instantiate --parse - < lang/$i.nix; then - echo "FAIL: $i shouldn't parse" - fail=1 - fi + safe_expect fail "$i" "shouldn't parse" \ + "nix-instantiate --parse - < lang/$i.nix" done for i in lang/parse-okay-*.nix; do echo "parsing $i (should succeed)"; i=$(basename $i .nix) - if ! nix-instantiate --parse - < lang/$i.nix > lang/$i.out; then - echo "FAIL: $i should parse" - fail=1 - fi + safe_expect pass "$i" "should parse" \ + "nix-instantiate --parse - < lang/$i.nix > lang/$i.out" done for i in lang/eval-fail-*.nix; do echo "evaluating $i (should fail)"; i=$(basename $i .nix) - if nix-instantiate --eval lang/$i.nix; then - echo "FAIL: $i shouldn't evaluate" - fail=1 - fi + safe_expect fail "$i" "shouldn't evaluate" \ + "nix-instantiate --eval lang/$i.nix" done for i in lang/eval-okay-*.nix; do @@ -45,23 +62,20 @@ for i in lang/eval-okay-*.nix; do if test -e lang/$i.exp; then flags= if test -e lang/$i.flags; then - flags=$(cat lang/$i.flags) + flags='$'"(cat lang/$i.flags)" # because vi does not highlight "\$(...)" properly ;-). fi - if ! NIX_PATH=lang/dir3:lang/dir4 nix-instantiate $flags --eval --strict lang/$i.nix > lang/$i.out; then - echo "FAIL: $i should evaluate" - fail=1 - elif ! diff lang/$i.out lang/$i.exp; then + safe_expect pass "$i" "should evaluate" \ + "NIX_PATH=lang/dir3:lang/dir4 nix-instantiate $flags --eval --strict lang/$i.nix > lang/$i.out" + if [ $just_failed -eq 0 ] && ! diff lang/$i.out lang/$i.exp; then echo "FAIL: evaluation result of $i not as expected" fail=1 fi fi if test -e lang/$i.exp.xml; then - if ! nix-instantiate --eval --xml --no-location --strict \ - lang/$i.nix > lang/$i.out.xml; then - echo "FAIL: $i should evaluate" - fail=1 - elif ! cmp -s lang/$i.out.xml lang/$i.exp.xml; then + safe_expect pass "$i" "should evaluate" \ + "nix-instantiate --eval --xml --no-location --strict lang/$i.nix > lang/$i.out.xml" + if [ $just_failed -eq 0 ] && ! cmp -s lang/$i.out.xml lang/$i.exp.xml; then echo "FAIL: XML evaluation result of $i not as expected" fail=1 fi From 401e60f2893d689772f716cbd800ee2ee0309362 Mon Sep 17 00:00:00 2001 From: Kjetil Orbekk Date: Tue, 2 Mar 2021 20:13:20 -0500 Subject: [PATCH 06/21] Resolve reference for remote repository Resolves the HEAD reference from the remote repository instead of assuming "master". --- src/libfetchers/git.cc | 104 ++++++++++++++++++++++++++++++++++++++--- tests/flakes.sh | 9 +++- 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index af40990e5..6b83638f6 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -5,9 +5,11 @@ #include "store-api.hh" #include "url-parts.hh" #include "pathlocks.hh" +#include "util.hh" #include "fetch-settings.hh" +#include #include #include @@ -19,7 +21,7 @@ namespace nix::fetchers { // The value itself does not matter, since we always fetch a specific revision or branch. // It is set with `-c init.defaultBranch=` instead of `--initial-branch=` to stay compatible with // old version of git, which will ignore unrecognized `-c` options. -const std::string gitInitialBranch = "__nix_dummy_branch"; +static const std::string gitInitialBranch = "__nix_dummy_branch"; static std::string getGitDir() { @@ -30,9 +32,92 @@ static std::string getGitDir() return *gitDir; } -static std::string readHead(const Path & path) +static bool isCacheFileWithinTtl(const time_t now, const struct stat& st) { + return st.st_mtime + settings.tarballTtl > now; +} + +static Path getCachePath(std::string key) { + return getCacheDir() + "/nix/gitv3/" + + hashString(htSHA256, key).to_string(Base32, false); +} + +// Returns the name of the HEAD branch. +// +// Returns the head branch name as reported by git ls-remote --symref, e.g., if +// ls-remote returns the output below, "main" is returned based on the ref line. +// +// ref: refs/heads/main HEAD +// ... +static std::optional readHead(const Path & path) { - return chomp(runProgram("git", true, { "-C", path, "--git-dir", ".git", "rev-parse", "--abbrev-ref", "HEAD" })); + auto [exit_code, output] = runProgram(RunOptions { + .program = "git", + .args = {"ls-remote", "--symref", path}, + }); + if (exit_code != 0) { + return std::nullopt; + } + + // Matches the common case when HEAD points to a branch, e.g.: + // "ref: refs/heads/main HEAD". + const static std::regex head_ref_regex("^ref:\\s*([^\\s]+)\\s*HEAD$"); + // Matches when HEAD points directly at a commit, e.g.: + // "71abcd... HEAD". + const static std::regex head_rev_regex("^([^\\s]+)\\s*HEAD$"); + + for (const auto & line : tokenizeString>(output, "\n")) { + std::smatch match; + if (std::regex_match(line, match, head_ref_regex)) { + debug("resolved HEAD ref '%s' for repo '%s'", match[1], path); + return match[1]; + } else if (std::regex_match(line, match, head_rev_regex)) { + debug("resolved HEAD ref '%s' for repo '%s'", match[1], path); + return match[1]; + } + } + + return std::nullopt; +} + +static std::optional readHeadCached(std::string actualUrl) +{ + // Create a cache path to store the branch of the HEAD ref. Append something + // in front of the URL to prevent collision with the repository itself. + Path cachePath = getCachePath("|" + actualUrl); + time_t now = time(0); + struct stat st; + std::optional cachedRef; + if (stat(cachePath.c_str(), &st) == 0 && + // The file may be empty, because writeFile() is not atomic. + st.st_size > 0) { + + // The cached ref is persisted unconditionally (see below). + cachedRef = readFile(cachePath); + if (isCacheFileWithinTtl(now, st)) { + debug("using cached HEAD ref '%s' for repo '%s'", *cachedRef, actualUrl); + return cachedRef; + } + } + + auto ref = readHead(actualUrl); + + if (ref) { + debug("storing cached HEAD ref '%s' for repo '%s' at '%s'", *ref, actualUrl, cachePath); + createDirs(dirOf(cachePath)); + writeFile(cachePath, *ref); + return ref; + } + + if (cachedRef) { + // If the cached git ref is expired in fetch() below, and the 'git fetch' + // fails, it falls back to continuing with the most recent version. + // This function must behave the same way, so we return the expired + // cached ref here. + warn("could not get HEAD ref for repository '%s'; using expired cached ref '%s'", actualUrl, *cachedRef); + return *cachedRef; + } + + return std::nullopt; } static bool isNotDotGitDirectory(const Path & path) @@ -331,7 +416,14 @@ struct GitInputScheme : InputScheme } } - if (!input.getRef()) input.attrs.insert_or_assign("ref", isLocal ? readHead(actualUrl) : "master"); + if (!input.getRef()) { + auto head = isLocal ? readHead(actualUrl) : readHeadCached(actualUrl); + if (!head) { + warn("could not read HEAD ref from repo at '%s', using 'master'", actualUrl); + head = "master"; + } + input.attrs.insert_or_assign("ref", *head); + } Attrs unlockedAttrs({ {"type", cacheType}, @@ -360,7 +452,7 @@ struct GitInputScheme : InputScheme } } - Path cacheDir = getCacheDir() + "/nix/gitv3/" + hashString(htSHA256, actualUrl).to_string(Base32, false); + Path cacheDir = getCachePath(actualUrl); repoDir = cacheDir; gitDir = "."; @@ -400,7 +492,7 @@ struct GitInputScheme : InputScheme git fetch to update the local ref to the remote ref. */ struct stat st; doFetch = stat(localRefFile.c_str(), &st) != 0 || - (uint64_t) st.st_mtime + settings.tarballTtl <= (uint64_t) now; + !isCacheFileWithinTtl(now, st); } } diff --git a/tests/flakes.sh b/tests/flakes.sh index 46e6a7982..24601784f 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -31,7 +31,14 @@ flakeFollowsE=$TEST_ROOT/follows/flakeA/flakeE for repo in $flake1Dir $flake2Dir $flake3Dir $flake7Dir $templatesDir $nonFlakeDir $flakeA $flakeB $flakeFollowsA; do rm -rf $repo $repo.tmp mkdir -p $repo - git -C $repo init + + # Give one repo a non-master initial branch. + extraArgs= + if [[ $repo == $flake2Dir ]]; then + extraArgs="--initial-branch=main" + fi + + git -C $repo init $extraArgs git -C $repo config user.email "foobar@example.com" git -C $repo config user.name "Foobar" done From 05a3fbac5a99e5738b951443afe17b680d5a7ead Mon Sep 17 00:00:00 2001 From: Kjetil Orbekk Date: Tue, 20 Apr 2021 07:52:50 -0400 Subject: [PATCH 07/21] Test fetchGit with non-'master' remote repo --- tests/fetchGit.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 9179e2071..166bccfc7 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -161,6 +161,14 @@ path4=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [[ $(cat $path4/hello) = dev ]] [[ $path3 = $path4 ]] +# Using remote path with branch other than 'master' should fetch the HEAD revision. +# (--tarball-ttl 0 to prevent using the cached repo above) +export _NIX_FORCE_HTTP=1 +path4=$(nix eval --tarball-ttl 0 --impure --raw --expr "(builtins.fetchGit $repo).outPath") +[[ $(cat $path4/hello) = dev ]] +[[ $path3 = $path4 ]] +unset _NIX_FORCE_HTTP + # Confirm same as 'dev' branch path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"dev\"; }).outPath") [[ $path3 = $path5 ]] From de54e1cd3fce6eb456048b6a346a0be2b88660ae Mon Sep 17 00:00:00 2001 From: Kjetil Orbekk Date: Sat, 29 Jan 2022 14:12:01 -0500 Subject: [PATCH 08/21] Refactor fetching of dirty workdir Extract the handling of a local dirty workdir to a helper function. --- src/libfetchers/git.cc | 224 ++++++++++++++++++++++------------------- 1 file changed, 123 insertions(+), 101 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 6b83638f6..00a7b85d8 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -16,14 +16,15 @@ using namespace std::string_literals; namespace nix::fetchers { +namespace { // Explicit initial branch of our bare repo to suppress warnings from new version of git. // The value itself does not matter, since we always fetch a specific revision or branch. // It is set with `-c init.defaultBranch=` instead of `--initial-branch=` to stay compatible with // old version of git, which will ignore unrecognized `-c` options. -static const std::string gitInitialBranch = "__nix_dummy_branch"; +const std::string gitInitialBranch = "__nix_dummy_branch"; -static std::string getGitDir() +std::string getGitDir() { auto gitDir = getEnv("GIT_DIR"); if (!gitDir) { @@ -32,11 +33,11 @@ static std::string getGitDir() return *gitDir; } -static bool isCacheFileWithinTtl(const time_t now, const struct stat& st) { +bool isCacheFileWithinTtl(const time_t now, const struct stat& st) { return st.st_mtime + settings.tarballTtl > now; } -static Path getCachePath(std::string key) { +Path getCachePath(std::string key) { return getCacheDir() + "/nix/gitv3/" + hashString(htSHA256, key).to_string(Base32, false); } @@ -48,7 +49,7 @@ static Path getCachePath(std::string key) { // // ref: refs/heads/main HEAD // ... -static std::optional readHead(const Path & path) +std::optional readHead(const Path & path) { auto [exit_code, output] = runProgram(RunOptions { .program = "git", @@ -79,7 +80,7 @@ static std::optional readHead(const Path & path) return std::nullopt; } -static std::optional readHeadCached(std::string actualUrl) +std::optional readHeadCached(std::string actualUrl) { // Create a cache path to store the branch of the HEAD ref. Append something // in front of the URL to prevent collision with the repository itself. @@ -120,11 +121,120 @@ static std::optional readHeadCached(std::string actualUrl) return std::nullopt; } -static bool isNotDotGitDirectory(const Path & path) +bool isNotDotGitDirectory(const Path & path) { return baseNameOf(path) != ".git"; } +struct WorkdirInfo +{ + bool clean = false; + bool hasHead = false; +}; + +// Returns whether a git workdir is clean and has commits. +WorkdirInfo getWorkdirInfo(const Input & input, const Path & workdir) +{ + const bool submodules = maybeGetBoolAttr(input.attrs, "submodules").value_or(false); + auto gitDir = getGitDir(); + + auto env = getEnv(); + // Set LC_ALL to C: because we rely on the error messages from git rev-parse to determine what went wrong + // that way unknown errors can lead to a failure instead of continuing through the wrong code path + env["LC_ALL"] = "C"; + + /* Check whether HEAD points to something that looks like a commit, + since that is the refrence we want to use later on. */ + auto result = runProgram(RunOptions { + .program = "git", + .args = { "-C", workdir, "--git-dir", gitDir, "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }, + .environment = env, + .mergeStderrToStdout = true + }); + auto exitCode = WEXITSTATUS(result.first); + auto errorMessage = result.second; + + if (errorMessage.find("fatal: not a git repository") != std::string::npos) { + throw Error("'%s' is not a Git repository", workdir); + } else if (errorMessage.find("fatal: Needed a single revision") != std::string::npos) { + // indicates that the repo does not have any commits + // we want to proceed and will consider it dirty later + } else if (exitCode != 0) { + // any other errors should lead to a failure + throw Error("getting the HEAD of the Git tree '%s' failed with exit code %d:\n%s", workdir, exitCode, errorMessage); + } + + bool clean = false; + bool hasHead = exitCode == 0; + + try { + if (hasHead) { + // Using git diff is preferrable over lower-level operations here, + // because its conceptually simpler and we only need the exit code anyways. + auto gitDiffOpts = Strings({ "-C", workdir, "diff", "HEAD", "--quiet"}); + if (!submodules) { + // Changes in submodules should only make the tree dirty + // when those submodules will be copied as well. + gitDiffOpts.emplace_back("--ignore-submodules"); + } + gitDiffOpts.emplace_back("--"); + runProgram("git", true, gitDiffOpts); + + clean = true; + } + } catch (ExecError & e) { + if (!WIFEXITED(e.status) || WEXITSTATUS(e.status) != 1) throw; + } + + return WorkdirInfo { .clean = clean, .hasHead = hasHead }; +} + +std::pair fetchFromWorkdir(ref store, Input & input, const Path & workdir, const WorkdirInfo & workdirInfo) +{ + const bool submodules = maybeGetBoolAttr(input.attrs, "submodules").value_or(false); + + if (!fetchSettings.allowDirty) + throw Error("Git tree '%s' is dirty", workdir); + + if (fetchSettings.warnDirty) + warn("Git tree '%s' is dirty", workdir); + + auto gitOpts = Strings({ "-C", workdir, "ls-files", "-z" }); + if (submodules) + gitOpts.emplace_back("--recurse-submodules"); + + auto files = tokenizeString>( + runProgram("git", true, gitOpts), "\0"s); + + Path actualPath(absPath(workdir)); + + PathFilter filter = [&](const Path & p) -> bool { + assert(hasPrefix(p, actualPath)); + std::string file(p, actualPath.size() + 1); + + auto st = lstat(p); + + if (S_ISDIR(st.st_mode)) { + auto prefix = file + "/"; + auto i = files.lower_bound(prefix); + return i != files.end() && hasPrefix(*i, prefix); + } + + return files.count(file); + }; + + auto storePath = store->addToStore(input.getName(), actualPath, FileIngestionMethod::Recursive, htSHA256, filter); + + // FIXME: maybe we should use the timestamp of the last + // modified dirty file? + input.attrs.insert_or_assign( + "lastModified", + workdirInfo.hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); + + return {std::move(storePath), input}; +} +} // end namespace + struct GitInputScheme : InputScheme { std::optional inputFromURL(const ParsedURL & url) override @@ -319,100 +429,12 @@ struct GitInputScheme : InputScheme auto [isLocal, actualUrl_] = getActualUrl(input); auto actualUrl = actualUrl_; // work around clang bug - // If this is a local directory and no ref or revision is - // given, then allow the use of an unclean working tree. + /* If this is a local directory and no ref or revision is given, + allow fetching directly from a dirty workdir. */ if (!input.getRef() && !input.getRev() && isLocal) { - bool clean = false; - - auto env = getEnv(); - // Set LC_ALL to C: because we rely on the error messages from git rev-parse to determine what went wrong - // that way unknown errors can lead to a failure instead of continuing through the wrong code path - env["LC_ALL"] = "C"; - - /* Check whether HEAD points to something that looks like a commit, - since that is the refrence we want to use later on. */ - auto result = runProgram(RunOptions { - .program = "git", - .args = { "-C", actualUrl, "--git-dir", gitDir, "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }, - .environment = env, - .mergeStderrToStdout = true - }); - auto exitCode = WEXITSTATUS(result.first); - auto errorMessage = result.second; - - if (errorMessage.find("fatal: not a git repository") != std::string::npos) { - throw Error("'%s' is not a Git repository", actualUrl); - } else if (errorMessage.find("fatal: Needed a single revision") != std::string::npos) { - // indicates that the repo does not have any commits - // we want to proceed and will consider it dirty later - } else if (exitCode != 0) { - // any other errors should lead to a failure - throw Error("getting the HEAD of the Git tree '%s' failed with exit code %d:\n%s", actualUrl, exitCode, errorMessage); - } - - bool hasHead = exitCode == 0; - try { - if (hasHead) { - // Using git diff is preferrable over lower-level operations here, - // because its conceptually simpler and we only need the exit code anyways. - auto gitDiffOpts = Strings({ "-C", actualUrl, "--git-dir", gitDir, "diff", "HEAD", "--quiet"}); - if (!submodules) { - // Changes in submodules should only make the tree dirty - // when those submodules will be copied as well. - gitDiffOpts.emplace_back("--ignore-submodules"); - } - gitDiffOpts.emplace_back("--"); - runProgram("git", true, gitDiffOpts); - - clean = true; - } - } catch (ExecError & e) { - if (!WIFEXITED(e.status) || WEXITSTATUS(e.status) != 1) throw; - } - - if (!clean) { - - /* This is an unclean working tree. So copy all tracked files. */ - - if (!fetchSettings.allowDirty) - throw Error("Git tree '%s' is dirty", actualUrl); - - if (fetchSettings.warnDirty) - warn("Git tree '%s' is dirty", actualUrl); - - auto gitOpts = Strings({ "-C", actualUrl, "--git-dir", gitDir, "ls-files", "-z" }); - if (submodules) - gitOpts.emplace_back("--recurse-submodules"); - - auto files = tokenizeString>( - runProgram("git", true, gitOpts), "\0"s); - - Path actualPath(absPath(actualUrl)); - - PathFilter filter = [&](const Path & p) -> bool { - assert(hasPrefix(p, actualPath)); - std::string file(p, actualPath.size() + 1); - - auto st = lstat(p); - - if (S_ISDIR(st.st_mode)) { - auto prefix = file + "/"; - auto i = files.lower_bound(prefix); - return i != files.end() && hasPrefix(*i, prefix); - } - - return files.count(file); - }; - - auto storePath = store->addToStore(input.getName(), actualPath, FileIngestionMethod::Recursive, htSHA256, filter); - - // FIXME: maybe we should use the timestamp of the last - // modified dirty file? - input.attrs.insert_or_assign( - "lastModified", - hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "--git-dir", gitDir, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); - - return {std::move(storePath), input}; + auto workdirInfo = getWorkdirInfo(input, actualUrl); + if (!workdirInfo.clean) { + return fetchFromWorkdir(store, input, actualUrl, workdirInfo); } } @@ -425,7 +447,7 @@ struct GitInputScheme : InputScheme input.attrs.insert_or_assign("ref", *head); } - Attrs unlockedAttrs({ + const Attrs unlockedAttrs({ {"type", cacheType}, {"name", name}, {"url", actualUrl}, From 1203e489263fe867d0a1ae3fd9270f40c8a1c24e Mon Sep 17 00:00:00 2001 From: Kjetil Orbekk Date: Sat, 29 Jan 2022 14:22:55 -0500 Subject: [PATCH 09/21] Store cached head in cached git repo The previous head caching implementation stored two paths in the local cache; one for the cached git repo and another textfile containing the resolved HEAD ref. This commit instead stores the resolved HEAD by setting the HEAD ref in the local cache appropriately. --- src/libfetchers/git.cc | 89 +++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 31 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 00a7b85d8..968cd642a 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -10,6 +10,7 @@ #include "fetch-settings.hh" #include +#include #include #include @@ -37,7 +38,19 @@ bool isCacheFileWithinTtl(const time_t now, const struct stat& st) { return st.st_mtime + settings.tarballTtl > now; } -Path getCachePath(std::string key) { +bool touchCacheFile(const Path& path, const time_t& touch_time) +{ + struct timeval times[2]; + times[0].tv_sec = touch_time; + times[0].tv_usec = 0; + times[1].tv_sec = touch_time; + times[1].tv_usec = 0; + + return lutimes(path.c_str(), times) == 0; +} + +Path getCachePath(std::string key) +{ return getCacheDir() + "/nix/gitv3/" + hashString(htSHA256, key).to_string(Base32, false); } @@ -80,32 +93,42 @@ std::optional readHead(const Path & path) return std::nullopt; } -std::optional readHeadCached(std::string actualUrl) +// Persist the HEAD ref from the remote repo in the local cached repo. +bool storeCachedHead(const std::string& actualUrl, const std::string& headRef) +{ + Path cacheDir = getCachePath(actualUrl); + try { + runProgram("git", true, { "-C", cacheDir, "symbolic-ref", "--", "HEAD", headRef }); + } catch (ExecError &e) { + if (!WIFEXITED(e.status)) throw; + return false; + } + /* No need to touch refs/HEAD, because `git symbolic-ref` updates the mtime. */ + return true; +} + +std::optional readHeadCached(const std::string& actualUrl) { // Create a cache path to store the branch of the HEAD ref. Append something // in front of the URL to prevent collision with the repository itself. - Path cachePath = getCachePath("|" + actualUrl); + Path cacheDir = getCachePath(actualUrl); + Path headRefFile = cacheDir + "/HEAD"; + time_t now = time(0); struct stat st; std::optional cachedRef; - if (stat(cachePath.c_str(), &st) == 0 && - // The file may be empty, because writeFile() is not atomic. - st.st_size > 0) { - - // The cached ref is persisted unconditionally (see below). - cachedRef = readFile(cachePath); - if (isCacheFileWithinTtl(now, st)) { + if (stat(headRefFile.c_str(), &st) == 0) { + cachedRef = readHead(cacheDir); + if (cachedRef != std::nullopt && + *cachedRef != gitInitialBranch && + isCacheFileWithinTtl(now, st)) { debug("using cached HEAD ref '%s' for repo '%s'", *cachedRef, actualUrl); return cachedRef; } } auto ref = readHead(actualUrl); - if (ref) { - debug("storing cached HEAD ref '%s' for repo '%s' at '%s'", *ref, actualUrl, cachePath); - createDirs(dirOf(cachePath)); - writeFile(cachePath, *ref); return ref; } @@ -438,15 +461,6 @@ struct GitInputScheme : InputScheme } } - if (!input.getRef()) { - auto head = isLocal ? readHead(actualUrl) : readHeadCached(actualUrl); - if (!head) { - warn("could not read HEAD ref from repo at '%s', using 'master'", actualUrl); - head = "master"; - } - input.attrs.insert_or_assign("ref", *head); - } - const Attrs unlockedAttrs({ {"type", cacheType}, {"name", name}, @@ -457,14 +471,30 @@ struct GitInputScheme : InputScheme Path repoDir; if (isLocal) { + if (!input.getRef()) { + auto head = readHead(actualUrl); + if (!head) { + warn("could not read HEAD ref from repo at '%s', using 'master'", actualUrl); + head = "master"; + } + input.attrs.insert_or_assign("ref", *head); + } if (!input.getRev()) input.attrs.insert_or_assign("rev", Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "--git-dir", gitDir, "rev-parse", *input.getRef() })), htSHA1).gitRev()); repoDir = actualUrl; - } else { + const bool useHeadRef = !input.getRef(); + if (useHeadRef) { + auto head = readHeadCached(actualUrl); + if (!head) { + warn("could not read HEAD ref from repo at '%s', using 'master'", actualUrl); + head = "master"; + } + input.attrs.insert_or_assign("ref", *head); + } if (auto res = getCache()->lookup(store, unlockedAttrs)) { auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); @@ -538,13 +568,10 @@ struct GitInputScheme : InputScheme warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl); } - struct timeval times[2]; - times[0].tv_sec = now; - times[0].tv_usec = 0; - times[1].tv_sec = now; - times[1].tv_usec = 0; - - utimes(localRefFile.c_str(), times); + if (!touchCacheFile(localRefFile, now)) + warn("could not update mtime for file '%s': %s", localRefFile, strerror(errno)); + if (useHeadRef && !storeCachedHead(actualUrl, *input.getRef())) + warn("could not update cached head '%s' for '%s'", *input.getRef(), actualUrl); } if (!input.getRev()) From c21afd684cb5f59337b879684728884fd8275ce4 Mon Sep 17 00:00:00 2001 From: Kjetil Orbekk Date: Sun, 30 Jan 2022 11:30:57 -0500 Subject: [PATCH 10/21] Update `nix flake` documentation of `ref` handling Update the documentation about how `ref` is resolved if it is not specified. Add a note about special handling of local workdirs with `git+file`. --- src/nix/flake.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/nix/flake.md b/src/nix/flake.md index 7d179a6c4..c8251eb74 100644 --- a/src/nix/flake.md +++ b/src/nix/flake.md @@ -153,7 +153,7 @@ Currently the `type` attribute can be one of the following: git(+http|+https|+ssh|+git|+file|):(//)?(\?)? ``` - The `ref` attribute defaults to `master`. + The `ref` attribute defaults to resolving the `HEAD` reference. The `rev` attribute must denote a commit that exists in the branch or tag specified by the `ref` attribute, since Nix doesn't do a full @@ -161,6 +161,11 @@ Currently the `type` attribute can be one of the following: doesn't allow fetching a `rev` without a known `ref`). The default is the commit currently pointed to by `ref`. + When `git+file` is used without specifying `ref` or `rev`, files are + fetched directly from the local `path` as long as they have been added + to the Git repository. If there are uncommitted changes, the reference + is treated as dirty and a warning is printed. + For example, the following are valid Git flake references: * `git+https://example.org/my/repo` From 9bf296c970bf33b7ed53d7e2d8fbe44197482518 Mon Sep 17 00:00:00 2001 From: Kjetil Orbekk Date: Fri, 29 Apr 2022 18:30:00 -0400 Subject: [PATCH 11/21] Extract git reference parsing to a shared library These utility functions can be shared between the git and github fetchers. --- src/libfetchers/git-utils.cc | 25 +++++++++++++++++++++++++ src/libfetchers/git-utils.hh | 23 +++++++++++++++++++++++ src/libfetchers/git.cc | 29 +++++++++++------------------ src/libfetchers/github.cc | 28 +++++++++++----------------- 4 files changed, 70 insertions(+), 35 deletions(-) create mode 100644 src/libfetchers/git-utils.cc create mode 100644 src/libfetchers/git-utils.hh diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc new file mode 100644 index 000000000..060077098 --- /dev/null +++ b/src/libfetchers/git-utils.cc @@ -0,0 +1,25 @@ +#include "git-utils.hh" + +#include + +std::optional parseListReferenceHeadRef(std::string_view line) { + const static std::regex head_ref_regex("^ref: ([^\\s]+)\\t+HEAD$"); + std::match_results match; + if (std::regex_match(line.cbegin(), line.cend(), match, head_ref_regex)) { + return match[1]; + } else { + return std::nullopt; + } +} + +std::optional parseListReferenceForRev(std::string_view rev, std::string_view line) { + const static std::regex rev_regex("^([^\\t]+)\\t+(.*)$"); + std::match_results match; + if (!std::regex_match(line.cbegin(), line.cend(), match, rev_regex)) { + return std::nullopt; + } + if (rev != match[2].str()) { + return std::nullopt; + } + return match[1]; +} diff --git a/src/libfetchers/git-utils.hh b/src/libfetchers/git-utils.hh new file mode 100644 index 000000000..946a68a9e --- /dev/null +++ b/src/libfetchers/git-utils.hh @@ -0,0 +1,23 @@ +#pragma once + +#include +#include +#include + +// Parses the HEAD ref as reported by `git ls-remote --symref` +// +// Returns the head branch name as reported by `git ls-remote --symref`, e.g., if +// ls-remote returns the output below, "main" is returned based on the ref line. +// +// ref: refs/heads/main HEAD +// +// If the repository is in 'detached head' state (HEAD is pointing to a rev +// instead of a branch), parseListReferenceForRev("HEAD") may be used instead. +std::optional parseListReferenceHeadRef(std::string_view line); + +// Parses a reference line from `git ls-remote --symref`, e.g., +// parseListReferenceForRev("refs/heads/master", line) will return 6926... +// given the line below. +// +// 6926beab444c33fb57b21819b6642d032016bb1e refs/heads/master +std::optional parseListReferenceForRev(std::string_view rev, std::string_view line); diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 968cd642a..9d4348cf1 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -6,6 +6,7 @@ #include "url-parts.hh" #include "pathlocks.hh" #include "util.hh" +#include "git-utils.hh" #include "fetch-settings.hh" @@ -69,27 +70,19 @@ std::optional readHead(const Path & path) .args = {"ls-remote", "--symref", path}, }); if (exit_code != 0) { - return std::nullopt; + return std::nullopt; } - // Matches the common case when HEAD points to a branch, e.g.: - // "ref: refs/heads/main HEAD". - const static std::regex head_ref_regex("^ref:\\s*([^\\s]+)\\s*HEAD$"); - // Matches when HEAD points directly at a commit, e.g.: - // "71abcd... HEAD". - const static std::regex head_rev_regex("^([^\\s]+)\\s*HEAD$"); - - for (const auto & line : tokenizeString>(output, "\n")) { - std::smatch match; - if (std::regex_match(line, match, head_ref_regex)) { - debug("resolved HEAD ref '%s' for repo '%s'", match[1], path); - return match[1]; - } else if (std::regex_match(line, match, head_rev_regex)) { - debug("resolved HEAD ref '%s' for repo '%s'", match[1], path); - return match[1]; - } + std::string_view line = output; + line = line.substr(0, line.find("\n")); + if (const auto ref = parseListReferenceHeadRef(line); ref) { + debug("resolved HEAD ref '%s' for repo '%s'", *ref, path); + return *ref; + } + if (const auto rev = parseListReferenceForRev("HEAD", line); rev) { + debug("resolved HEAD rev '%s' for repo '%s'", *rev, path); + return *rev; } - return std::nullopt; } diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 58b6e7c04..1bdf2759f 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -4,7 +4,7 @@ #include "store-api.hh" #include "types.hh" #include "url-parts.hh" - +#include "git-utils.hh" #include "fetchers.hh" #include "fetch-settings.hh" @@ -383,35 +383,29 @@ struct SourceHutInputScheme : GitArchiveInputScheme std::string line; getline(is, line); - auto ref_index = line.find("ref: "); - if (ref_index == std::string::npos) { + auto r = parseListReferenceHeadRef(line); + if (!r) { throw BadURL("in '%d', couldn't resolve HEAD ref '%d'", input.to_string(), ref); } - - ref_uri = line.substr(ref_index+5, line.length()-1); - } else + ref_uri = *r; + } else { ref_uri = fmt("refs/(heads|tags)/%s", ref); + } auto file = store->toRealPath( downloadFile(store, fmt("%s/info/refs", base_url), "source", false, headers).storePath); std::ifstream is(file); std::string line; - std::string id; - while(getline(is, line)) { - // Append $ to avoid partial name matches - std::regex pattern(fmt("%s$", ref_uri)); - - if (std::regex_search(line, pattern)) { - id = line.substr(0, line.find('\t')); - break; - } + std::optional id; + while(!id && getline(is, line)) { + id = parseListReferenceForRev(ref_uri, line); } - if(id.empty()) + if(!id) throw BadURL("in '%d', couldn't find ref '%d'", input.to_string(), ref); - auto rev = Hash::parseAny(id, htSHA1); + auto rev = Hash::parseAny(*id, htSHA1); debug("HEAD revision for '%s' is %s", fmt("%s/%s", base_url, ref), rev.gitRev()); return rev; } From 1849e6a1f64734c488c2b1469249d65ce08cef93 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 30 Apr 2022 15:56:12 +0200 Subject: [PATCH 12/21] libfetchers/git: fix every occasion of a permission error I'm afraid I missed a few problematic `git(1)`-calls while implementing PR #6440, sorry for that! Upon investigating what went wrong, I realized that I only tested against the "cached"-case by accident because my git-checkout with my system's flake was apparently cached during my debugging. I managed to trigger the original issue again by running: $ git commit --allow-empty -m "tmp" $ sudo nixos-rebuild switch --flake .# -L --builders '' Since `repoDir` points to the checkout that's potentially owned by another user, I decided to add `--git-dir` to each call affecting `repoDir`. Since the `tmpDir` for the temporary submodule-checkout is created by Nix itself, it doesn't seem to be an issue. Sorry for that, it should be fine now. --- src/libfetchers/git.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index af40990e5..2ff27795d 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -383,7 +383,7 @@ struct GitInputScheme : InputScheme repo. */ if (input.getRev()) { try { - runProgram("git", true, { "-C", repoDir, "cat-file", "-e", input.getRev()->gitRev() }); + runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "cat-file", "-e", input.getRev()->gitRev() }); doFetch = false; } catch (ExecError & e) { if (WIFEXITED(e.status)) { @@ -418,7 +418,7 @@ struct GitInputScheme : InputScheme : ref == "HEAD" ? *ref : "refs/heads/" + *ref; - runProgram("git", true, { "-C", repoDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }); + runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }); } catch (Error & e) { if (!pathExists(localRefFile)) throw; warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl); @@ -459,7 +459,7 @@ struct GitInputScheme : InputScheme auto result = runProgram(RunOptions { .program = "git", - .args = { "-C", repoDir, "cat-file", "commit", input.getRev()->gitRev() }, + .args = { "-C", repoDir, "--git-dir", gitDir, "cat-file", "commit", input.getRev()->gitRev() }, .mergeStderrToStdout = true }); if (WEXITSTATUS(result.first) == 128 @@ -498,7 +498,7 @@ struct GitInputScheme : InputScheme auto source = sinkToSource([&](Sink & sink) { runProgram2({ .program = "git", - .args = { "-C", repoDir, "archive", input.getRev()->gitRev() }, + .args = { "-C", repoDir, "--git-dir", gitDir, "archive", input.getRev()->gitRev() }, .standardOut = &sink }); }); @@ -508,7 +508,7 @@ struct GitInputScheme : InputScheme auto storePath = store->addToStore(name, tmpDir, FileIngestionMethod::Recursive, htSHA256, filter); - auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", "--no-show-signature", input.getRev()->gitRev() })); + auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "log", "-1", "--format=%ct", "--no-show-signature", input.getRev()->gitRev() })); Attrs infoAttrs({ {"rev", input.getRev()->gitRev()}, @@ -517,7 +517,7 @@ struct GitInputScheme : InputScheme if (!shallow) infoAttrs.insert_or_assign("revCount", - std::stoull(runProgram("git", true, { "-C", repoDir, "rev-list", "--count", input.getRev()->gitRev() }))); + std::stoull(runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "rev-list", "--count", input.getRev()->gitRev() }))); if (!_input.getRev()) getCache()->add( From dde71899dd075a2d6479c8b953041b470bdfc19b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 2 May 2022 09:38:51 +0200 Subject: [PATCH 13/21] tests: Don't create tests/result https://hydra.nixos.org/log/lns780srkka4dv7r69mn4zfy6fdij4yr-nix-2.9.0pre20220428_4bb111c.drv --- tests/build-remote.sh | 3 ++- tests/fetchClosure.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/build-remote.sh b/tests/build-remote.sh index d1da134dc..e73c37ea4 100644 --- a/tests/build-remote.sh +++ b/tests/build-remote.sh @@ -34,7 +34,7 @@ outPath=$(readlink -f $TEST_ROOT/result) grep 'FOO BAR BAZ' $TEST_ROOT/machine0/$outPath -testPrintOutPath=$(nix build -L -v -f $file --print-out-paths --max-jobs 0 \ +testPrintOutPath=$(nix build -L -v -f $file --no-link --print-out-paths --max-jobs 0 \ --arg busybox $busybox \ --store $TEST_ROOT/machine0 \ --builders "$(join_by '; ' "${builders[@]}")" @@ -72,6 +72,7 @@ fi # Behavior of keep-failed out="$(nix-build 2>&1 failing.nix \ + --no-out-link \ --builders "$(join_by '; ' "${builders[@]}")" \ --keep-failed \ --store $TEST_ROOT/machine0 \ diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh index 96e4bb741..44050c878 100644 --- a/tests/fetchClosure.sh +++ b/tests/fetchClosure.sh @@ -7,7 +7,7 @@ clearStore clearCacheCache # Initialize binary cache. -nonCaPath=$(nix build --json --file ./dependencies.nix | jq -r .[].outputs.out) +nonCaPath=$(nix build --json --file ./dependencies.nix --no-link | jq -r .[].outputs.out) caPath=$(nix store make-content-addressed --json $nonCaPath | jq -r '.rewrites | map(.) | .[]') nix copy --to file://$cacheDir $nonCaPath From 61289ceee3417cd2aa8e8db9fe117a5813969aed Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 2 May 2022 13:37:53 +0200 Subject: [PATCH 14/21] Style fixes --- src/libfetchers/git-utils.cc | 6 ++++-- src/libfetchers/git.cc | 9 +++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 060077098..b2d6b7893 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -2,7 +2,8 @@ #include -std::optional parseListReferenceHeadRef(std::string_view line) { +std::optional parseListReferenceHeadRef(std::string_view line) +{ const static std::regex head_ref_regex("^ref: ([^\\s]+)\\t+HEAD$"); std::match_results match; if (std::regex_match(line.cbegin(), line.cend(), match, head_ref_regex)) { @@ -12,7 +13,8 @@ std::optional parseListReferenceHeadRef(std::string_view line) { } } -std::optional parseListReferenceForRev(std::string_view rev, std::string_view line) { +std::optional parseListReferenceForRev(std::string_view rev, std::string_view line) +{ const static std::regex rev_regex("^([^\\t]+)\\t+(.*)$"); std::match_results match; if (!std::regex_match(line.cbegin(), line.cend(), match, rev_regex)) { diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 2b81900fa..266246fe9 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -28,14 +28,11 @@ const std::string gitInitialBranch = "__nix_dummy_branch"; std::string getGitDir() { - auto gitDir = getEnv("GIT_DIR"); - if (!gitDir) { - return ".git"; - } - return *gitDir; + return getEnv("GIT_DIR").value_or(".git"); } -bool isCacheFileWithinTtl(const time_t now, const struct stat& st) { +bool isCacheFileWithinTtl(const time_t now, const struct stat & st) +{ return st.st_mtime + settings.tarballTtl > now; } From 4845886bed18bb12dbfb7bf909b24cc49abd8da6 Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Mon, 2 May 2022 14:22:00 +0200 Subject: [PATCH 15/21] Revert "tests: Distinguish crashes from expected failures" This reverts commit 143b73f52dabb35cd56551c24caef95466bce488. --- tests/lang.sh | 58 +++++++++++++++++++-------------------------------- 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/tests/lang.sh b/tests/lang.sh index 6cb4b6e2b..61bb444ba 100644 --- a/tests/lang.sh +++ b/tests/lang.sh @@ -10,49 +10,32 @@ nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" (throw set +x fail=0 -just_failed=0 - -safe_expect () { - expected=$1 name=$2 msg=$3 cmd=$4 - just_failed=0 - res=0 - eval "$cmd" || res=$? - - if [ $res -ge 2 ]; then - [ $res -le 128 ] || { echo "FAIL: $name returned unexpected code '$res' (should be 0 or 1)"; just_failed=1; } - [ $res -gt 128 ] || { echo "FAIL: $name crashed with $res ($(kill -l $(($res - 128))))"; just_failed=1; } - elif [ $expected = "fail" ]; then - [ $res -ne 0 ] || { echo "FAIL: $name $msg"; just_failed=1; } - elif [ $expected = "pass" ]; then - [ $res -eq 0 ] || { echo "FAIL: $name $msg"; just_failed=1; } - else - echo "usage: safe_expect [pass|fail] name msg command" - exit 1 - fi - - [ $just_failed -eq 0 ] || fail=1 -} - for i in lang/parse-fail-*.nix; do echo "parsing $i (should fail)"; i=$(basename $i .nix) - safe_expect fail "$i" "shouldn't parse" \ - "nix-instantiate --parse - < lang/$i.nix" + if nix-instantiate --parse - < lang/$i.nix; then + echo "FAIL: $i shouldn't parse" + fail=1 + fi done for i in lang/parse-okay-*.nix; do echo "parsing $i (should succeed)"; i=$(basename $i .nix) - safe_expect pass "$i" "should parse" \ - "nix-instantiate --parse - < lang/$i.nix > lang/$i.out" + if ! nix-instantiate --parse - < lang/$i.nix > lang/$i.out; then + echo "FAIL: $i should parse" + fail=1 + fi done for i in lang/eval-fail-*.nix; do echo "evaluating $i (should fail)"; i=$(basename $i .nix) - safe_expect fail "$i" "shouldn't evaluate" \ - "nix-instantiate --eval lang/$i.nix" + if nix-instantiate --eval lang/$i.nix; then + echo "FAIL: $i shouldn't evaluate" + fail=1 + fi done for i in lang/eval-okay-*.nix; do @@ -62,20 +45,23 @@ for i in lang/eval-okay-*.nix; do if test -e lang/$i.exp; then flags= if test -e lang/$i.flags; then - flags='$'"(cat lang/$i.flags)" # because vi does not highlight "\$(...)" properly ;-). + flags=$(cat lang/$i.flags) fi - safe_expect pass "$i" "should evaluate" \ - "NIX_PATH=lang/dir3:lang/dir4 nix-instantiate $flags --eval --strict lang/$i.nix > lang/$i.out" - if [ $just_failed -eq 0 ] && ! diff lang/$i.out lang/$i.exp; then + if ! NIX_PATH=lang/dir3:lang/dir4 nix-instantiate $flags --eval --strict lang/$i.nix > lang/$i.out; then + echo "FAIL: $i should evaluate" + fail=1 + elif ! diff lang/$i.out lang/$i.exp; then echo "FAIL: evaluation result of $i not as expected" fail=1 fi fi if test -e lang/$i.exp.xml; then - safe_expect pass "$i" "should evaluate" \ - "nix-instantiate --eval --xml --no-location --strict lang/$i.nix > lang/$i.out.xml" - if [ $just_failed -eq 0 ] && ! cmp -s lang/$i.out.xml lang/$i.exp.xml; then + if ! nix-instantiate --eval --xml --no-location --strict \ + lang/$i.nix > lang/$i.out.xml; then + echo "FAIL: $i should evaluate" + fail=1 + elif ! cmp -s lang/$i.out.xml lang/$i.exp.xml; then echo "FAIL: XML evaluation result of $i not as expected" fail=1 fi From 275f8561eb80a0f341fa2a03d731b239b1783da0 Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Mon, 2 May 2022 15:12:50 +0200 Subject: [PATCH 16/21] tests/lang: Distinguish crashes from expected failures --- tests/common.sh.in | 11 ++++++----- tests/lang.sh | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/common.sh.in b/tests/common.sh.in index 8ce28d318..6cb579e0d 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -157,11 +157,12 @@ expect() { local expected res expected="$1" shift - set +e - "$@" - res="$?" - set -e - [[ $res -eq $expected ]] + "$@" || res="$?" + if [[ $res -ne $expected ]]; then + echo "Expected '$expected' but got '$res' while running '$*'" + return 1 + fi + return 0 } needLocalStore() { diff --git a/tests/lang.sh b/tests/lang.sh index 61bb444ba..f09eaeb31 100644 --- a/tests/lang.sh +++ b/tests/lang.sh @@ -4,6 +4,7 @@ export TEST_VAR=foo # for eval-okay-getenv.nix export NIX_REMOTE=dummy:// nix-instantiate --eval -E 'builtins.trace "Hello" 123' 2>&1 | grep -q Hello +nix-instantiate --eval -E 'builtins.addErrorContext "Hello" 123' 2>&1 (! nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" 123' 2>&1 | grep -q Hello) nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" (throw "Foo")' 2>&1 | grep -q Hello @@ -14,7 +15,7 @@ fail=0 for i in lang/parse-fail-*.nix; do echo "parsing $i (should fail)"; i=$(basename $i .nix) - if nix-instantiate --parse - < lang/$i.nix; then + if ! expect 1 nix-instantiate --parse - < lang/$i.nix; then echo "FAIL: $i shouldn't parse" fail=1 fi @@ -23,7 +24,7 @@ done for i in lang/parse-okay-*.nix; do echo "parsing $i (should succeed)"; i=$(basename $i .nix) - if ! nix-instantiate --parse - < lang/$i.nix > lang/$i.out; then + if ! expect 0 nix-instantiate --parse - < lang/$i.nix > lang/$i.out; then echo "FAIL: $i should parse" fail=1 fi @@ -32,7 +33,7 @@ done for i in lang/eval-fail-*.nix; do echo "evaluating $i (should fail)"; i=$(basename $i .nix) - if nix-instantiate --eval lang/$i.nix; then + if ! expect 1 nix-instantiate --eval lang/$i.nix; then echo "FAIL: $i shouldn't evaluate" fail=1 fi @@ -47,7 +48,7 @@ for i in lang/eval-okay-*.nix; do if test -e lang/$i.flags; then flags=$(cat lang/$i.flags) fi - if ! NIX_PATH=lang/dir3:lang/dir4 nix-instantiate $flags --eval --strict lang/$i.nix > lang/$i.out; then + if ! expect 0 env NIX_PATH=lang/dir3:lang/dir4 nix-instantiate $flags --eval --strict lang/$i.nix > lang/$i.out; then echo "FAIL: $i should evaluate" fail=1 elif ! diff lang/$i.out lang/$i.exp; then @@ -57,7 +58,7 @@ for i in lang/eval-okay-*.nix; do fi if test -e lang/$i.exp.xml; then - if ! nix-instantiate --eval --xml --no-location --strict \ + if ! expect 0 nix-instantiate --eval --xml --no-location --strict \ lang/$i.nix > lang/$i.out.xml; then echo "FAIL: $i should evaluate" fail=1 From 4a79cba5118f29b896f3d50164beacd4901ab01f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 22 Apr 2022 15:17:01 +0200 Subject: [PATCH 17/21] Allow selecting derivation outputs using 'installable!outputs' E.g. 'nixpkgs#glibc^dev,static' or 'nixpkgs#glibc^*'. --- doc/manual/src/release-notes/rl-next.md | 10 ++++++ src/libcmd/installables.cc | 46 ++++++++++++++++++++----- src/libcmd/installables.hh | 2 ++ src/libexpr/flake/flakeref.cc | 11 ++++++ src/libexpr/flake/flakeref.hh | 8 +++++ src/libstore/path-with-outputs.cc | 16 +++++++++ src/libstore/path-with-outputs.hh | 12 +++++++ src/libstore/tests/path-with-outputs.cc | 46 +++++++++++++++++++++++++ src/nix/bundle.cc | 4 +-- src/nix/develop.cc | 1 + src/nix/flake.cc | 2 +- src/nix/nix.md | 45 ++++++++++++++++++++++++ src/nix/profile.cc | 1 + tests/build.sh | 41 ++++++++++++++++++++++ tests/multiple-outputs.nix | 7 ++++ tests/shell-hello.nix | 11 +++++- tests/shell.sh | 4 +++ 17 files changed, 255 insertions(+), 12 deletions(-) create mode 100644 src/libstore/tests/path-with-outputs.cc diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 7b3ad4e58..efd893662 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -14,3 +14,13 @@ * `nix build` has a new `--print-out-paths` flag to print the resulting output paths. This matches the default behaviour of `nix-build`. + +* You can now specify which outputs of a derivation `nix` should + operate on using the syntax `installable^outputs`, + e.g. `nixpkgs#glibc^dev,static` or `nixpkgs#glibc^*`. By default, + `nix` will use the outputs specified by the derivation's + `meta.outputsToInstall` attribute if it exists, or all outputs + otherwise. + + Selecting derivation outputs using the attribute selection syntax + (e.g. `nixpkgs#glibc.dev`) no longer works. diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index e2ee47dea..55a5e91e9 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -464,9 +464,19 @@ struct InstallableAttrPath : InstallableValue SourceExprCommand & cmd; RootValue v; std::string attrPath; + OutputsSpec outputsSpec; - InstallableAttrPath(ref state, SourceExprCommand & cmd, Value * v, const std::string & attrPath) - : InstallableValue(state), cmd(cmd), v(allocRootValue(v)), attrPath(attrPath) + InstallableAttrPath( + ref state, + SourceExprCommand & cmd, + Value * v, + const std::string & attrPath, + OutputsSpec outputsSpec) + : InstallableValue(state) + , cmd(cmd) + , v(allocRootValue(v)) + , attrPath(attrPath) + , outputsSpec(std::move(outputsSpec)) { } std::string what() const override { return attrPath; } @@ -495,9 +505,15 @@ std::vector InstallableAttrPath::toDerivations auto drvPath = drvInfo.queryDrvPath(); if (!drvPath) throw Error("'%s' is not a derivation", what()); + std::set outputsToInstall; - for (auto & output : drvInfo.queryOutputs(false, true)) - outputsToInstall.insert(output.first); + + if (auto outputNames = std::get_if(&outputsSpec)) + outputsToInstall = *outputNames; + else + for (auto & output : drvInfo.queryOutputs(false, std::get_if(&outputsSpec))) + outputsToInstall.insert(output.first); + res.push_back(DerivationInfo { .drvPath = *drvPath, .outputsToInstall = std::move(outputsToInstall) @@ -578,6 +594,7 @@ InstallableFlake::InstallableFlake( ref state, FlakeRef && flakeRef, std::string_view fragment, + OutputsSpec outputsSpec, Strings attrPaths, Strings prefixes, const flake::LockFlags & lockFlags) @@ -585,6 +602,7 @@ InstallableFlake::InstallableFlake( flakeRef(flakeRef), attrPaths(fragment == "" ? attrPaths : Strings{(std::string) fragment}), prefixes(fragment == "" ? Strings{} : prefixes), + outputsSpec(std::move(outputsSpec)), lockFlags(lockFlags) { if (cmd && cmd->getAutoArgs(*state)->size()) @@ -609,14 +627,19 @@ std::tuple InstallableF for (auto & s : aOutputsToInstall->getListOfStrings()) outputsToInstall.insert(s); - if (outputsToInstall.empty()) + if (outputsToInstall.empty() || std::get_if(&outputsSpec)) { + outputsToInstall.clear(); if (auto aOutputs = attr->maybeGetAttr(state->sOutputs)) for (auto & s : aOutputs->getListOfStrings()) outputsToInstall.insert(s); + } if (outputsToInstall.empty()) outputsToInstall.insert("out"); + if (auto outputNames = std::get_if(&outputsSpec)) + outputsToInstall = *outputNames; + auto drvInfo = DerivationInfo { .drvPath = std::move(drvPath), .outputsToInstall = std::move(outputsToInstall), @@ -742,8 +765,14 @@ std::vector> SourceExprCommand::parseInstallables( state->eval(e, *vFile); } - for (auto & s : ss) - result.push_back(std::make_shared(state, *this, vFile, s == "." ? "" : s)); + for (auto & s : ss) { + auto [prefix, outputsSpec] = parseOutputsSpec(s); + result.push_back( + std::make_shared( + state, *this, vFile, + prefix == "." ? "" : prefix, + outputsSpec)); + } } else { @@ -762,12 +791,13 @@ std::vector> SourceExprCommand::parseInstallables( } try { - auto [flakeRef, fragment] = parseFlakeRefWithFragment(s, absPath(".")); + auto [flakeRef, fragment, outputsSpec] = parseFlakeRefWithFragmentAndOutputsSpec(s, absPath(".")); result.push_back(std::make_shared( this, getEvalState(), std::move(flakeRef), fragment, + outputsSpec, getDefaultFlakeAttrPaths(), getDefaultFlakeAttrPathPrefixes(), lockFlags)); diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index 3c2c33549..1a5a96153 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -156,6 +156,7 @@ struct InstallableFlake : InstallableValue FlakeRef flakeRef; Strings attrPaths; Strings prefixes; + OutputsSpec outputsSpec; const flake::LockFlags & lockFlags; mutable std::shared_ptr _lockedFlake; @@ -164,6 +165,7 @@ struct InstallableFlake : InstallableValue ref state, FlakeRef && flakeRef, std::string_view fragment, + OutputsSpec outputsSpec, Strings attrPaths, Strings prefixes, const flake::LockFlags & lockFlags); diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index c1eae413f..1dcc4555a 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -238,4 +238,15 @@ std::pair FlakeRef::fetchTree(ref store) const return {std::move(tree), FlakeRef(std::move(lockedInput), subdir)}; } +std::tuple parseFlakeRefWithFragmentAndOutputsSpec( + const std::string & url, + const std::optional & baseDir, + bool allowMissing, + bool isFlake) +{ + auto [prefix, outputsSpec] = parseOutputsSpec(url); + auto [flakeRef, fragment] = parseFlakeRefWithFragment(prefix, baseDir, allowMissing, isFlake); + return {std::move(flakeRef), fragment, outputsSpec}; +} + } diff --git a/src/libexpr/flake/flakeref.hh b/src/libexpr/flake/flakeref.hh index 1fddfd9a0..a9182f4bf 100644 --- a/src/libexpr/flake/flakeref.hh +++ b/src/libexpr/flake/flakeref.hh @@ -3,6 +3,7 @@ #include "types.hh" #include "hash.hh" #include "fetchers.hh" +#include "path-with-outputs.hh" #include @@ -79,4 +80,11 @@ std::pair parseFlakeRefWithFragment( std::optional> maybeParseFlakeRefWithFragment( const std::string & url, const std::optional & baseDir = {}); +std::tuple parseFlakeRefWithFragmentAndOutputsSpec( + const std::string & url, + const std::optional & baseDir = {}, + bool allowMissing = false, + bool isFlake = true); + + } diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index 078c117bd..7d180a0f6 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -1,6 +1,8 @@ #include "path-with-outputs.hh" #include "store-api.hh" +#include + namespace nix { std::string StorePathWithOutputs::to_string(const Store & store) const @@ -68,4 +70,18 @@ StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std: return StorePathWithOutputs { store.followLinksToStorePath(path), std::move(outputs) }; } +std::pair parseOutputsSpec(const std::string & s) +{ + static std::regex regex(R"((.*)\^((\*)|([a-z]+(,[a-z]+)*)))"); + + std::smatch match; + if (!std::regex_match(s, match, regex)) + return {s, DefaultOutputs()}; + + if (match[3].matched) + return {match[1], AllOutputs()}; + + return {match[1], tokenizeString(match[4].str(), ",")}; +} + } diff --git a/src/libstore/path-with-outputs.hh b/src/libstore/path-with-outputs.hh index 4c4023dcb..e4235d197 100644 --- a/src/libstore/path-with-outputs.hh +++ b/src/libstore/path-with-outputs.hh @@ -32,4 +32,16 @@ StorePathWithOutputs parsePathWithOutputs(const Store & store, std::string_view StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std::string_view pathWithOutputs); +typedef std::set OutputNames; + +struct AllOutputs { }; + +struct DefaultOutputs { }; + +typedef std::variant OutputsSpec; + +/* Parse a string of the form 'prefix^output1,...outputN' or + 'prefix^*', returning the prefix and the outputs spec. */ +std::pair parseOutputsSpec(const std::string & s); + } diff --git a/src/libstore/tests/path-with-outputs.cc b/src/libstore/tests/path-with-outputs.cc new file mode 100644 index 000000000..350ea7ffd --- /dev/null +++ b/src/libstore/tests/path-with-outputs.cc @@ -0,0 +1,46 @@ +#include "path-with-outputs.hh" + +#include + +namespace nix { + +TEST(parseOutputsSpec, basic) +{ + { + auto [prefix, outputsSpec] = parseOutputsSpec("foo"); + ASSERT_EQ(prefix, "foo"); + ASSERT_TRUE(std::get_if(&outputsSpec)); + } + + { + auto [prefix, outputsSpec] = parseOutputsSpec("foo^*"); + ASSERT_EQ(prefix, "foo"); + ASSERT_TRUE(std::get_if(&outputsSpec)); + } + + { + auto [prefix, outputsSpec] = parseOutputsSpec("foo^out"); + ASSERT_EQ(prefix, "foo"); + ASSERT_TRUE(std::get(outputsSpec) == OutputNames({"out"})); + } + + { + auto [prefix, outputsSpec] = parseOutputsSpec("foo^out,bin"); + ASSERT_EQ(prefix, "foo"); + ASSERT_TRUE(std::get(outputsSpec) == OutputNames({"out", "bin"})); + } + + { + auto [prefix, outputsSpec] = parseOutputsSpec("foo^bar^out,bin"); + ASSERT_EQ(prefix, "foo^bar"); + ASSERT_TRUE(std::get(outputsSpec) == OutputNames({"out", "bin"})); + } + + { + auto [prefix, outputsSpec] = parseOutputsSpec("foo^&*()"); + ASSERT_EQ(prefix, "foo^&*()"); + ASSERT_TRUE(std::get_if(&outputsSpec)); + } +} + +} diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 2421adf4e..2e48e4c74 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -75,10 +75,10 @@ struct CmdBundle : InstallableCommand auto val = installable->toValue(*evalState).first; - auto [bundlerFlakeRef, bundlerName] = parseFlakeRefWithFragment(bundler, absPath(".")); + auto [bundlerFlakeRef, bundlerName, outputsSpec] = parseFlakeRefWithFragmentAndOutputsSpec(bundler, absPath(".")); const flake::LockFlags lockFlags{ .writeLockFile = false }; InstallableFlake bundler{this, - evalState, std::move(bundlerFlakeRef), bundlerName, + evalState, std::move(bundlerFlakeRef), bundlerName, outputsSpec, {"bundlers." + settings.thisSystem.get() + ".default", "defaultBundler." + settings.thisSystem.get() }, diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 7fc74d34e..1190b8348 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -507,6 +507,7 @@ struct CmdDevelop : Common, MixEnvironment state, installable->nixpkgsFlakeRef(), "bashInteractive", + DefaultOutputs(), Strings{}, Strings{"legacyPackages." + settings.thisSystem.get() + "."}, nixpkgsLockFlags); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 6a34ca67b..1938ce4e6 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -724,7 +724,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand auto [templateFlakeRef, templateName] = parseFlakeRefWithFragment(templateUrl, absPath(".")); auto installable = InstallableFlake(nullptr, - evalState, std::move(templateFlakeRef), templateName, + evalState, std::move(templateFlakeRef), templateName, DefaultOutputs(), defaultTemplateAttrPaths, defaultTemplateAttrPathsPrefixes, lockFlags); diff --git a/src/nix/nix.md b/src/nix/nix.md index 0dacadee6..d48682a94 100644 --- a/src/nix/nix.md +++ b/src/nix/nix.md @@ -146,6 +146,51 @@ For most commands, if no installable is specified, the default is `.`, i.e. Nix will operate on the default flake output attribute of the flake in the current directory. +## Derivation output selection + +Derivations can have multiple outputs, each corresponding to a +different store path. For instance, a package can have a `bin` output +that contains programs, and a `dev` output that provides development +artifacts like C/C++ header files. The outputs on which `nix` commands +operate are determined as follows: + +* You can explicitly specify the desired outputs using the syntax + *installable*`^`*output1*`,`*...*`,`*outputN*. For example, you can + obtain the `dev` and `static` outputs of the `glibc` package: + + ```console + # nix build 'nixpkgs#glibc^dev,static' + # ls ./result-dev/include/ ./result-static/lib/ + … + ``` + +* You can also specify that *all* outputs should be used using the + syntax *installable*`^*`. For example, the following shows the size + of all outputs of the `glibc` package in the binary cache: + + ```console + # nix path-info -S --eval-store auto --store https://cache.nixos.org 'nixpkgs#glibc^*' + /nix/store/g02b1lpbddhymmcjb923kf0l7s9nww58-glibc-2.33-123 33208200 + /nix/store/851dp95qqiisjifi639r0zzg5l465ny4-glibc-2.33-123-bin 36142896 + /nix/store/kdgs3q6r7xdff1p7a9hnjr43xw2404z7-glibc-2.33-123-debug 155787312 + /nix/store/n4xa8h6pbmqmwnq0mmsz08l38abb06zc-glibc-2.33-123-static 42488328 + /nix/store/q6580lr01jpcsqs4r5arlh4ki2c1m9rv-glibc-2.33-123-dev 44200560 + ``` + +* If you didn't specify the desired outputs, but the derivation has an + attribute `meta.outputsToInstall`, Nix will use those outputs. For + example, since the package `nixpkgs#libxml2` has this attribute: + + ```console + # nix eval 'nixpkgs#libxml2.meta.outputsToInstall' + [ "bin" "man" ] + ``` + + a command like `nix shell nixpkgs#libxml2` will provide only those + two outputs by default. + +* Otherwise, Nix will use all outputs of the derivation. + # Nix stores Most `nix` subcommands operate on a *Nix store*. diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 52c918016..78c8af80c 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -443,6 +443,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf getEvalState(), FlakeRef(element.source->originalRef), "", + DefaultOutputs(), // FIXME Strings{element.source->attrPath}, Strings{}, lockFlags); diff --git a/tests/build.sh b/tests/build.sh index 339155991..ff16d1603 100644 --- a/tests/build.sh +++ b/tests/build.sh @@ -2,6 +2,8 @@ source common.sh clearStore +set -o pipefail + # Make sure that 'nix build' returns all outputs by default. nix build -f multiple-outputs.nix --json a b --no-link | jq --exit-status ' (.[0] | @@ -15,6 +17,45 @@ nix build -f multiple-outputs.nix --json a b --no-link | jq --exit-status ' (.outputs.out | match(".*multiple-outputs-b"))) ' +# Test output selection using the '^' syntax. +nix build -f multiple-outputs.nix --json a^first --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys == ["first"])) +' + +nix build -f multiple-outputs.nix --json a^second,first --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys == ["first", "second"])) +' + +nix build -f multiple-outputs.nix --json 'a^*' --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys == ["first", "second"])) +' + +# Test that 'outputsToInstall' is respected by default. +nix build -f multiple-outputs.nix --json e --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-e.drv")) and + (.outputs | keys == ["a", "b"])) +' + +# But not when it's overriden. +nix build -f multiple-outputs.nix --json e^a --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-e.drv")) and + (.outputs | keys == ["a"])) +' + +nix build -f multiple-outputs.nix --json 'e^*' --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-e.drv")) and + (.outputs | keys == ["a", "b", "c"])) +' + testNormalization () { clearStore outPath=$(nix-build ./simple.nix --no-out-link) diff --git a/tests/multiple-outputs.nix b/tests/multiple-outputs.nix index b915493f7..624a5dade 100644 --- a/tests/multiple-outputs.nix +++ b/tests/multiple-outputs.nix @@ -80,4 +80,11 @@ rec { ''; }).a; + e = mkDerivation { + name = "multiple-outputs-e"; + outputs = [ "a" "b" "c" ]; + meta.outputsToInstall = [ "a" "b" ]; + buildCommand = "mkdir $a $b $c"; + }; + } diff --git a/tests/shell-hello.nix b/tests/shell-hello.nix index 77dcbd2a9..3fdd3501d 100644 --- a/tests/shell-hello.nix +++ b/tests/shell-hello.nix @@ -3,15 +3,24 @@ with import ./config.nix; { hello = mkDerivation { name = "hello"; + outputs = [ "out" "dev" ]; + meta.outputsToInstall = [ "out" ]; buildCommand = '' - mkdir -p $out/bin + mkdir -p $out/bin $dev/bin + cat > $out/bin/hello < $dev/bin/hello2 < Date: Tue, 3 May 2022 14:37:28 +0200 Subject: [PATCH 18/21] nix profile: Support overriding outputs --- src/libstore/path-with-outputs.cc | 40 ++++++++++++++++++++++++++++ src/libstore/path-with-outputs.hh | 14 ++++++++-- src/libutil/experimental-features.cc | 6 +++-- src/libutil/experimental-features.hh | 4 +-- src/nix/profile-install.md | 7 +++++ src/nix/profile.cc | 30 ++++++++++++--------- tests/nix-profile.sh | 19 +++++++++++++ 7 files changed, 101 insertions(+), 19 deletions(-) diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index 7d180a0f6..d6d67ea05 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -1,5 +1,6 @@ #include "path-with-outputs.hh" #include "store-api.hh" +#include "nlohmann/json.hpp" #include @@ -84,4 +85,43 @@ std::pair parseOutputsSpec(const std::string & s) return {match[1], tokenizeString(match[4].str(), ",")}; } +std::string printOutputsSpec(const OutputsSpec & outputsSpec) +{ + if (std::get_if(&outputsSpec)) + return ""; + + if (std::get_if(&outputsSpec)) + return "^*"; + + if (auto outputNames = std::get_if(&outputsSpec)) + return "^" + concatStringsSep(",", *outputNames); + + assert(false); +} + +void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec) +{ + if (std::get_if(&outputsSpec)) + json = nullptr; + + else if (std::get_if(&outputsSpec)) + json = std::vector({"*"}); + + else if (auto outputNames = std::get_if(&outputsSpec)) + json = *outputNames; +} + +void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec) +{ + if (json.is_null()) + outputsSpec = DefaultOutputs(); + else { + auto names = json.get(); + if (names == OutputNames({"*"})) + outputsSpec = AllOutputs(); + else + outputsSpec = names; + } +} + } diff --git a/src/libstore/path-with-outputs.hh b/src/libstore/path-with-outputs.hh index e4235d197..0cb5eb223 100644 --- a/src/libstore/path-with-outputs.hh +++ b/src/libstore/path-with-outputs.hh @@ -4,6 +4,7 @@ #include "path.hh" #include "derived-path.hh" +#include "nlohmann/json_fwd.hpp" namespace nix { @@ -34,9 +35,13 @@ StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std: typedef std::set OutputNames; -struct AllOutputs { }; +struct AllOutputs { + bool operator < (const AllOutputs & _) const { return false; } +}; -struct DefaultOutputs { }; +struct DefaultOutputs { + bool operator < (const DefaultOutputs & _) const { return false; } +}; typedef std::variant OutputsSpec; @@ -44,4 +49,9 @@ typedef std::variant OutputsSpec; 'prefix^*', returning the prefix and the outputs spec. */ std::pair parseOutputsSpec(const std::string & s); +std::string printOutputsSpec(const OutputsSpec & outputsSpec); + +void to_json(nlohmann::json &, const OutputsSpec &); +void from_json(const nlohmann::json &, OutputsSpec &); + } diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index df37edf57..9326cd08c 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -58,11 +58,13 @@ std::ostream & operator <<(std::ostream & str, const ExperimentalFeature & featu return str << showExperimentalFeature(feature); } -void to_json(nlohmann::json& j, const ExperimentalFeature& feature) { +void to_json(nlohmann::json & j, const ExperimentalFeature & feature) +{ j = showExperimentalFeature(feature); } -void from_json(const nlohmann::json& j, ExperimentalFeature& feature) { +void from_json(const nlohmann::json & j, ExperimentalFeature & feature) +{ const std::string input = j; const auto parsed = parseExperimentalFeature(input); diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index a6d080094..57512830c 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -55,7 +55,7 @@ public: * Semi-magic conversion to and from json. * See the nlohmann/json readme for more details. */ -void to_json(nlohmann::json&, const ExperimentalFeature&); -void from_json(const nlohmann::json&, ExperimentalFeature&); +void to_json(nlohmann::json &, const ExperimentalFeature &); +void from_json(const nlohmann::json &, ExperimentalFeature &); } diff --git a/src/nix/profile-install.md b/src/nix/profile-install.md index e3009491e..aed414963 100644 --- a/src/nix/profile-install.md +++ b/src/nix/profile-install.md @@ -20,6 +20,13 @@ R""( # nix profile install nixpkgs/d73407e8e6002646acfdef0e39ace088bacc83da#hello ``` +* Install a specific output of a package: + + ```console + # nix profile install nixpkgs#bash^man + ``` + + # Description This command adds *installables* to a Nix profile. diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 78c8af80c..685776bec 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -22,13 +22,13 @@ struct ProfileElementSource // FIXME: record original attrpath. FlakeRef resolvedRef; std::string attrPath; - // FIXME: output names + OutputsSpec outputs; bool operator < (const ProfileElementSource & other) const { return - std::pair(originalRef.to_string(), attrPath) < - std::pair(other.originalRef.to_string(), other.attrPath); + std::tuple(originalRef.to_string(), attrPath, outputs) < + std::tuple(other.originalRef.to_string(), other.attrPath, other.outputs); } }; @@ -42,7 +42,7 @@ struct ProfileElement std::string describe() const { if (source) - return fmt("%s#%s", source->originalRef, source->attrPath); + return fmt("%s#%s%s", source->originalRef, source->attrPath, printOutputsSpec(source->outputs)); StringSet names; for (auto & path : storePaths) names.insert(DrvName(path.name()).name); @@ -98,7 +98,7 @@ struct ProfileManifest auto version = json.value("version", 0); std::string sUrl; std::string sOriginalUrl; - switch(version){ + switch (version) { case 1: sUrl = "uri"; sOriginalUrl = "originalUri"; @@ -116,11 +116,12 @@ struct ProfileManifest for (auto & p : e["storePaths"]) element.storePaths.insert(state.store->parseStorePath((std::string) p)); element.active = e["active"]; - if (e.value(sUrl,"") != "") { - element.source = ProfileElementSource{ + if (e.value(sUrl, "") != "") { + element.source = ProfileElementSource { parseFlakeRef(e[sOriginalUrl]), parseFlakeRef(e[sUrl]), - e["attrPath"] + e["attrPath"], + e["outputs"].get() }; } elements.emplace_back(std::move(element)); @@ -156,6 +157,7 @@ struct ProfileManifest obj["originalUrl"] = element.source->originalRef.to_string(); obj["url"] = element.source->resolvedRef.to_string(); obj["attrPath"] = element.source->attrPath; + obj["outputs"] = element.source->outputs; } array.push_back(obj); } @@ -283,10 +285,11 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile if (auto installable2 = std::dynamic_pointer_cast(installable)) { // FIXME: make build() return this? auto [attrPath, resolvedRef, drv] = installable2->toDerivation(); - element.source = ProfileElementSource{ + element.source = ProfileElementSource { installable2->flakeRef, resolvedRef, attrPath, + installable2->outputsSpec }; } @@ -443,7 +446,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf getEvalState(), FlakeRef(element.source->originalRef), "", - DefaultOutputs(), // FIXME + element.source->outputs, Strings{element.source->attrPath}, Strings{}, lockFlags); @@ -455,10 +458,11 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf printInfo("upgrading '%s' from flake '%s' to '%s'", element.source->attrPath, element.source->resolvedRef, resolvedRef); - element.source = ProfileElementSource{ + element.source = ProfileElementSource { installable->flakeRef, resolvedRef, attrPath, + installable->outputsSpec }; installables.push_back(installable); @@ -514,8 +518,8 @@ struct CmdProfileList : virtual EvalCommand, virtual StoreCommand, MixDefaultPro for (size_t i = 0; i < manifest.elements.size(); ++i) { auto & element(manifest.elements[i]); logger->cout("%d %s %s %s", i, - element.source ? element.source->originalRef.to_string() + "#" + element.source->attrPath : "-", - element.source ? element.source->resolvedRef.to_string() + "#" + element.source->attrPath : "-", + element.source ? element.source->originalRef.to_string() + "#" + element.source->attrPath + printOutputsSpec(element.source->outputs) : "-", + element.source ? element.source->resolvedRef.to_string() + "#" + element.source->attrPath + printOutputsSpec(element.source->outputs) : "-", concatStringsSep(" ", store->printStorePathSet(element.storePaths))); } } diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh index 814192252..f8da3d929 100644 --- a/tests/nix-profile.sh +++ b/tests/nix-profile.sh @@ -101,3 +101,22 @@ printf Utrecht > $flake1Dir/who nix profile install $flake1Dir [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello Utrecht" ]] [[ $(nix path-info --json $(realpath $TEST_HOME/.nix-profile/bin/hello) | jq -r .[].ca) =~ fixed:r:sha256: ]] + +# Override the outputs. +nix profile remove 0 1 +nix profile install "$flake1Dir^*" +[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello Utrecht" ]] +[ -e $TEST_HOME/.nix-profile/share/man ] +[ -e $TEST_HOME/.nix-profile/include ] + +printf Nix > $flake1Dir/who +nix profile upgrade 0 +[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello Nix" ]] +[ -e $TEST_HOME/.nix-profile/share/man ] +[ -e $TEST_HOME/.nix-profile/include ] + +nix profile remove 0 +nix profile install "$flake1Dir^man" +(! [ -e $TEST_HOME/.nix-profile/bin/hello ]) +[ -e $TEST_HOME/.nix-profile/share/man ] +(! [ -e $TEST_HOME/.nix-profile/include ]) From 1385b2007804c8a0370f2a6555045a00e34b07c7 Mon Sep 17 00:00:00 2001 From: Alain Zscheile Date: Wed, 4 May 2022 07:44:32 +0200 Subject: [PATCH 19/21] Get rid of most `.at` calls (#6393) Use one of `get` or `getOr` instead which will either return a null-pointer (with a nicer error message) or a default value when the key is missing. --- src/libcmd/installables.cc | 12 +- src/libexpr/flake/config.cc | 11 +- src/libexpr/flake/flakeref.cc | 6 +- src/libexpr/get-drvs.cc | 2 +- src/libexpr/primops.cc | 16 ++- src/libstore/build/derivation-goal.cc | 33 +++-- src/libstore/build/local-derivation-goal.cc | 115 ++++++++++-------- src/libstore/build/worker.cc | 5 +- src/libstore/builtins/fetchurl.cc | 2 +- src/libstore/derivations.cc | 13 +- src/libstore/derived-path.cc | 23 ++-- src/libstore/filetransfer.cc | 8 +- src/libstore/local-store.cc | 6 +- src/libstore/misc.cc | 9 +- src/libstore/remote-store.cc | 17 +-- src/libstore/store-api.cc | 2 +- src/libutil/experimental-features.cc | 4 +- src/libutil/tests/tests.cc | 20 ++- src/libutil/util.cc | 14 +++ src/libutil/util.hh | 27 +++- src/nix-build/nix-build.cc | 2 +- .../resolve-system-dependencies.cc | 2 +- 22 files changed, 231 insertions(+), 118 deletions(-) mode change 100755 => 100644 src/nix-build/nix-build.cc diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 55a5e91e9..a94e60aca 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -871,12 +871,13 @@ std::vector, BuiltPath>> Installable::bui auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive auto drvOutputs = drv.outputsAndOptPaths(*store); for (auto & output : bfd.outputs) { - if (!outputHashes.count(output)) + auto outputHash = get(outputHashes, output); + if (!outputHash) throw Error( "the derivation '%s' doesn't have an output named '%s'", store->printStorePath(bfd.drvPath), output); if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - DrvOutput outputId { outputHashes.at(output), output }; + DrvOutput outputId { *outputHash, output }; auto realisation = store->queryRealisation(outputId); if (!realisation) throw Error( @@ -887,10 +888,11 @@ std::vector, BuiltPath>> Installable::bui } else { // If ca-derivations isn't enabled, assume that // the output path is statically known. - assert(drvOutputs.count(output)); - assert(drvOutputs.at(output).second); + auto drvOutput = get(drvOutputs, output); + assert(drvOutput); + assert(drvOutput->second); outputs.insert_or_assign( - output, *drvOutputs.at(output).second); + output, *drvOutput->second); } } res.push_back({installable, BuiltPath::Built { bfd.drvPath, outputs }}); diff --git a/src/libexpr/flake/config.cc b/src/libexpr/flake/config.cc index a811e59a1..92ec27046 100644 --- a/src/libexpr/flake/config.cc +++ b/src/libexpr/flake/config.cc @@ -50,13 +50,11 @@ void ConfigFile::apply() else assert(false); - if (!whitelist.count(baseName)) { - auto trustedList = readTrustedList(); - + if (!whitelist.count(baseName) && !nix::fetchSettings.acceptFlakeConfig) { bool trusted = false; - if (nix::fetchSettings.acceptFlakeConfig){ - trusted = true; - } else if (auto saved = get(get(trustedList, name).value_or(std::map()), valueS)) { + auto trustedList = readTrustedList(); + auto tlname = get(trustedList, name); + if (auto saved = tlname ? get(*tlname, valueS) : nullptr) { trusted = *saved; warn("Using saved setting for '%s = %s' from ~/.local/share/nix/trusted-settings.json.", name,valueS); } else { @@ -69,7 +67,6 @@ void ConfigFile::apply() writeTrustedList(trustedList); } } - if (!trusted) { warn("ignoring untrusted flake configuration setting '%s'", name); continue; diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index 1dcc4555a..eede493f8 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -176,7 +176,7 @@ std::pair parseFlakeRefWithFragment( parsedURL.query.insert_or_assign("shallow", "1"); return std::make_pair( - FlakeRef(Input::fromURL(parsedURL), get(parsedURL.query, "dir").value_or("")), + FlakeRef(Input::fromURL(parsedURL), getOr(parsedURL.query, "dir", "")), fragment); } @@ -189,7 +189,7 @@ std::pair parseFlakeRefWithFragment( if (!hasPrefix(path, "/")) throw BadURL("flake reference '%s' is not an absolute path", url); auto query = decodeQuery(match[2]); - path = canonPath(path + "/" + get(query, "dir").value_or("")); + path = canonPath(path + "/" + getOr(query, "dir", "")); } fetchers::Attrs attrs; @@ -208,7 +208,7 @@ std::pair parseFlakeRefWithFragment( input.parent = baseDir; return std::make_pair( - FlakeRef(std::move(input), get(parsedURL.query, "dir").value_or("")), + FlakeRef(std::move(input), getOr(parsedURL.query, "dir", "")), fragment); } } diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 11f2b279d..d616b3921 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -34,7 +34,7 @@ DrvInfo::DrvInfo(EvalState & state, ref store, const std::string & drvPat outputName = selectedOutputs.empty() - ? get(drv.env, "outputName").value_or("out") + ? getOr(drv.env, "outputName", "out") : *selectedOutputs.begin(); auto i = drv.outputs.find(outputName); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 40e9f7091..a62d11e4e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -68,14 +68,15 @@ StringMap EvalState::realiseContext(const PathSet & context) /* Get all the output paths corresponding to the placeholders we had */ for (auto & [drvPath, outputs] : drvs) { - auto outputPaths = store->queryDerivationOutputMap(drvPath); + const auto outputPaths = store->queryDerivationOutputMap(drvPath); for (auto & outputName : outputs) { - if (outputPaths.count(outputName) == 0) + auto outputPath = get(outputPaths, outputName); + if (!outputPath) throw Error("derivation '%s' does not have an output named '%s'", store->printStorePath(drvPath), outputName); res.insert_or_assign( downstreamPlaceholder(*store, drvPath, outputName), - store->printStorePath(outputPaths.at(outputName)) + store->printStorePath(*outputPath) ); } } @@ -1249,8 +1250,13 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * * switch (hashModulo.kind) { case DrvHash::Kind::Regular: for (auto & i : outputs) { - auto h = hashModulo.hashes.at(i); - auto outPath = state.store->makeOutputPath(i, h, drvName); + auto h = get(hashModulo.hashes, i); + if (!h) + throw AssertionError({ + .msg = hintfmt("derivation produced no hash for output '%s'", i), + .errPos = state.positions[posDrvName], + }); + auto outPath = state.store->makeOutputPath(i, *h, drvName); drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign( i, diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d095a0f02..3fff2385f 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -984,21 +984,28 @@ void DerivationGoal::resolvedFinished() realWantedOutputs = resolvedDrv.outputNames(); for (auto & wantedOutput : realWantedOutputs) { - assert(initialOutputs.count(wantedOutput) != 0); - assert(resolvedHashes.count(wantedOutput) != 0); - auto realisation = resolvedResult.builtOutputs.at( - DrvOutput { resolvedHashes.at(wantedOutput), wantedOutput }); + auto initialOutput = get(initialOutputs, wantedOutput); + auto resolvedHash = get(resolvedHashes, wantedOutput); + if ((!initialOutput) || (!resolvedHash)) + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,resolve)", + worker.store.printStorePath(drvPath), wantedOutput); + auto realisation = get(resolvedResult.builtOutputs, DrvOutput { *resolvedHash, wantedOutput }); + if (!realisation) + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,realisation)", + worker.store.printStorePath(resolvedDrvGoal->drvPath), wantedOutput); if (drv->type().isPure()) { - auto newRealisation = realisation; - newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput }; + auto newRealisation = *realisation; + newRealisation.id = DrvOutput { initialOutput->outputHash, wantedOutput }; newRealisation.signatures.clear(); if (!drv->type().isFixed()) - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation->outPath); signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } - outputPaths.insert(realisation.outPath); - builtOutputs.emplace(realisation.id, realisation); + outputPaths.insert(realisation->outPath); + builtOutputs.emplace(realisation->id, *realisation); } runPostBuildHook( @@ -1294,7 +1301,11 @@ std::pair DerivationGoal::checkPathValidity() DrvOutputs validOutputs; for (auto & i : queryPartialDerivationOutputMap()) { - InitialOutput & info = initialOutputs.at(i.first); + auto initialOutput = get(initialOutputs, i.first); + if (!initialOutput) + // this is an invalid output, gets catched with (!wantedOutputsLeft.empty()) + continue; + auto & info = *initialOutput; info.wanted = wantOutput(i.first, wantedOutputs); if (info.wanted) wantedOutputsLeft.erase(i.first); @@ -1309,7 +1320,7 @@ std::pair DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } - auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; + auto drvOutput = DrvOutput{info.outputHash, i.first}; if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4c91fa4fb..7f44276bd 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -482,7 +482,7 @@ void LocalDerivationGoal::startBuilder() temporary build directory. The text files have the format used by `nix-store --register-validity'. However, the deriver fields are left empty. */ - auto s = get(drv->env, "exportReferencesGraph").value_or(""); + auto s = getOr(drv->env, "exportReferencesGraph", ""); Strings ss = tokenizeString(s); if (ss.size() % 2 != 0) throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s); @@ -989,7 +989,7 @@ void LocalDerivationGoal::initTmpDir() { there is no size constraint). */ if (!parsedDrv->getStructuredAttrs()) { - StringSet passAsFile = tokenizeString(get(drv->env, "passAsFile").value_or("")); + StringSet passAsFile = tokenizeString(getOr(drv->env, "passAsFile", "")); for (auto & i : drv->env) { if (passAsFile.find(i.first) == passAsFile.end()) { env[i.first] = i.second; @@ -2128,12 +2128,22 @@ DrvOutputs LocalDerivationGoal::registerOutputs() std::map> outputReferencesIfUnregistered; std::map outputStats; for (auto & [outputName, _] : drv->outputs) { - auto actualPath = toRealPathChroot(worker.store.printStorePath(scratchOutputs.at(outputName))); + auto scratchOutput = get(scratchOutputs, outputName); + if (!scratchOutput) + throw BuildError( + "builder for '%s' has no scratch output for '%s'", + worker.store.printStorePath(drvPath), outputName); + auto actualPath = toRealPathChroot(worker.store.printStorePath(*scratchOutput)); outputsToSort.insert(outputName); /* Updated wanted info to remove the outputs we definitely don't need to register */ - auto & initialInfo = initialOutputs.at(outputName); + auto initialOutput = get(initialOutputs, outputName); + if (!initialOutput) + throw BuildError( + "builder for '%s' has no initial output for '%s'", + worker.store.printStorePath(drvPath), outputName); + auto & initialInfo = *initialOutput; /* Don't register if already valid, and not checking */ initialInfo.wanted = buildMode == bmCheck @@ -2185,6 +2195,11 @@ DrvOutputs LocalDerivationGoal::registerOutputs() auto sortedOutputNames = topoSort(outputsToSort, {[&](const std::string & name) { + auto orifu = get(outputReferencesIfUnregistered, name); + if (!orifu) + throw BuildError( + "no output reference for '%s' in build of '%s'", + name, worker.store.printStorePath(drvPath)); return std::visit(overloaded { /* Since we'll use the already installed versions of these, we can treat them as leaves and ignore any references they @@ -2199,7 +2214,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() referencedOutputs.insert(o); return referencedOutputs; }, - }, outputReferencesIfUnregistered.at(name)); + }, *orifu); }}, {[&](const std::string & path, const std::string & parent) { // TODO with more -vvvv also show the temporary paths for manual inspection. @@ -2213,9 +2228,10 @@ DrvOutputs LocalDerivationGoal::registerOutputs() OutputPathMap finalOutputs; for (auto & outputName : sortedOutputNames) { - auto output = drv->outputs.at(outputName); - auto & scratchPath = scratchOutputs.at(outputName); - auto actualPath = toRealPathChroot(worker.store.printStorePath(scratchPath)); + auto output = get(drv->outputs, outputName); + auto scratchPath = get(scratchOutputs, outputName); + assert(output && scratchPath); + auto actualPath = toRealPathChroot(worker.store.printStorePath(*scratchPath)); auto finish = [&](StorePath finalStorePath) { /* Store the final path */ @@ -2223,10 +2239,13 @@ DrvOutputs LocalDerivationGoal::registerOutputs() /* The rewrite rule will be used in downstream outputs that refer to use. This is why the topological sort is essential to do first before this for loop. */ - if (scratchPath != finalStorePath) - outputRewrites[std::string { scratchPath.hashPart() }] = std::string { finalStorePath.hashPart() }; + if (*scratchPath != finalStorePath) + outputRewrites[std::string { scratchPath->hashPart() }] = std::string { finalStorePath.hashPart() }; }; + auto orifu = get(outputReferencesIfUnregistered, outputName); + assert(orifu); + std::optional referencesOpt = std::visit(overloaded { [&](const AlreadyRegistered & skippedFinalPath) -> std::optional { finish(skippedFinalPath.path); @@ -2235,7 +2254,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() [&](const PerhapsNeedToRegister & r) -> std::optional { return r.refs; }, - }, outputReferencesIfUnregistered.at(outputName)); + }, *orifu); if (!referencesOpt) continue; @@ -2268,25 +2287,29 @@ DrvOutputs LocalDerivationGoal::registerOutputs() for (auto & r : references) { auto name = r.name(); auto origHash = std::string { r.hashPart() }; - if (r == scratchPath) + if (r == *scratchPath) { res.first = true; - else if (outputRewrites.count(origHash) == 0) - res.second.insert(r); - else { - std::string newRef = outputRewrites.at(origHash); + } else if (auto outputRewrite = get(outputRewrites, origHash)) { + std::string newRef = *outputRewrite; newRef += '-'; newRef += name; res.second.insert(StorePath { newRef }); + } else { + res.second.insert(r); } } return res; }; auto newInfoFromCA = [&](const DerivationOutput::CAFloating outputHash) -> ValidPathInfo { - auto & st = outputStats.at(outputName); + auto st = get(outputStats, outputName); + if (!st) + throw BuildError( + "output path %1% without valid stats info", + actualPath); if (outputHash.method == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ - if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) + if (!S_ISREG(st->st_mode) || (st->st_mode & S_IXUSR) != 0) throw BuildError( "output path '%1%' should be a non-executable regular file " "since recursive hashing is not enabled (outputHashMode=flat)", @@ -2294,7 +2317,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() } rewriteOutput(); /* FIXME optimize and deduplicate with addToStore */ - std::string oldHashPart { scratchPath.hashPart() }; + std::string oldHashPart { scratchPath->hashPart() }; HashModuloSink caSink { outputHash.hashType, oldHashPart }; switch (outputHash.method) { case FileIngestionMethod::Recursive: @@ -2313,7 +2336,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() outputPathName(drv->name, outputName), refs.second, refs.first); - if (scratchPath != finalPath) { + if (*scratchPath != finalPath) { // Also rewrite the output path auto source = sinkToSource([&](Sink & nextSink) { StringSink sink; @@ -2354,9 +2377,9 @@ DrvOutputs LocalDerivationGoal::registerOutputs() auto requiredFinalPath = output.path; /* Preemptively add rewrite rule for final hash, as that is what the NAR hash will use rather than normalized-self references */ - if (scratchPath != requiredFinalPath) + if (*scratchPath != requiredFinalPath) outputRewrites.insert_or_assign( - std::string { scratchPath.hashPart() }, + std::string { scratchPath->hashPart() }, std::string { requiredFinalPath.hashPart() }); rewriteOutput(); auto narHashAndSize = hashPath(htSHA256, actualPath); @@ -2409,7 +2432,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() }); }, - }, output.raw()); + }, output->raw()); /* FIXME: set proper permissions in restorePath() so we don't have to do another traversal. */ @@ -2425,7 +2448,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() derivations. */ PathLocks dynamicOutputLock; dynamicOutputLock.setDeletion(true); - auto optFixedPath = output.path(worker.store, drv->name, outputName); + auto optFixedPath = output->path(worker.store, drv->name, outputName); if (!optFixedPath || worker.store.printStorePath(*optFixedPath) != finalDestPath) { @@ -2491,11 +2514,10 @@ DrvOutputs LocalDerivationGoal::registerOutputs() /* For debugging, print out the referenced and unreferenced paths. */ for (auto & i : inputPaths) { - auto j = references.find(i); - if (j == references.end()) - debug("unreferenced input: '%1%'", worker.store.printStorePath(i)); - else + if (references.count(i)) debug("referenced input: '%1%'", worker.store.printStorePath(i)); + else + debug("unreferenced input: '%1%'", worker.store.printStorePath(i)); } if (curRound == nrRounds) { @@ -2612,9 +2634,11 @@ DrvOutputs LocalDerivationGoal::registerOutputs() DrvOutputs builtOutputs; for (auto & [outputName, newInfo] : infos) { + auto oldinfo = get(initialOutputs, outputName); + assert(oldinfo); auto thisRealisation = Realisation { .id = DrvOutput { - initialOutputs.at(outputName).outputHash, + oldinfo->outputHash, outputName }, .outPath = newInfo.path @@ -2710,9 +2734,10 @@ void LocalDerivationGoal::checkOutputs(const std::mappath); + else + throw BuildError("derivation contains an illegal reference specifier '%s'", i); } auto used = recursive @@ -2751,24 +2776,18 @@ void LocalDerivationGoal::checkOutputs(const std::mapgetStructuredAttrs()) { - auto outputChecks = structuredAttrs->find("outputChecks"); - if (outputChecks != structuredAttrs->end()) { - auto output = outputChecks->find(outputName); - - if (output != outputChecks->end()) { + if (auto outputChecks = get(*structuredAttrs, "outputChecks")) { + if (auto output = get(*outputChecks, outputName)) { Checks checks; - auto maxSize = output->find("maxSize"); - if (maxSize != output->end()) + if (auto maxSize = get(*output, "maxSize")) checks.maxSize = maxSize->get(); - auto maxClosureSize = output->find("maxClosureSize"); - if (maxClosureSize != output->end()) + if (auto maxClosureSize = get(*output, "maxClosureSize")) checks.maxClosureSize = maxClosureSize->get(); - auto get = [&](const std::string & name) -> std::optional { - auto i = output->find(name); - if (i != output->end()) { + auto get_ = [&](const std::string & name) -> std::optional { + if (auto i = get(*output, name)) { Strings res; for (auto j = i->begin(); j != i->end(); ++j) { if (!j->is_string()) @@ -2781,10 +2800,10 @@ void LocalDerivationGoal::checkOutputs(const std::map fds2(j->fds); std::vector buffer(4096); for (auto & k : fds2) { - if (pollStatus.at(fdToPollStatus.at(k)).revents) { + const auto fdPollStatusId = get(fdToPollStatus, k); + assert(fdPollStatusId); + assert(*fdPollStatusId < pollStatus.size()); + if (pollStatus.at(*fdPollStatusId).revents) { ssize_t rd = ::read(k, buffer.data(), buffer.size()); // FIXME: is there a cleaner way to handle pt close // than EIO? Is this even standard? diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index af3dfc409..7d7924d77 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -24,7 +24,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) Path storePath = getAttr("out"); auto mainUrl = getAttr("url"); - bool unpack = get(drv.env, "unpack").value_or("") == "1"; + bool unpack = getOr(drv.env, "unpack", "") == "1"; /* Note: have to use a fresh fileTransfer here because we're in a forked process. */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 1c695de82..fe99c3c5e 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -661,8 +661,10 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut if (res.kind == DrvHash::Kind::Deferred) kind = DrvHash::Kind::Deferred; for (auto & outputName : inputOutputs) { - const auto h = res.hashes.at(outputName); - inputs2[h.to_string(Base16, false)].insert(outputName); + const auto h = get(res.hashes, outputName); + if (!h) + throw Error("no hash for output '%s' of derivation '%s'", outputName, drv.name); + inputs2[h->to_string(Base16, false)].insert(outputName); } } @@ -836,8 +838,11 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { if (std::holds_alternative(output.raw())) { - auto & h = hashModulo.hashes.at(outputName); - auto outPath = store.makeOutputPath(outputName, h, drv.name); + auto h = get(hashModulo.hashes, outputName); + if (!h) + throw Error("derivation '%s' output '%s' has no hash (derivations.cc/rewriteDerivation)", + drv.name, outputName); + auto outPath = store.makeOutputPath(outputName, *h, drv.name); drv.env[outputName] = store.printStorePath(outPath); output = DerivationOutput::InputAddressed { .path = std::move(outPath), diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 319b1c790..44587ae78 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -4,6 +4,8 @@ #include +#include + namespace nix { nlohmann::json DerivedPath::Opaque::toJSON(ref store) const { @@ -17,12 +19,12 @@ nlohmann::json DerivedPath::Built::toJSON(ref store) const { res["drvPath"] = store->printStorePath(drvPath); // Fallback for the input-addressed derivation case: We expect to always be // able to print the output paths, so let’s do it - auto knownOutputs = store->queryPartialDerivationOutputMap(drvPath); + const auto knownOutputs = store->queryPartialDerivationOutputMap(drvPath); for (const auto& output : outputs) { - if (knownOutputs.at(output)) - res["outputs"][output] = store->printStorePath(knownOutputs.at(output).value()); - else - res["outputs"][output] = nullptr; + auto knownOutput = get(knownOutputs, output); + res["outputs"][output] = (knownOutput && *knownOutput) + ? store->printStorePath(**knownOutput) + : nullptr; } return res; } @@ -123,10 +125,15 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const for (auto& [outputName, outputPath] : p.outputs) { if (settings.isExperimentalFeatureEnabled( Xp::CaDerivations)) { + auto drvOutput = get(drvHashes, outputName); + if (!drvOutput) + throw Error( + "the derivation '%s' has unrealised output '%s' (derived-path.cc/toRealisedPaths)", + store.printStorePath(p.drvPath), outputName); auto thisRealisation = store.queryRealisation( - DrvOutput{drvHashes.at(outputName), outputName}); - assert(thisRealisation); // We’ve built it, so we must h - // ve the realisation + DrvOutput{*drvOutput, outputName}); + assert(thisRealisation); // We’ve built it, so we must + // have the realisation res.insert(*thisRealisation); } else { res.insert(outputPath); diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 529a41891..8454ad7d2 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -692,10 +692,10 @@ struct curlFileTransfer : public FileTransfer #if ENABLE_S3 auto [bucketName, key, params] = parseS3Uri(request.uri); - std::string profile = get(params, "profile").value_or(""); - std::string region = get(params, "region").value_or(Aws::Region::US_EAST_1); - std::string scheme = get(params, "scheme").value_or(""); - std::string endpoint = get(params, "endpoint").value_or(""); + std::string profile = getOr(params, "profile", ""); + std::string region = getOr(params, "region", Aws::Region::US_EAST_1); + std::string scheme = getOr(params, "scheme", ""); + std::string endpoint = getOr(params, "endpoint", ""); S3Helper s3Helper(profile, region, scheme, endpoint); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 5cc5c91cc..eba3b0fa5 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -718,7 +718,11 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat // somewhat expensive so we do lazily hashesModulo = hashDerivationModulo(*this, drv, true); } - StorePath recomputed = makeOutputPath(i.first, hashesModulo->hashes.at(i.first), drvName); + auto currentOutputHash = get(hashesModulo->hashes, i.first); + if (!currentOutputHash) + throw Error("derivation '%s' has unexpected output '%s' (local-store / hashesModulo) named '%s'", + printStorePath(drvPath), printStorePath(doia.path), i.first); + StorePath recomputed = makeOutputPath(i.first, *currentOutputHash, drvName); if (doia.path != recomputed) throw Error("derivation '%s' has incorrect output '%s', should be '%s'", printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 2bbd7aa70..fb985c97b 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -278,11 +278,16 @@ std::map drvOutputReferences( std::set inputRealisations; for (const auto & [inputDrv, outputNames] : drv.inputDrvs) { - auto outputHashes = + const auto outputHashes = staticOutputHashes(store, store.readDerivation(inputDrv)); for (const auto & outputName : outputNames) { + auto outputHash = get(outputHashes, outputName); + if (!outputHash) + throw Error( + "output '%s' of derivation '%s' isn't realised", outputName, + store.printStorePath(inputDrv)); auto thisRealisation = store.queryRealisation( - DrvOutput{outputHashes.at(outputName), outputName}); + DrvOutput{*outputHash, outputName}); if (!thisRealisation) throw Error( "output '%s' of derivation '%s' isn't built", outputName, diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 347e32094..14aeba75c 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -853,15 +853,15 @@ std::vector RemoteStore::buildPathsWithResults( OutputPathMap outputs; auto drv = evalStore->readDerivation(bfd.drvPath); - auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive - auto drvOutputs = drv.outputsAndOptPaths(*this); + const auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive + const auto drvOutputs = drv.outputsAndOptPaths(*this); for (auto & output : bfd.outputs) { - if (!outputHashes.count(output)) + auto outputHash = get(outputHashes, output); + if (!outputHash) throw Error( "the derivation '%s' doesn't have an output named '%s'", printStorePath(bfd.drvPath), output); - auto outputId = - DrvOutput{outputHashes.at(output), output}; + auto outputId = DrvOutput{ *outputHash, output }; if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { auto realisation = queryRealisation(outputId); @@ -874,13 +874,14 @@ std::vector RemoteStore::buildPathsWithResults( } else { // If ca-derivations isn't enabled, assume that // the output path is statically known. - assert(drvOutputs.count(output)); - assert(drvOutputs.at(output).second); + const auto drvOutput = get(drvOutputs, output); + assert(drvOutput); + assert(drvOutput->second); res.builtOutputs.emplace( outputId, Realisation { .id = outputId, - .outPath = *drvOutputs.at(output).second + .outPath = *drvOutput->second, }); } } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 59937be4d..8861274a2 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1314,7 +1314,7 @@ static bool isNonUriPath(const std::string & spec) { std::shared_ptr openFromNonUri(const std::string & uri, const Store::Params & params) { if (uri == "" || uri == "auto") { - auto stateDir = get(params, "state").value_or(settings.nixStateDir); + auto stateDir = getOr(params, "state", settings.nixStateDir); if (access(stateDir.c_str(), R_OK | W_OK) == 0) return std::make_shared(params); else if (pathExists(settings.nixDaemonSocketFile)) diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 9326cd08c..315de64a4 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -35,7 +35,9 @@ const std::optional parseExperimentalFeature(const std::str std::string_view showExperimentalFeature(const ExperimentalFeature feature) { - return stringifiedXpFeatures.at(feature); + const auto ret = get(stringifiedXpFeatures, feature); + assert(ret); + return *ret; } std::set parseFeatures(const std::set & rawFeatures) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 92972ed14..6e325db98 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -548,7 +548,7 @@ namespace nix { TEST(get, emptyContainer) { StringMap s = { }; - auto expected = std::nullopt; + auto expected = nullptr; ASSERT_EQ(get(s, "one"), expected); } @@ -559,7 +559,23 @@ namespace nix { s["two"] = "er"; auto expected = "yi"; - ASSERT_EQ(get(s, "one"), expected); + ASSERT_EQ(*get(s, "one"), expected); + } + + TEST(getOr, emptyContainer) { + StringMap s = { }; + auto expected = "yi"; + + ASSERT_EQ(getOr(s, "one", "yi"), expected); + } + + TEST(getOr, getFromContainer) { + StringMap s; + s["one"] = "yi"; + s["two"] = "er"; + auto expected = "yi"; + + ASSERT_EQ(getOr(s, "one", "nope"), expected); } /* ---------------------------------------------------------------------------- diff --git a/src/libutil/util.cc b/src/libutil/util.cc index b49c1e466..8bc99f531 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1586,6 +1586,20 @@ std::string stripIndentation(std::string_view s) } +const nlohmann::json * get(const nlohmann::json & map, const std::string & key) +{ + auto i = map.find(key); + if (i == map.end()) return nullptr; + return &*i; +} + +nlohmann::json * get(nlohmann::json & map, const std::string & key) +{ + auto i = map.find(key); + if (i == map.end()) return nullptr; + return &*i; +} + ////////////////////////////////////////////////////////////////////// diff --git a/src/libutil/util.hh b/src/libutil/util.hh index a1d0e0e6b..6319ced47 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -543,13 +543,34 @@ std::string stripIndentation(std::string_view s); /* Get a value for the specified key from an associate container. */ template -std::optional get(const T & map, const typename T::key_type & key) +const typename T::mapped_type * get(const T & map, const typename T::key_type & key) { auto i = map.find(key); - if (i == map.end()) return {}; - return std::optional(i->second); + if (i == map.end()) return nullptr; + return &i->second; } +template +typename T::mapped_type * get(T & map, const typename T::key_type & key) +{ + auto i = map.find(key); + if (i == map.end()) return nullptr; + return &i->second; +} + +const nlohmann::json * get(const nlohmann::json & map, const std::string & key); +nlohmann::json * get(nlohmann::json & map, const std::string & key); + +/* Get a value for the specified key from an associate container, or a default value if the key isn't present. */ +template +const typename T::mapped_type & getOr(T & map, + const typename T::key_type & key, + const typename T::mapped_type & defaultValue) +{ + auto i = map.find(key); + if (i == map.end()) return defaultValue; + return i->second; +} /* Remove and return the first item from a container. */ template diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc old mode 100755 new mode 100644 index faa8c078f..519855ea3 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -440,7 +440,7 @@ static void main_nix_build(int argc, char * * argv) env["NIX_STORE"] = store->storeDir; env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores); - auto passAsFile = tokenizeString(get(drv.env, "passAsFile").value_or("")); + auto passAsFile = tokenizeString(getOr(drv.env, "passAsFile", "")); bool keepTmp = false; int fileNr = 0; diff --git a/src/resolve-system-dependencies/resolve-system-dependencies.cc b/src/resolve-system-dependencies/resolve-system-dependencies.cc index 4dd691981..c6023eb03 100644 --- a/src/resolve-system-dependencies/resolve-system-dependencies.cc +++ b/src/resolve-system-dependencies/resolve-system-dependencies.cc @@ -176,7 +176,7 @@ int main(int argc, char ** argv) impurePaths.insert(argv[2]); else { auto drv = store->derivationFromPath(store->parseStorePath(argv[1])); - impurePaths = tokenizeString(get(drv.env, "__impureHostDeps").value_or("")); + impurePaths = tokenizeString(getOr(drv.env, "__impureHostDeps", "")); impurePaths.insert("/usr/lib/libSystem.dylib"); } From 3e87c8e62b3eacecde4e9f467b5463fbbdaa6a3f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 4 May 2022 11:22:06 +0200 Subject: [PATCH 20/21] Move json stuff out of util.cc --- src/libstore/build/local-derivation-goal.cc | 3 +-- src/libutil/json-utils.hh | 21 +++++++++++++++++++++ src/libutil/util.cc | 15 --------------- src/libutil/util.hh | 3 --- 4 files changed, 22 insertions(+), 20 deletions(-) create mode 100644 src/libutil/json-utils.hh diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 7f44276bd..3ac9c20f9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -14,6 +14,7 @@ #include "worker-protocol.hh" #include "topo-sort.hh" #include "callback.hh" +#include "json-utils.hh" #include #include @@ -56,8 +57,6 @@ #include #include -#include - namespace nix { void handleDiffHook( diff --git a/src/libutil/json-utils.hh b/src/libutil/json-utils.hh new file mode 100644 index 000000000..b8a031227 --- /dev/null +++ b/src/libutil/json-utils.hh @@ -0,0 +1,21 @@ +#pragma once + +#include + +namespace nix { + +const nlohmann::json * get(const nlohmann::json & map, const std::string & key) +{ + auto i = map.find(key); + if (i == map.end()) return nullptr; + return &*i; +} + +nlohmann::json * get(nlohmann::json & map, const std::string & key) +{ + auto i = map.find(key); + if (i == map.end()) return nullptr; + return &*i; +} + +} diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 8bc99f531..d4d78329d 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1586,23 +1586,8 @@ std::string stripIndentation(std::string_view s) } -const nlohmann::json * get(const nlohmann::json & map, const std::string & key) -{ - auto i = map.find(key); - if (i == map.end()) return nullptr; - return &*i; -} - -nlohmann::json * get(nlohmann::json & map, const std::string & key) -{ - auto i = map.find(key); - if (i == map.end()) return nullptr; - return &*i; -} - ////////////////////////////////////////////////////////////////////// - static Sync> windowSize{{0, 0}}; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 6319ced47..09ccfa591 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -558,9 +558,6 @@ typename T::mapped_type * get(T & map, const typename T::key_type & key) return &i->second; } -const nlohmann::json * get(const nlohmann::json & map, const std::string & key); -nlohmann::json * get(nlohmann::json & map, const std::string & key); - /* Get a value for the specified key from an associate container, or a default value if the key isn't present. */ template const typename T::mapped_type & getOr(T & map, From 107613ad2b2b61ef92edf9ee53ec71ad664be71b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 4 May 2022 11:31:39 +0200 Subject: [PATCH 21/21] Fix compiler warning --- src/libstore/build/worker.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index e7c01a737..b192fbc77 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -350,7 +350,7 @@ void Worker::waitForInput() become `available'. Note that `available' (i.e., non-blocking) includes EOF. */ std::vector pollStatus; - std::map fdToPollStatus; + std::map fdToPollStatus; for (auto & i : children) { for (auto & j : i.fds) { pollStatus.push_back((struct pollfd) { .fd = j, .events = POLLIN });