From 1ddabe1a0120787ff5bbdba5383222a6eb59c219 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 20 Apr 2022 16:39:47 +0200 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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";