Skip to content

Commit

Permalink
Add a set of reasons to Poison
Browse files Browse the repository at this point in the history
  • Loading branch information
9999years committed Mar 12, 2024
1 parent 109a461 commit f1737d0
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/libexpr/eval-cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ string_t AttrCursor::getStringWithContext()
return o.path;
},
[&](const NixStringContextElem::Poison & p) -> const StorePath & {
root->state.error<PoisonContextError>().debugThrow();
root->state.error<PoisonContextError>(p).debugThrow();
},
}, c.raw);
if (!root->state.store->isValidPath(path)) {
Expand Down
6 changes: 0 additions & 6 deletions src/libexpr/eval-error.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,4 @@ template class EvalErrorBuilder<CachedEvalError>;
template class EvalErrorBuilder<InvalidPathError>;
template class EvalErrorBuilder<PoisonContextError>;

PoisonContextError::PoisonContextError(EvalState & state)
: EvalError(state, "Found 'poison' context that may not be built or included in derivations")
, value(*state.allocValue())
{
}

}
19 changes: 14 additions & 5 deletions src/libexpr/eval-error.hh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "pos-idx.hh"
#include "print-options.hh"
#include "print.hh"
#include "value/context.hh"

namespace nix {

Expand Down Expand Up @@ -61,14 +62,22 @@ public:
struct PoisonContextError : public EvalError
{
public:
Value & value;
PoisonContextError(EvalState & state, Value & value)
: EvalError(state, "Value contains 'poison' context that may not be built or included in derivations: %1%", ValuePrinter(state, value, errorPrintOptions))
, value(value)
std::shared_ptr<Value> value;
const Poison & poison;

PoisonContextError(EvalState & state, const Poison & poison)
: EvalError(state, "Value is poisoned from %1% and may not be built or included in derivations", poison)
, value(nullptr)
, poison(poison)
{
}

PoisonContextError(EvalState & state);
PoisonContextError(EvalState & state, const Poison & poison, Value * value)
: EvalError(state, "Value is poisoned from %1% and may not be built or included in derivations: %2%", poison, ValuePrinter(state, *value, errorPrintOptions))
, value(value)
, poison(poison)
{
}
};

/**
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2474,7 +2474,7 @@ std::pair<SingleDerivedPath, std::string_view> EvalState::coerceToSingleDerivedP
return std::move(b);
},
[&](NixStringContextElem::Poison && p) -> SingleDerivedPath {
error<PoisonContextError>(v).debugThrow();
error<PoisonContextError>(p, &v).debugThrow();
},
}, ((NixStringContextElem &&) *context.begin()).raw);
return {
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ StringMap EvalState::realiseContext(Value & v, const NixStringContext & context)
ensureValid(d.drvPath);
},
[&](const NixStringContextElem::Poison & p) {
error<PoisonContextError>(v).debugThrow();
error<PoisonContextError>(p, &v).debugThrow();
},
}, c.raw);
}
Expand Down Expand Up @@ -1292,7 +1292,7 @@ drvName, Bindings * attrs, Value & input, Value & v)
drv.inputSrcs.insert(o.path);
},
[&](const NixStringContextElem::Poison & p) {
state.error<PoisonContextError>(input)
state.error<PoisonContextError>(p, &input)
.debugThrow();
},
}, c.raw);
Expand Down
16 changes: 14 additions & 2 deletions src/libexpr/primops/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V
return std::move(c);
},
[&](const NixStringContextElem::Poison & p) -> NixStringContextElem::DrvDeep {
state.error<PoisonContextError>(*args[0]).debugThrow();
state.error<PoisonContextError>(p, args[0]).debugThrow();
},
}, context.begin()->raw) }),
};
Expand Down Expand Up @@ -178,6 +178,7 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,
bool path = false;
bool allOutputs = false;
Strings outputs;
Strings reasons;
};
NixStringContext context;
state.forceString(*args[0], context, pos, "while evaluating the argument passed to builtins.getContext");
Expand All @@ -197,7 +198,11 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,
contextInfos[state.store->printStorePath(o.path)].path = true;
},
[&](NixStringContextElem::Poison && p) {
contextInfos["poison"] = {};
Strings reasons;
for (auto & reason : p.reasons) {
reasons.push_front(reason);
}
contextInfos["poison"].reasons = reasons;
},
}, ((NixStringContextElem &&) i).raw);
}
Expand All @@ -206,6 +211,7 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,

auto sPath = state.symbols.create("path");
auto sAllOutputs = state.symbols.create("allOutputs");
auto sReasons = state.symbols.create("reasons");
for (const auto & info : contextInfos) {
auto infoAttrs = state.buildBindings(3);
if (info.second.path)
Expand All @@ -218,6 +224,12 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,
for (const auto & [i, output] : enumerate(info.second.outputs))
(outputsVal.listElems()[i] = state.allocValue())->mkString(output);
}
if (!info.second.reasons.empty()) {
auto & reasonsVal = infoAttrs.alloc(sReasons);
state.mkList(reasonsVal, info.second.reasons.size());
for (const auto & [i, reason] : enumerate(info.second.reasons))
(reasonsVal.listElems()[i] = state.allocValue())->mkString(reason);
}
attrs.alloc(info.first).mkAttrs(infoAttrs);
}

Expand Down
56 changes: 52 additions & 4 deletions src/libexpr/value/context.cc
Original file line number Diff line number Diff line change
@@ -1,10 +1,51 @@
#include <optional>
#include <vector>

#include "util.hh"
#include "value/context.hh"

#include <optional>

namespace nix {

Poison Poison::parse(std::string_view raw)
{
auto ret = Poison();
for (auto & reason : tokenizeString<std::vector<std::string_view>>(raw, "|")) {
ret.addReason(std::string(reason));
}
return ret;
}

void Poison::addReason(std::string reason)
{
if (reason.find(';')) {
throw BadNixStringContextElem(reason, "poison context reasons can't contain `;`, which is used to delimit context elements");
}
if (reason.find('|')) {
throw BadNixStringContextElem(reason, "poison context reasons can't contain `|`, which is used to delimit poison reasons");
}

reasons.insert(reason);
}

void Poison::combine(Poison & other)
{
for (auto & reason : other.reasons) {
addReason(reason);
}
}

std::ostream & operator << (std::ostream & output, const Poison & poison)
{
auto end = poison.reasons.end();
for(auto iterator = poison.reasons.begin(); iterator != end; ++iterator) {
output << *iterator;
if (iterator != end) {
output << ", ";
}
}
return output;
}

NixStringContextElem NixStringContextElem::parse(
std::string_view s0,
const ExperimentalFeatureSettings & xpSettings)
Expand Down Expand Up @@ -58,7 +99,7 @@ NixStringContextElem NixStringContextElem::parse(
};
}
case '%': {
return Poison();
return Poison::parse(s.substr(1));
}
default: {
// Ensure no '!'
Expand Down Expand Up @@ -104,7 +145,14 @@ std::string NixStringContextElem::to_string() const
res += d.drvPath.to_string();
},
[&](const Poison & p) {
res += "%poison";
res += "%";
auto end = p.reasons.end();
for(auto iterator = p.reasons.begin(); iterator != end; ++iterator) {
res += *iterator;
if (iterator != end) {
res += "|";
}
}
},
}, raw);

Expand Down
46 changes: 31 additions & 15 deletions src/libexpr/value/context.hh
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#pragma once
///@file

#include <set>

#include "comparator.hh"
#include "derived-path.hh"
#include "source-path.hh"
#include "variant-wrapper.hh"

#include <nlohmann/json_fwd.hpp>
Expand All @@ -24,6 +26,32 @@ public:
}
};

/**
* "Poison pill" output that is rejected by `builtins.derivation`.
*
* Used to ensure the implementation of functions like
* `builtins.toStringDebug` do not get hashed into derivations.
*
* Encoded as ‘%<reason>|<reason>|...’.
*/
struct Poison {
friend std::ostream & operator << (std::ostream & output, const Poison & poison);

std::set<std::string> reasons;

Poison() {}

static Poison parse(std::string_view raw);

void addReason(std::string reason);

void combine(Poison & other);

GENERATE_CMP(Poison, me->reasons);
};

std::ostream & operator << (std::ostream & output, const Poison & poison);

struct NixStringContextElem {
/**
* Plain opaque path to some store object.
Expand Down Expand Up @@ -54,19 +82,7 @@ struct NixStringContextElem {
*/
using Built = SingleDerivedPath::Built;

/**
* "Poison pill" output that is rejected by `builtins.derivation`.
*
* Used to ensure the implementation of functions like
* `builtins.toStringDebug` do not get hashed into derivations.
*
* Encoded as ‘%poison’.
*/
struct Poison {
bool operator ==(const Poison & other) const { return true; }
bool operator <(const Poison & other) const { return false; }
bool operator >(const Poison & other) const { return false; }
};
using Poison = Poison;

using Raw = std::variant<
Opaque,
Expand All @@ -86,7 +102,7 @@ struct NixStringContextElem {
* - ‘<path>’
* - ‘=<path>’
* - ‘!<name>!<path>’
* - ‘%poison
* - ‘%<reason>|<reason>|...
*
* @param xpSettings Stop-gap to avoid globals during unit tests.
*/
Expand Down
1 change: 1 addition & 0 deletions src/libutil/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ template<class C> C tokenizeString(std::string_view s, std::string_view separato
template Strings tokenizeString(std::string_view s, std::string_view separators);
template StringSet tokenizeString(std::string_view s, std::string_view separators);
template std::vector<std::string> tokenizeString(std::string_view s, std::string_view separators);
template std::vector<std::string_view> tokenizeString(std::string_view s, std::string_view separators);


std::string chomp(std::string_view s)
Expand Down
2 changes: 1 addition & 1 deletion src/nix/app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ UnresolvedApp InstallableValue::toApp(EvalState & state)
};
},
[&](const NixStringContextElem::Poison & p) -> DerivedPath {
state.error<PoisonContextError>(attr->getValue()).debugThrow();
state.error<PoisonContextError>(p, &attr->getValue()).debugThrow();
},
}, c.raw));
}
Expand Down

0 comments on commit f1737d0

Please sign in to comment.