Skip to content

Commit

Permalink
Stratify ExtraPathInfo along Installable hierarchy
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 Mar 24, 2023
1 parent e00abd3 commit cf8d487
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 56 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
4 changes: 4 additions & 0 deletions src/libcmd/installable-attr-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
.drvPath = drvPath,
.outputs = outputs,
},
.info = make_ref<ExtraPathInfoValue>(ExtraPathInfoValue::Value {
/* FIXME: reconsider backwards compatibility above
so we can fill in this info. */
}),
});

return res;
Expand Down
5 changes: 4 additions & 1 deletion src/libcmd/installable-derived-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ std::string InstallableDerivedPath::what() const

DerivedPathsWithInfo InstallableDerivedPath::toDerivedPaths()
{
return {{.path = derivedPath, .info = {} }};
return {{
.path = derivedPath,
.info = make_ref<ExtraPathInfo>(),
}};
}

std::optional<StorePath> InstallableDerivedPath::getStorePath()
Expand Down
26 changes: 16 additions & 10 deletions src/libcmd/installable-flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
return {{
.path = DerivedPath::Opaque {
.path = std::move(storePath),
}
},
.info = make_ref<ExtraPathInfo>(),
}};
}

Expand All @@ -113,7 +114,8 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
return {{
.path = DerivedPath::Opaque {
.path = std::move(*storePath),
}
},
.info = make_ref<ExtraPathInfo>(),
}};
} else
throw Error("flake output attribute '%s' evaluates to the string '%s' which is not a store path", attrPath, s);
Expand Down Expand Up @@ -160,13 +162,16 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
},
}, extendedOutputsSpec.raw()),
},
.info = {
.priority = priority,
.originalRef = flakeRef,
.resolvedRef = getLockedFlake()->flake.lockedRef,
.attrPath = attrPath,
.extendedOutputsSpec = extendedOutputsSpec,
}
.info = make_ref<ExtraPathInfoFlake>(
ExtraPathInfoValue::Value {
.priority = priority,
.attrPath = attrPath,
.extendedOutputsSpec = extendedOutputsSpec,
},
ExtraPathInfoFlake::Flake {
.originalRef = flakeRef,
.resolvedRef = getLockedFlake()->flake.lockedRef,
}),
}};
}

Expand Down Expand Up @@ -212,6 +217,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 @@ -229,7 +235,7 @@ FlakeRef InstallableFlake::nixpkgsFlakeRef() const
}
}

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

}
14 changes: 14 additions & 0 deletions src/libcmd/installable-flake.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@

namespace nix {

struct ExtraPathInfoFlake : ExtraPathInfoValue
{
struct Flake {
FlakeRef originalRef;
FlakeRef resolvedRef;
};

Flake flake;

ExtraPathInfoFlake(Value && v, Flake && f)
: ExtraPathInfoValue(std::move(v)), flake(f)
{ }
};

struct InstallableFlake : InstallableValue
{
FlakeRef flakeRef;
Expand Down
31 changes: 31 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,32 @@ struct UnresolvedApp
App resolve(ref<Store> evalStore, ref<Store> store);
};

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

Value value;

ExtraPathInfoValue(Value && v)
: value(v)
{ }

virtual ~ExtraPathInfoValue() = default;
};

struct InstallableValue : Installable
{
ref<EvalState> state;

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

virtual ~InstallableValue() { }

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 +63,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
2 changes: 1 addition & 1 deletion src/libcmd/installables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build

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

Expand Down
25 changes: 7 additions & 18 deletions src/libcmd/installables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,14 @@
#include "path.hh"
#include "outputs-spec.hh"
#include "derived-path.hh"
#include "eval.hh"
#include "store-api.hh"
#include "flake/flake.hh"
#include "build-result.hh"

#include <optional>

namespace nix {

struct DrvInfo;
struct SourceExprCommand;

namespace eval_cache { class EvalCache; class AttrCursor; }

enum class Realise {
/* Build the derivation. Postcondition: the
Expand All @@ -39,28 +34,27 @@ enum class OperateOn {
Derivation
};

/**
* Extra info about a derived class. Will be sub-classed by the
* subclasses of Installable to provide more info.
*/
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;
virtual ~ExtraPathInfo() = default;
};

/* A derived path with any additional info that commands might
need from the derivation. */
struct DerivedPathWithInfo
{
DerivedPath path;
ExtraPathInfo info;
ref<ExtraPathInfo> info;
};

struct BuiltPathWithResult
{
BuiltPath path;
ExtraPathInfo info;
ref<ExtraPathInfo> info;
std::optional<BuildResult> result;
};

Expand All @@ -86,11 +80,6 @@ struct Installable
return {};
}

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

static std::vector<BuiltPathWithResult> build(
ref<Store> evalStore,
ref<Store> store,
Expand Down
1 change: 0 additions & 1 deletion src/nix/build.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#include "eval.hh"
#include "command.hh"
#include "common-args.hh"
#include "shared.hh"
Expand Down
1 change: 1 addition & 0 deletions src/nix/bundle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "store-api.hh"
#include "local-fs-store.hh"
#include "fs-accessor.hh"
#include "eval-inline.hh"

using namespace nix;

Expand Down
12 changes: 6 additions & 6 deletions src/nix/develop.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "eval.hh"
#include "command.hh"
#include "installable-flake.hh"
#include "command-installable-value.hh"
#include "common-args.hh"
#include "shared.hh"
#include "store-api.hh"
Expand Down Expand Up @@ -252,7 +252,7 @@ static StorePath getDerivationEnvironment(ref<Store> store, ref<Store> evalStore
throw Error("get-env.sh failed to produce an environment");
}

struct Common : InstallableCommand, MixProfile
struct Common : InstallableValueCommand, MixProfile
{
std::set<std::string> ignoreVars{
"BASHOPTS",
Expand Down Expand Up @@ -374,7 +374,7 @@ struct Common : InstallableCommand, MixProfile
return res;
}

StorePath getShellOutPath(ref<Store> store, ref<Installable> installable)
StorePath getShellOutPath(ref<Store> store, ref<InstallableValue> installable)
{
auto path = installable->getStorePath();
if (path && hasSuffix(path->to_string(), "-env"))
Expand All @@ -393,7 +393,7 @@ struct Common : InstallableCommand, MixProfile
}

std::pair<BuildEnvironment, std::string>
getBuildEnvironment(ref<Store> store, ref<Installable> installable)
getBuildEnvironment(ref<Store> store, ref<InstallableValue> installable)
{
auto shellOutPath = getShellOutPath(store, installable);

Expand Down Expand Up @@ -481,7 +481,7 @@ struct CmdDevelop : Common, MixEnvironment
;
}

void run(ref<Store> store, ref<Installable> installable) override
void run(ref<Store> store, ref<InstallableValue> installable) override
{
auto [buildEnvironment, gcroot] = getBuildEnvironment(store, installable);

Expand Down Expand Up @@ -605,7 +605,7 @@ struct CmdPrintDevEnv : Common, MixJSON

Category category() override { return catUtility; }

void run(ref<Store> store, ref<Installable> installable) override
void run(ref<Store> store, ref<InstallableValue> installable) override
{
auto buildEnvironment = getBuildEnvironment(store, installable).first;

Expand Down
Loading

0 comments on commit cf8d487

Please sign in to comment.