Skip to content

Commit

Permalink
Associate each derivation with a single ExtraPathInfo
Browse files Browse the repository at this point in the history
Do this as opposed to associating it with each `DerivedPath` produced by
the installable. (Installables can produce multiple). `getExtraPathInfo`
is a new method which gets instead.

Additionally, the new `getExtraPathInfo`, and `nixpkgsFlakeRef`, are moved
to `InstallableValue`.

To my pleasant surprise, the extra path infos associated with each
`DerivedPath` of a given installable were all the same, so this was an
easy refactor with no behavior change.

I did this because NixOS/rfcs#134 ; with these
things moved to `InstallableValue`, the base `Installable` no longer
depends on libexpr! This is a major step towards that.
  • Loading branch information
Ericson2314 committed Feb 21, 2023
1 parent 403bf9d commit 9160e5c
Show file tree
Hide file tree
Showing 15 changed files with 110 additions and 99 deletions.
2 changes: 1 addition & 1 deletion src/libcmd/command.hh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include "installables.hh"
#include "installable-value.hh"
#include "args.hh"
#include "common-eval-args.hh"
#include "path.hh"
Expand Down
11 changes: 8 additions & 3 deletions src/libcmd/installable-attr-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ std::pair<Value *, PosIdx> InstallableAttrPath::toValue(EvalState & state)
return {vRes, pos};
}

DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
DerivedPaths InstallableAttrPath::toDerivedPaths()
{
auto v = toValue(*state).first;

Expand Down Expand Up @@ -80,10 +80,10 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
iter->second = iter->second.union_(newOutputs);
}

DerivedPathsWithInfo res;
DerivedPaths res;
for (auto & [drvPath, outputs] : byDrvPath)
res.push_back({
.path = DerivedPath::Built {
DerivedPath::Built {
.drvPath = drvPath,
.outputs = outputs,
},
Expand All @@ -92,6 +92,11 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
return res;
}

ExtraPathInfo InstallableAttrPath::getExtraPathInfo()
{
return {};
}

InstallableAttrPath InstallableAttrPath::parse(
ref<EvalState> state,
SourceExprCommand & cmd,
Expand Down
4 changes: 3 additions & 1 deletion src/libcmd/installable-attr-path.hh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ class InstallableAttrPath : public InstallableValue

std::pair<Value *, PosIdx> toValue(EvalState & state) override;

DerivedPathsWithInfo toDerivedPaths() override;
DerivedPaths toDerivedPaths() override;

ExtraPathInfo getExtraPathInfo() override;

public:

Expand Down
4 changes: 2 additions & 2 deletions src/libcmd/installable-derived-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ std::string InstallableDerivedPath::what() const
return derivedPath.to_string(*store);
}

DerivedPathsWithInfo InstallableDerivedPath::toDerivedPaths()
DerivedPaths InstallableDerivedPath::toDerivedPaths()
{
return {{.path = derivedPath, .info = {} }};
return {derivedPath};
}

std::optional<StorePath> InstallableDerivedPath::getStorePath()
Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/installable-derived-path.hh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ struct InstallableDerivedPath : Installable

std::string what() const override;

DerivedPathsWithInfo toDerivedPaths() override;
DerivedPaths toDerivedPaths() override;

std::optional<StorePath> getStorePath() override;

Expand Down
63 changes: 38 additions & 25 deletions src/libcmd/installable-flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ InstallableFlake::InstallableFlake(
throw UsageError("'--arg' and '--argstr' are incompatible with flakes");
}

DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
DerivedPaths InstallableFlake::toDerivedPaths()
{
Activity act(*logger, lvlTalkative, actUnknown, fmt("evaluating derivation '%s'", what()));

Expand All @@ -98,23 +98,23 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
if (v.type() == nPath) {
PathSet context;
auto storePath = state->copyPathToStore(context, Path(v.path));
return {{
.path = DerivedPath::Opaque {
return {
DerivedPath::Opaque {
.path = std::move(storePath),
}
}};
};
}

else if (v.type() == nString) {
PathSet context;
auto s = state->forceString(v, context, noPos, fmt("while evaluating the flake output attribute '%s'", attrPath));
auto storePath = state->store->maybeParseStorePath(s);
if (storePath && context.count(std::string(s))) {
return {{
.path = DerivedPath::Opaque {
return {
DerivedPath::Opaque {
.path = std::move(*storePath),
}
}};
};
} else
throw Error("flake output attribute '%s' evaluates to the string '%s' which is not a store path", attrPath, s);
}
Expand All @@ -125,16 +125,12 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()

auto drvPath = attr->forceDerivation();

std::optional<NixInt> priority;

if (attr->maybeGetAttr(state->sOutputSpecified)) {
} else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
if (auto aPriority = aMeta->maybeGetAttr("priority"))
priority = aPriority->getInt();
}
// Do this for side effects, like setting `applyNixConfig`.
// FIXME do this differently.
getLockedFlake();

return {{
.path = DerivedPath::Built {
return {
DerivedPath::Built {
.drvPath = std::move(drvPath),
.outputs = std::visit(overloaded {
[&](const ExtendedOutputsSpec::Default & d) -> OutputsSpec {
Expand All @@ -160,14 +156,30 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
},
}, extendedOutputsSpec.raw()),
},
.info = {
.priority = priority,
.originalRef = flakeRef,
.resolvedRef = getLockedFlake()->flake.lockedRef,
.attrPath = attrPath,
.extendedOutputsSpec = extendedOutputsSpec,
}
}};
};
}

ExtraPathInfo InstallableFlake::getExtraPathInfo()
{
auto attr = getCursor(*state);

auto attrPath = attr->getAttrPathStr();

std::optional<NixInt> priority;

if (attr->maybeGetAttr(state->sOutputSpecified)) {
} else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
if (auto aPriority = aMeta->maybeGetAttr("priority"))
priority = aPriority->getInt();
}

return {
.priority = priority,
.originalRef = flakeRef,
.resolvedRef = getLockedFlake()->flake.lockedRef,
.attrPath = attrPath,
.extendedOutputsSpec = extendedOutputsSpec,
};
}

std::pair<Value *, PosIdx> InstallableFlake::toValue(EvalState & state)
Expand Down Expand Up @@ -213,6 +225,7 @@ std::shared_ptr<flake::LockedFlake> InstallableFlake::getLockedFlake() const
{
if (!_lockedFlake) {
flake::LockFlags lockFlagsApplyConfig = lockFlags;
// FIXME why this side effect?
lockFlagsApplyConfig.applyNixConfig = true;
_lockedFlake = std::make_shared<flake::LockedFlake>(lockFlake(*state, flakeRef, lockFlagsApplyConfig));
}
Expand All @@ -230,7 +243,7 @@ FlakeRef InstallableFlake::nixpkgsFlakeRef() const
}
}

return Installable::nixpkgsFlakeRef();
return InstallableValue::nixpkgsFlakeRef();
}

}
4 changes: 3 additions & 1 deletion src/libcmd/installable-flake.hh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ struct InstallableFlake : InstallableValue

Value * getFlakeOutputs(EvalState & state, const flake::LockedFlake & lockedFlake);

DerivedPathsWithInfo toDerivedPaths() override;
DerivedPaths toDerivedPaths() override;

ExtraPathInfo getExtraPathInfo() override;

std::pair<Value *, PosIdx> toValue(EvalState & state) override;

Expand Down
25 changes: 25 additions & 0 deletions src/libcmd/installable-value.hh
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
#pragma once

#include "installables.hh"
#include "flake/flake.hh"

namespace nix {

struct DrvInfo;
struct SourceExprCommand;

namespace eval_cache { class EvalCache; class AttrCursor; }

struct App
{
std::vector<DerivedPath> context;
Expand All @@ -17,12 +23,26 @@ struct UnresolvedApp
App resolve(ref<Store> evalStore, ref<Store> store);
};

struct ExtraPathInfo
{
std::optional<NixInt> priority;
std::optional<FlakeRef> originalRef;
std::optional<FlakeRef> resolvedRef;
std::optional<std::string> attrPath;
// FIXME: merge with DerivedPath's 'outputs' field?
std::optional<ExtendedOutputsSpec> extendedOutputsSpec;
};

struct InstallableValue : Installable
{
ref<EvalState> state;

InstallableValue(ref<EvalState> state) : state(state) {}

virtual ~InstallableValue() { }

virtual ExtraPathInfo getExtraPathInfo() = 0;

virtual std::pair<Value *, PosIdx> toValue(EvalState & state) = 0;

/* Get a cursor to each value this Installable could refer to. However
Expand All @@ -37,6 +57,11 @@ struct InstallableValue : Installable

UnresolvedApp toApp(EvalState & state);

virtual FlakeRef nixpkgsFlakeRef() const
{
return FlakeRef::fromAttrs({{"type","indirect"}, {"id", "nixpkgs"}});
}

static InstallableValue & require(Installable & installable);
static ref<InstallableValue> require(ref<Installable> installable);
};
Expand Down
34 changes: 13 additions & 21 deletions src/libcmd/installables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ void completeFlakeRef(ref<Store> store, std::string_view prefix)
}
}

DerivedPathWithInfo Installable::toDerivedPath()
DerivedPath Installable::toDerivedPath()
{
auto buildables = toDerivedPaths();
if (buildables.size() != 1)
Expand Down Expand Up @@ -515,19 +515,13 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
if (mode == Realise::Nothing)
settings.readOnlyMode = true;

struct Aux
{
ExtraPathInfo info;
ref<Installable> installable;
};

std::vector<DerivedPath> pathsToBuild;
std::map<DerivedPath, std::vector<Aux>> backmap;
std::map<DerivedPath, std::vector<ref<Installable>>> backmap;

for (auto & i : installables) {
for (auto b : i->toDerivedPaths()) {
pathsToBuild.push_back(b.path);
backmap[b.path].push_back({.info = b.info, .installable = i});
pathsToBuild.push_back(b);
backmap[b].push_back(i);
}
}

Expand All @@ -540,18 +534,18 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
printMissing(store, pathsToBuild, lvlError);

for (auto & path : pathsToBuild) {
for (auto & aux : backmap[path]) {
for (auto & installable : backmap[path]) {
std::visit(overloaded {
[&](const DerivedPath::Built & bfd) {
auto outputs = resolveDerivedPath(*store, bfd, &*evalStore);
res.push_back({aux.installable, {
res.push_back({installable, {
.path = BuiltPath::Built { bfd.drvPath, outputs },
.info = aux.info}});
}});
},
[&](const DerivedPath::Opaque & bo) {
res.push_back({aux.installable, {
res.push_back({installable, {
.path = BuiltPath::Opaque { bo.path },
.info = aux.info}});
}});
},
}, path.raw());
}
Expand All @@ -567,21 +561,19 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
if (!buildResult.success())
buildResult.rethrow();

for (auto & aux : backmap[buildResult.path]) {
for (auto & installable : backmap[buildResult.path]) {
std::visit(overloaded {
[&](const DerivedPath::Built & bfd) {
std::map<std::string, StorePath> outputs;
for (auto & path : buildResult.builtOutputs)
outputs.emplace(path.first.outputName, path.second.outPath);
res.push_back({aux.installable, {
res.push_back({installable, {
.path = BuiltPath::Built { bfd.drvPath, outputs },
.info = aux.info,
.result = buildResult}});
},
[&](const DerivedPath::Opaque & bo) {
res.push_back({aux.installable, {
res.push_back({installable, {
.path = BuiltPath::Opaque { bo.path },
.info = aux.info,
.result = buildResult}});
},
}, buildResult.path.raw());
Expand Down Expand Up @@ -667,7 +659,7 @@ StorePathSet Installable::toDerivations(
[&](const DerivedPath::Built & bfd) {
drvPaths.insert(bfd.drvPath);
},
}, b.path.raw());
}, b.raw());

return drvPaths;
}
Expand Down
Loading

0 comments on commit 9160e5c

Please sign in to comment.