Skip to content

Commit

Permalink
fetchTree: Return a path instead of a store path
Browse files Browse the repository at this point in the history
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: Robert Hensing <robert@roberthensing.nl>

fixup: remove FlakeCache

Make path values lazy

This fixes the double copy problem and improves performance
for expressions that don't force the whole source to be added to the
store.

Rules for fast expressions:

- Use path literals where possible
   - import ./foo.nix
- Use + operator with slash in string
   - src = fetchTree foo + "/src";
- Use source filtering, lib.fileset

- AVOID toString
- If possible, AVOID interpolations ("${./.}")
- If possible, move slashes into the interpolation to add less to the store
   - "${./src}/foo" -> "${./src/foo}"

toString may be improved later as part of lazy-trees, so these
recommendations are a snapshot. Path values are quite nice though.

SourceAccessor: insert colon after prefix

This allows clever editors/IDEs to discern the path more easily
for Ctrl+Click navigate to functionality, e.g. when building
.?ref=HEAD

Fix evalState::rootFS paths' to_string()

This showPath is getting a little too ad hoc, but it works for now.

Fix nix flake init eval for path value

WIP fix baseNameOf (needs test maybe)

NixOS#10252 (comment)

fix findFile assertion failure

A string is only allowed to create one path component; containing
no slashes.

tests: nix:derivation-internal.nix renders with a scheme now

fixup: Re-enable import .drv code
  • Loading branch information
tomberek committed Nov 2, 2024
1 parent 55fe4ee commit 2334f5f
Show file tree
Hide file tree
Showing 21 changed files with 270 additions and 159 deletions.
11 changes: 5 additions & 6 deletions src/libcmd/common-eval-args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ EvalSettings evalSettings {
// FIXME `parseFlakeRef` should take a `std::string_view`.
auto flakeRef = parseFlakeRef(fetchSettings, std::string { rest }, {}, true, false);
debug("fetching flake search path element '%s''", rest);
auto storePath = flakeRef.resolve(store).fetchTree(store).first;
return store->toRealPath(storePath);
return flakeRef.resolve(store).lazyFetch(store).first;
},
},
},
Expand Down Expand Up @@ -176,15 +175,15 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas
state.store,
state.fetchSettings,
EvalSettings::resolvePseudoUrl(s));
auto storePath = fetchToStore(*state.store, SourcePath(accessor), FetchMode::Copy);
return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
state.registerAccessor(accessor);
return SourcePath(accessor);
}

else if (hasPrefix(s, "flake:")) {
experimentalFeatureSettings.require(Xp::Flakes);
auto flakeRef = parseFlakeRef(fetchSettings, std::string(s.substr(6)), {}, true, false);
auto storePath = flakeRef.resolve(state.store).fetchTree(state.store).first;
return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
auto accessor = flakeRef.resolve(state.store).lazyFetch(state.store).first;
return SourcePath(accessor);
}

else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') {
Expand Down
3 changes: 2 additions & 1 deletion src/libexpr/eval-settings.hh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace nix {

class Store;
struct SourcePath;

struct EvalSettings : Config
{
Expand All @@ -22,7 +23,7 @@ struct EvalSettings : Config
* @todo Return (`std::optional` of) `SourceAccssor` or something
* more structured instead of mere `std::string`?
*/
using LookupPathHook = std::optional<std::string>(ref<Store> store, std::string_view);
using LookupPathHook = std::optional<SourcePath>(ref<Store> store, std::string_view);

/**
* Map from "scheme" to a `LookupPathHook`.
Expand Down
66 changes: 50 additions & 16 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ EvalState::EvalState(
{
corepkgsFS->setPathDisplay("<nix", ">");
internalFS->setPathDisplay("«nix-internal»", "");
rootFS->setPathDisplay("/", "");

countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0";

Expand Down Expand Up @@ -842,6 +843,11 @@ void Value::mkPath(const SourcePath & path)
mkPath(&*path.accessor, makeImmutableString(path.path.abs()));
}

void EvalState::registerAccessor(const ref<SourceAccessor> accessor)
{
sourceAccessors.push_back(accessor);
}


inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)
{
Expand Down Expand Up @@ -1937,10 +1943,22 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * const * lists, co
v.mkList(list);
}

// FIXME limit recursion
Value * resolveOutPath(EvalState & state, Value * v, const PosIdx pos)
{
state.forceValue(*v, pos);
if (v->type() != nAttrs)
return v;
auto found = v->attrs()->find(state.sOutPath);
if (found != v->attrs()->end())
return resolveOutPath(state, found->value, pos);
return v;
}

void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
{
NixStringContext context;
std::shared_ptr<SourceAccessor> accessor;
std::vector<BackedStringView> s;
size_t sSize = 0;
NixInt n{0};
Expand Down Expand Up @@ -1974,15 +1992,19 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
Value * vTmpP = values.data();

for (auto & [i_pos, i] : *es) {
Value & vTmp = *vTmpP++;
i->eval(state, env, vTmp);
Value & vTmp0 = *vTmpP++;
i->eval(state, env, vTmp0);
Value & vTmp = *resolveOutPath(state, &vTmp0, i_pos);

/* If the first element is a path, then the result will also
be a path, we don't copy anything (yet - that's done later,
since paths are copied when they are used in a derivation),
and none of the strings are allowed to have contexts. */
if (first) {
firstType = vTmp.type();
if (firstType == nPath) {
accessor = vTmp.path().accessor;
}
}

if (firstType == nInt) {
Expand Down Expand Up @@ -2029,7 +2051,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
else if (firstType == nPath) {
if (!context.empty())
state.error<EvalError>("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow();
v.mkPath(state.rootPath(CanonPath(canonPath(str()))));
v.mkPath({ref(accessor), CanonPath(str())});
} else
v.mkStringMove(c_str(), context);
}
Expand Down Expand Up @@ -2279,6 +2301,8 @@ BackedStringView EvalState::coerceToString(
v.payload.path.path
: copyToStore
? store->printStorePath(copyPathToStore(context, v.path()))
: v.path().accessor->toStringReturnsStorePath()
? store->printStorePath(copyPathToStore(context, SourcePath(v.path().accessor, CanonPath::root))) + v.path().path.absOrEmpty()
: std::string(v.path().path.abs());
}

Expand Down Expand Up @@ -2391,10 +2415,14 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext
if (v.type() == nPath)
return v.path();

/* Similarly, handle __toString where the result may be a path
/* Similarly, handle outPath and __toString where the result may be a path
value. */
if (v.type() == nAttrs) {
auto i = v.attrs()->find(sToString);
auto i = v.attrs()->find(sOutPath);
if (i != v.attrs()->end()) {
return coerceToPath(pos, *i->value, context, errorCtx);
}
i = v.attrs()->find(sToString);
if (i != v.attrs()->end()) {
Value v1;
callFunction(*i->value, v, v1, pos);
Expand Down Expand Up @@ -3017,12 +3045,14 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_
if (!suffixOpt) continue;
auto suffix = *suffixOpt;

auto rOpt = resolveLookupPathPath(i.path);
auto rOpt = resolveLookupPathPath(
i.path,
true);
if (!rOpt) continue;
auto r = *rOpt;

Path res = suffix == "" ? r : concatStrings(r, "/", suffix);
if (pathExists(res)) return rootPath(CanonPath(canonPath(res)));
auto res = r / CanonPath(suffix);
if (res.pathExists()) return res;
}

if (hasPrefix(path, "nix/"))
Expand All @@ -3037,26 +3067,30 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_
}


std::optional<std::string> EvalState::resolveLookupPathPath(const LookupPath::Path & value0, bool initAccessControl)
std::optional<SourcePath> EvalState::resolveLookupPathPath(const LookupPath::Path & value0, bool initAccessControl)
{
auto & value = value0.s;
auto i = lookupPathResolved.find(value);
if (i != lookupPathResolved.end()) return i->second;

auto finish = [&](std::string res) {
auto finish = [&](SourcePath res) {
debug("resolved search path element '%s' to '%s'", value, res);
lookupPathResolved.emplace(value, res);
return res;
};

std::optional<SourcePath> res;

if (EvalSettings::isPseudoUrl(value)) {
try {
auto accessor = fetchers::downloadTarball(
store,
fetchSettings,
EvalSettings::resolvePseudoUrl(value));
// Traditional search path lookups use the absolute path space for
// historical consistency.
auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy);
return finish(store->toRealPath(storePath));
res.emplace(rootPath(CanonPath(store->toRealPath(storePath))));
} catch (Error & e) {
logWarning({
.msg = HintFmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value)
Expand All @@ -3075,22 +3109,22 @@ std::optional<std::string> EvalState::resolveLookupPathPath(const LookupPath::Pa
}

{
auto path = absPath(value);
auto path = rootPath(value);

/* Allow access to paths in the search path. */
if (initAccessControl) {
allowPath(path);
if (store->isInStore(path)) {
allowPath(path.path.abs());
if (store->isInStore(path.path.abs())) {
try {
StorePathSet closure;
store->computeFSClosure(store->toStorePath(path).first, closure);
store->computeFSClosure(store->toStorePath(path.path.abs()).first, closure);
for (auto & p : closure)
allowPath(p);
} catch (InvalidPath &) { }
}
}

if (pathExists(path))
if (path.pathExists())
return finish(std::move(path));
else {
logWarning({
Expand Down
9 changes: 7 additions & 2 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ public:

const SourcePath callFlakeInternal;

/* A collection of InputAccessors, just to keep them alive. */
std::list<ref<SourceAccessor>> sourceAccessors;

/**
* Store used to materialise .drv files.
*/
Expand Down Expand Up @@ -342,7 +345,7 @@ private:

LookupPath lookupPath;

std::map<std::string, std::optional<std::string>> lookupPathResolved;
std::map<std::string, std::optional<SourcePath>> lookupPathResolved;

/**
* Cache used by prim_match().
Expand Down Expand Up @@ -384,6 +387,8 @@ public:
*/
SourcePath rootPath(PathView path);

void registerAccessor(ref<SourceAccessor> accessor);

/**
* Allow access to a path.
*/
Expand Down Expand Up @@ -449,7 +454,7 @@ public:
*
* If it is not found, return `std::nullopt`
*/
std::optional<std::string> resolveLookupPathPath(
std::optional<SourcePath> resolveLookupPathPath(
const LookupPath::Path & elem,
bool initAccessControl = false);

Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/nixexpr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void ExprString::show(const SymbolTable & symbols, std::ostream & str) const

void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const
{
str << s;
str << path;
}

void ExprVar::show(const SymbolTable & symbols, std::ostream & str) const
Expand Down
11 changes: 7 additions & 4 deletions src/libexpr/nixexpr.hh
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,15 @@ struct ExprString : Expr

struct ExprPath : Expr
{
ref<SourceAccessor> accessor;
std::string s;
const SourcePath path;
Value v;
ExprPath(ref<SourceAccessor> accessor, std::string s) : accessor(accessor), s(std::move(s))
ExprPath(SourcePath && path)
: path(path)
{
v.mkPath(&*accessor, this->s.c_str());
v.mkPath(
&*path.accessor,
// TODO: GC_STRDUP
strdup(path.path.abs().c_str()));
}
Value * maybeThunk(EvalState & state, Env & env) override;
COMMON_METHODS
Expand Down
33 changes: 23 additions & 10 deletions src/libexpr/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
std::vector<std::pair<nix::AttrName, nix::PosIdx>> * inheritAttrs;
std::vector<std::pair<nix::PosIdx, nix::Expr *>> * string_parts;
std::vector<std::pair<nix::PosIdx, std::variant<nix::Expr *, nix::StringToken>>> * ind_string_parts;
struct {
nix::Expr * e;
bool appendSlash;
} pathStart;
}

%type <e> start expr expr_function expr_if expr_op
Expand All @@ -149,7 +153,8 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
%type <inheritAttrs> attrs
%type <string_parts> string_parts_interpolated
%type <ind_string_parts> ind_string_parts
%type <e> path_start string_parts string_attr
%type <pathStart> path_start
%type <e> string_parts string_attr
%type <id> attr
%token <id> ID
%token <str> STR IND_STR
Expand Down Expand Up @@ -304,9 +309,11 @@ expr_simple
$$ = state->stripIndentation(CUR_POS, std::move(*$2));
delete $2;
}
| path_start PATH_END
| path_start PATH_END { $$ = $1.e; }
| path_start string_parts_interpolated PATH_END {
$2->insert($2->begin(), {state->at(@1), $1});
if ($1.appendSlash)
$2->insert($2->begin(), {noPos, new ExprString("/")});
$2->insert($2->begin(), {state->at(@1), $1.e});
$$ = new ExprConcatStrings(CUR_POS, false, $2);
}
| SPATH {
Expand Down Expand Up @@ -359,11 +366,17 @@ string_parts_interpolated

path_start
: PATH {
Path path(absPath(std::string_view{$1.p, $1.l}, state->basePath.path.abs()));
/* add back in the trailing '/' to the first segment */
if ($1.p[$1.l-1] == '/' && $1.l > 1)
path += "/";
$$ = new ExprPath(ref<SourceAccessor>(state->rootFS), std::move(path));
std::string_view path({$1.p, $1.l});
$$ = {
.e = new ExprPath(
/* Absolute paths are always interpreted relative to the
root filesystem accessor, rather than the accessor of the
current Nix expression. */
hasPrefix(path, "/")
? SourcePath{state->rootFS, CanonPath(path)}
: SourcePath{state->basePath.accessor, CanonPath(path, state->basePath.path)}),
.appendSlash = hasSuffix(path, "/")
};
}
| HPATH {
if (state->settings.pureEval) {
Expand All @@ -372,8 +385,8 @@ path_start
std::string_view($1.p, $1.l)
);
}
Path path(getHome() + std::string($1.p + 1, $1.l - 1));
$$ = new ExprPath(ref<SourceAccessor>(state->rootFS), std::move(path));
CanonPath path(getHome() + std::string($1.p + 1, $1.l - 1));
$$ = {.e = new ExprPath(SourcePath{state->rootFS, std::move(path)}), .appendSlash = true};
}
;

Expand Down
Loading

0 comments on commit 2334f5f

Please sign in to comment.