From 087be7281a02a6d0e548ae56b910771837023d21 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Sun, 13 Jan 2019 10:33:15 -0500 Subject: [PATCH 1/3] Treat plain derivation paths in context as normal paths. Previously, plain derivation paths in the string context (e.g. those that arose from builtins.storePath on a drv file, not those that arose from accessing .drvPath of a derivation) were treated somewhat like derivaiton paths derived from .drvPath, except their dependencies weren't recursively added to the input set. With this change, such plain derivation paths are simply treated as paths and added to the source inputs set accordingly, simplifying context handling code and removing the inconsistency. If drvPath-like behavior is desired, the .drv file can be imported and then .drvPath can be accessed. This is a backwards-incompatibility, but storePath is never used on drv files within nixpkgs and almost never used elsewhere. --- .gitignore | 3 +++ src/libexpr/primops.cc | 20 +++----------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/.gitignore b/.gitignore index 0f2f3ddee..b75c5d489 100644 --- a/.gitignore +++ b/.gitignore @@ -81,6 +81,9 @@ perl/Makefile.config /tests/common.sh /tests/dummy /tests/result* +/tests/restricted-innocent +/tests/shell +/tests/shell.drv # /tests/lang/ /tests/lang/*.out diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 60698f740..113ceaf29 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -687,21 +687,12 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } } - /* See prim_unsafeDiscardOutputDependency. */ - else if (path.at(0) == '~') - drv.inputSrcs.insert(string(path, 1)); - /* Handle derivation outputs of the form ‘!!’. */ else if (path.at(0) == '!') { std::pair ctx = decodeContext(path); drv.inputDrvs[ctx.first].insert(ctx.second); } - /* Handle derivation contexts returned by - ‘builtins.storePath’. */ - else if (isDerivation(path)) - drv.inputDrvs[path] = state.store->queryDerivationOutputNames(path); - /* Otherwise it's a source file. */ else drv.inputSrcs.insert(path); @@ -1004,13 +995,8 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu PathSet refs; for (auto path : context) { - if (path.at(0) == '=') path = string(path, 1); - if (isDerivation(path)) { - /* See prim_unsafeDiscardOutputDependency. */ - if (path.at(0) != '~') - throw EvalError(format("in 'toFile': the file '%1%' cannot refer to derivation outputs, at %2%") % name % pos); - path = string(path, 1); - } + if (path.at(0) != '/') + throw EvalError(format("in 'toFile': the file '%1%' cannot refer to derivation outputs, at %2%") % name % pos); refs.insert(path); } @@ -1823,7 +1809,7 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const Pos & po PathSet context2; for (auto & p : context) - context2.insert(p.at(0) == '=' ? "~" + string(p, 1) : p); + context2.insert(p.at(0) == '=' ? string(p, 1) : p); mkString(v, s, context2); } From 1d757292d0cb78beec32fcdfe15c2944a4bc4a95 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Sun, 13 Jan 2019 12:43:46 -0500 Subject: [PATCH 2/3] Add builtins.getContext. This can be very helpful when debugging, as well as enabling complex black magic like surgically removing a single dependency from a string's context. --- src/libexpr/eval.hh | 3 + src/libexpr/primops.cc | 38 ----- src/libexpr/primops/context.cc | 132 ++++++++++++++++++ .../lang/eval-okay-context-introspection.exp | 1 + .../lang/eval-okay-context-introspection.nix | 22 +++ 5 files changed, 158 insertions(+), 38 deletions(-) create mode 100644 src/libexpr/primops/context.cc create mode 100644 tests/lang/eval-okay-context-introspection.exp create mode 100644 tests/lang/eval-okay-context-introspection.nix diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index d0f298e16..9fe387891 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -316,6 +316,9 @@ private: /* Return a string representing the type of the value `v'. */ string showType(const Value & v); +/* Decode a context string ‘!!’ into a pair . */ +std::pair decodeContext(const string & s); /* If `path' refers to a directory, then append "/default.nix". */ Path resolveExprPath(Path path); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 113ceaf29..0da9f702f 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1780,41 +1780,6 @@ static void prim_stringLength(EvalState & state, const Pos & pos, Value * * args } -static void prim_unsafeDiscardStringContext(EvalState & state, const Pos & pos, Value * * args, Value & v) -{ - PathSet context; - string s = state.coerceToString(pos, *args[0], context); - mkString(v, s, PathSet()); -} - - -static void prim_hasContext(EvalState & state, const Pos & pos, Value * * args, Value & v) -{ - PathSet context; - state.forceString(*args[0], context, pos); - mkBool(v, !context.empty()); -} - - -/* Sometimes we want to pass a derivation path (i.e. pkg.drvPath) to a - builder without causing the derivation to be built (for instance, - in the derivation that builds NARs in nix-push, when doing - source-only deployment). This primop marks the string context so - that builtins.derivation adds the path to drv.inputSrcs rather than - drv.inputDrvs. */ -static void prim_unsafeDiscardOutputDependency(EvalState & state, const Pos & pos, Value * * args, Value & v) -{ - PathSet context; - string s = state.coerceToString(pos, *args[0], context); - - PathSet context2; - for (auto & p : context) - context2.insert(p.at(0) == '=' ? string(p, 1) : p); - - mkString(v, s, context2); -} - - /* Return the cryptographic hash of a string in base-16. */ static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, Value & v) { @@ -2285,9 +2250,6 @@ void EvalState::createBaseEnv() addPrimOp("toString", 1, prim_toString); addPrimOp("__substring", 3, prim_substring); addPrimOp("__stringLength", 1, prim_stringLength); - addPrimOp("__hasContext", 1, prim_hasContext); - addPrimOp("__unsafeDiscardStringContext", 1, prim_unsafeDiscardStringContext); - addPrimOp("__unsafeDiscardOutputDependency", 1, prim_unsafeDiscardOutputDependency); addPrimOp("__hashString", 2, prim_hashString); addPrimOp("__match", 2, prim_match); addPrimOp("__split", 2, prim_split); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc new file mode 100644 index 000000000..6849aa261 --- /dev/null +++ b/src/libexpr/primops/context.cc @@ -0,0 +1,132 @@ +#include "primops.hh" +#include "eval-inline.hh" + +namespace nix { + +static void prim_unsafeDiscardStringContext(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + PathSet context; + string s = state.coerceToString(pos, *args[0], context); + mkString(v, s, PathSet()); +} + +static RegisterPrimOp r1("__unsafeDiscardStringContext", 1, prim_unsafeDiscardStringContext); + + +static void prim_hasContext(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + PathSet context; + state.forceString(*args[0], context, pos); + mkBool(v, !context.empty()); +} + +static RegisterPrimOp r2("__hasContext", 1, prim_hasContext); + + +/* Sometimes we want to pass a derivation path (i.e. pkg.drvPath) to a + builder without causing the derivation to be built (for instance, + in the derivation that builds NARs in nix-push, when doing + source-only deployment). This primop marks the string context so + that builtins.derivation adds the path to drv.inputSrcs rather than + drv.inputDrvs. */ +static void prim_unsafeDiscardOutputDependency(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + PathSet context; + string s = state.coerceToString(pos, *args[0], context); + + PathSet context2; + for (auto & p : context) + context2.insert(p.at(0) == '=' ? string(p, 1) : p); + + mkString(v, s, context2); +} + +static RegisterPrimOp r3("__unsafeDiscardOutputDependency", 1, prim_unsafeDiscardOutputDependency); + + +/* Extract the context of a string as a structured Nix value. + + The context is represented as an attribute set whose keys are the + paths in the context set and whose values are attribute sets with + the following keys: + path: True if the relevant path is in the context as a plain store + path (i.e. the kind of context you get when interpolating + a Nix path (e.g. ./.) into a string). False if missing. + allOutputs: True if the relevant path is a derivation and it is + in the context as a drv file with all of its outputs + (i.e. the kind of context you get when referencing + .drvPath of some derivation). False if missing. + outputs: If a non-empty list, the relevant path is a derivation + and the provided outputs are referenced in the context + (i.e. the kind of context you get when referencing + .outPath of some derivation). Empty list if missing. + Note that for a given path any combination of the above attributes + may be present, but at least one must be set to something other + than the default. +*/ +static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + struct ContextInfo { + bool path = false; + bool allOutputs = false; + Strings outputs; + }; + PathSet context; + state.forceString(*args[0], context, pos); + auto contextInfos = std::map(); + for (const auto & p : context) { + Path drv; + string output; + const Path * path = &p; + if (p.at(0) == '=') { + drv = string(p, 1); + path = &drv; + } else if (p.at(0) == '!') { + std::pair ctx = decodeContext(p); + drv = ctx.first; + output = ctx.second; + path = &drv; + } + auto isPath = drv.empty(); + auto isAllOutputs = (!drv.empty()) && output.empty(); + + auto iter = contextInfos.find(*path); + if (iter == contextInfos.end()) { + contextInfos.emplace(*path, ContextInfo{isPath, isAllOutputs, output.empty() ? Strings{} : Strings{std::move(output)}}); + } else { + if (isPath) + iter->second.path = true; + else if (isAllOutputs) + iter->second.allOutputs = true; + else + iter->second.outputs.emplace_back(std::move(output)); + } + } + + state.mkAttrs(v, contextInfos.size()); + + auto sPath = state.symbols.create("path"); + auto sAllOutputs = state.symbols.create("allOutputs"); + for (const auto & info : contextInfos) { + auto & infoVal = *state.allocAttr(v, state.symbols.create(info.first)); + state.mkAttrs(infoVal, 3); + if (info.second.path) + mkBool(*state.allocAttr(infoVal, sPath), true); + if (info.second.allOutputs) + mkBool(*state.allocAttr(infoVal, sAllOutputs), true); + if (!info.second.outputs.empty()) { + auto & outputsVal = *state.allocAttr(infoVal, state.sOutputs); + state.mkList(outputsVal, info.second.outputs.size()); + size_t i = 0; + for (const auto & output : info.second.outputs) { + mkString(*(outputsVal.listElems()[i++] = state.allocValue()), output); + } + } + infoVal.attrs->sort(); + } + v.attrs->sort(); +} + +static RegisterPrimOp r4("__getContext", 1, prim_getContext); + +} diff --git a/tests/lang/eval-okay-context-introspection.exp b/tests/lang/eval-okay-context-introspection.exp new file mode 100644 index 000000000..27ba77dda --- /dev/null +++ b/tests/lang/eval-okay-context-introspection.exp @@ -0,0 +1 @@ +true diff --git a/tests/lang/eval-okay-context-introspection.nix b/tests/lang/eval-okay-context-introspection.nix new file mode 100644 index 000000000..d9b2ea354 --- /dev/null +++ b/tests/lang/eval-okay-context-introspection.nix @@ -0,0 +1,22 @@ +let + drv = derivation { + name = "fail"; + builder = "/bin/false"; + system = "x86_64-linux"; + outputs = [ "out" "foo" ]; + }; + + path = "${./eval-okay-context-introspection.nix}"; + + desired-context = { + "${builtins.unsafeDiscardStringContext path}" = { + path = true; + }; + "${builtins.unsafeDiscardStringContext drv.drvPath}" = { + outputs = [ "foo" "out" ]; + allOutputs = true; + }; + }; + + legit-context = "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}"; +in builtins.getContext legit-context == desired-context From b30be6b450f872f8be6dc8afa28f4b030fa8d1d1 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Mon, 14 Jan 2019 11:34:54 -0500 Subject: [PATCH 3/3] Add builtins.appendContext. A partner of builtins.getContext, useful for the same reasons. --- src/libexpr/primops/context.cc | 59 ++++++++++++++++++- .../lang/eval-okay-context-introspection.nix | 6 +- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 6849aa261..2d79739ea 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -1,5 +1,6 @@ #include "primops.hh" #include "eval-inline.hh" +#include "derivations.hh" namespace nix { @@ -61,8 +62,7 @@ static RegisterPrimOp r3("__unsafeDiscardOutputDependency", 1, prim_unsafeDiscar (i.e. the kind of context you get when referencing .outPath of some derivation). Empty list if missing. Note that for a given path any combination of the above attributes - may be present, but at least one must be set to something other - than the default. + may be present. */ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, Value & v) { @@ -129,4 +129,59 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, static RegisterPrimOp r4("__getContext", 1, prim_getContext); + +/* Append the given context to a given string. + + See the commentary above unsafeGetContext for details of the + context representation. +*/ +static void prim_appendContext(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + PathSet context; + auto orig = state.forceString(*args[0], context, pos); + + state.forceAttrs(*args[1], pos); + + auto sPath = state.symbols.create("path"); + auto sAllOutputs = state.symbols.create("allOutputs"); + for (auto & i : *args[1]->attrs) { + if (!state.store->isStorePath(i.name)) + throw EvalError("Context key '%s' is not a store path, at %s", i.name, i.pos); + if (!settings.readOnlyMode) + state.store->ensurePath(i.name); + state.forceAttrs(*i.value, *i.pos); + auto iter = i.value->attrs->find(sPath); + if (iter != i.value->attrs->end()) { + if (state.forceBool(*iter->value, *iter->pos)) + context.insert(i.name); + } + + iter = i.value->attrs->find(sAllOutputs); + if (iter != i.value->attrs->end()) { + if (state.forceBool(*iter->value, *iter->pos)) { + if (!isDerivation(i.name)) { + throw EvalError("Tried to add all-outputs context of %s, which is not a derivation, to a string, at %s", i.name, i.pos); + } + context.insert("=" + string(i.name)); + } + } + + iter = i.value->attrs->find(state.sOutputs); + if (iter != i.value->attrs->end()) { + state.forceList(*iter->value, *iter->pos); + if (iter->value->listSize() && !isDerivation(i.name)) { + throw EvalError("Tried to add derivation output context of %s, which is not a derivation, to a string, at %s", i.name, i.pos); + } + for (unsigned int n = 0; n < iter->value->listSize(); ++n) { + auto name = state.forceStringNoCtx(*iter->value->listElems()[n], *iter->pos); + context.insert("!" + name + "!" + string(i.name)); + } + } + } + + mkString(v, orig, context); +} + +static RegisterPrimOp r5("__appendContext", 2, prim_appendContext); + } diff --git a/tests/lang/eval-okay-context-introspection.nix b/tests/lang/eval-okay-context-introspection.nix index d9b2ea354..43178bd2e 100644 --- a/tests/lang/eval-okay-context-introspection.nix +++ b/tests/lang/eval-okay-context-introspection.nix @@ -18,5 +18,7 @@ let }; }; - legit-context = "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}"; -in builtins.getContext legit-context == desired-context + legit-context = builtins.getContext "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}"; + + constructed-context = builtins.getContext (builtins.appendContext "" desired-context); +in legit-context == constructed-context