Skip to content

Commit

Permalink
nix: Respect meta.outputsToInstall, and use all outputs by default
Browse files Browse the repository at this point in the history
'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.
  • Loading branch information
edolstra committed Apr 26, 2022
1 parent a81622c commit 1ddabe1
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 27 deletions.
33 changes: 26 additions & 7 deletions src/libcmd/installables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -497,7 +495,13 @@ std::vector<InstallableValue::DerivationInfo> InstallableAttrPath::toDerivations
auto drvPath = drvInfo.queryDrvPath();
if (!drvPath)
throw Error("'%s' is not a derivation", what());
res.push_back({ *drvPath, drvInfo.queryOutputName() });
std::set<std::string> outputsToInstall;
for (auto & output : drvInfo.queryOutputs(false, true))
outputsToInstall.insert(output.first);
res.push_back(DerivationInfo {
.drvPath = *drvPath,
.outputsToInstall = std::move(outputsToInstall)
});
}

return res;
Expand Down Expand Up @@ -598,9 +602,24 @@ std::tuple<std::string, FlakeRef, InstallableValue::DerivationInfo> InstallableF

auto drvPath = attr->forceDerivation();

std::set<std::string> 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");

This comment has been minimized.

Copy link
@roberth

roberth Jun 3, 2024

Member

This seems wrong, and still works like this on master, see #10825 (comment).
Nix never used to default to "out" but rather to the default output, defined as the first output.
But even then, should we even default to anything when the package author has declared that nothing should be installed?

(and we should be manipulating "derivable" paths and/or output paths, not derivations, but that's orthogonal to the default semantics question)


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)};
Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/installables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ struct InstallableValue : Installable
struct DerivationInfo
{
StorePath drvPath;
std::string outputName;
std::set<std::string> outputsToInstall;
};

virtual std::vector<DerivationInfo> toDerivations() = 0;
Expand Down
54 changes: 53 additions & 1 deletion src/libexpr/eval-cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,24 @@ struct AttrDb
});
}

AttrId setListOfStrings(
AttrKey key,
const std::vector<std::string> & 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([&]()
Expand Down Expand Up @@ -269,6 +287,8 @@ struct AttrDb
}
case AttrType::Bool:
return {{rowId, queryAttribute.getInt(2) != 0}};
case AttrType::ListOfStrings:
return {{rowId, tokenizeString<std::vector<std::string>>(queryAttribute.getStr(2), "\t")}};
case AttrType::Missing:
return {{rowId, missing_t()}};
case AttrType::Misc:
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -601,6 +621,38 @@ bool AttrCursor::getBool()
return v.boolean;
}

std::vector<std::string> AttrCursor::getListOfStrings()
{
if (root->db) {
if (!cachedValue)
cachedValue = root->db->getAttr(getKey());
if (cachedValue && !std::get_if<placeholder_t>(&cachedValue->second)) {
if (auto l = std::get_if<std::vector<std::string>>(&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<std::string> 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<Symbol> AttrCursor::getAttrs()
{
if (root->db) {
Expand Down
6 changes: 5 additions & 1 deletion src/libexpr/eval-cache.hh
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ enum AttrType {
Misc = 4,
Failed = 5,
Bool = 6,
ListOfStrings = 7,
};

struct placeholder_t {};
Expand All @@ -61,7 +62,8 @@ typedef std::variant<
missing_t,
misc_t,
failed_t,
bool
bool,
std::vector<std::string>
> AttrValue;

class AttrCursor : public std::enable_shared_from_this<AttrCursor>
Expand Down Expand Up @@ -114,6 +116,8 @@ public:

bool getBool();

std::vector<std::string> getListOfStrings();

std::vector<Symbol> getAttrs();

bool isDerivation();
Expand Down
2 changes: 1 addition & 1 deletion src/nix/app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1015,8 +1015,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
auto name = visitor.getAttr(state->sName)->getString();
if (json) {
std::optional<std::string> 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");
Expand Down
6 changes: 3 additions & 3 deletions src/nix/search.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 2 additions & 9 deletions tests/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/ca/content-addressed.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
3 changes: 2 additions & 1 deletion tests/ca/substitute.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1ddabe1

Please sign in to comment.