diff options
Diffstat (limited to 'src/libexpr')
| -rw-r--r-- | src/libexpr/eval.cc | 96 | ||||
| -rw-r--r-- | src/libexpr/eval.hh | 56 | ||||
| -rw-r--r-- | src/libexpr/primops.cc | 95 | ||||
| -rw-r--r-- | src/libexpr/print.cc | 20 | ||||
| -rw-r--r-- | src/libexpr/print.hh | 6 | ||||
| -rw-r--r-- | src/libexpr/tests/derived-path.cc | 65 | ||||
| -rw-r--r-- | src/libexpr/tests/value/context.cc | 6 |
7 files changed, 284 insertions, 60 deletions
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e2b455b91..740a5e677 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -94,7 +94,6 @@ RootValue allocRootValue(Value * v) #endif } - void Value::print(const SymbolTable & symbols, std::ostream & str, std::set<const void *> * seen) const { @@ -1048,6 +1047,27 @@ void EvalState::mkStorePathString(const StorePath & p, Value & v) } +void EvalState::mkOutputString( + Value & value, + const StorePath & drvPath, + const std::string outputName, + std::optional<StorePath> optOutputPath) +{ + value.mkString( + optOutputPath + ? store->printStorePath(*std::move(optOutputPath)) + /* Downstream we would substitute this for an actual path once + we build the floating CA derivation */ + : downstreamPlaceholder(*store, drvPath, outputName), + NixStringContext { + NixStringContextElem::Built { + .drvPath = drvPath, + .output = outputName, + } + }); +} + + /* Create a thunk for the delayed computation of the given expression in the given environment. But if the expression is a variable, then look it up right away. This significantly reduces the number @@ -2298,6 +2318,80 @@ StorePath EvalState::coerceToStorePath(const PosIdx pos, Value & v, NixStringCon } +std::pair<DerivedPath, std::string_view> EvalState::coerceToDerivedPathUnchecked(const PosIdx pos, Value & v, std::string_view errorCtx) +{ + NixStringContext context; + auto s = forceString(v, context, pos, errorCtx); + auto csize = context.size(); + if (csize != 1) + error( + "string '%s' has %d entries in its context. It should only have exactly one entry", + s, csize) + .withTrace(pos, errorCtx).debugThrow<EvalError>(); + auto derivedPath = std::visit(overloaded { + [&](NixStringContextElem::Opaque && o) -> DerivedPath { + return DerivedPath::Opaque { + .path = std::move(o.path), + }; + }, + [&](NixStringContextElem::DrvDeep &&) -> DerivedPath { + error( + "string '%s' has a context which refers to a complete source and binary closure. This is not supported at this time", + s).withTrace(pos, errorCtx).debugThrow<EvalError>(); + }, + [&](NixStringContextElem::Built && b) -> DerivedPath { + return DerivedPath::Built { + .drvPath = std::move(b.drvPath), + .outputs = OutputsSpec::Names { std::move(b.output) }, + }; + }, + }, ((NixStringContextElem &&) *context.begin()).raw()); + return { + std::move(derivedPath), + std::move(s), + }; +} + + +DerivedPath EvalState::coerceToDerivedPath(const PosIdx pos, Value & v, std::string_view errorCtx) +{ + auto [derivedPath, s_] = coerceToDerivedPathUnchecked(pos, v, errorCtx); + auto s = s_; + std::visit(overloaded { + [&](const DerivedPath::Opaque & o) { + auto sExpected = store->printStorePath(o.path); + if (s != sExpected) + error( + "path string '%s' has context with the different path '%s'", + s, sExpected) + .withTrace(pos, errorCtx).debugThrow<EvalError>(); + }, + [&](const DerivedPath::Built & b) { + // TODO need derived path with single output to make this + // total. Will add as part of RFC 92 work and then this is + // cleaned up. + auto output = *std::get<OutputsSpec::Names>(b.outputs).begin(); + + auto drv = store->readDerivation(b.drvPath); + auto i = drv.outputs.find(output); + if (i == drv.outputs.end()) + throw Error("derivation '%s' does not have output '%s'", store->printStorePath(b.drvPath), output); + auto optOutputPath = i->second.path(*store, drv.name, output); + // This is testing for the case of CA derivations + auto sExpected = optOutputPath + ? store->printStorePath(*optOutputPath) + : downstreamPlaceholder(*store, b.drvPath, output); + if (s != sExpected) + error( + "string '%s' has context with the output '%s' from derivation '%s', but the string is not the right placeholder for this derivation output. It should be '%s'", + s, output, store->printStorePath(b.drvPath), sExpected) + .withTrace(pos, errorCtx).debugThrow<EvalError>(); + } + }, derivedPath.raw()); + return derivedPath; +} + + bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx) { forceValue(v1, noPos); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index bb3ac2b22..a90ff34c0 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -21,6 +21,7 @@ namespace nix { class Store; class EvalState; class StorePath; +struct DerivedPath; enum RepairFlag : bool; @@ -473,6 +474,28 @@ public: */ StorePath coerceToStorePath(const PosIdx pos, Value & v, NixStringContext & context, std::string_view errorCtx); + /** + * Part of `coerceToDerivedPath()` without any store IO which is exposed for unit testing only. + */ + std::pair<DerivedPath, std::string_view> coerceToDerivedPathUnchecked(const PosIdx pos, Value & v, std::string_view errorCtx); + + /** + * Coerce to `DerivedPath`. + * + * Must be a string which is either a literal store path or a + * "placeholder (see `downstreamPlaceholder()`). + * + * Even more importantly, the string context must be exactly one + * element, which is either a `NixStringContextElem::Opaque` or + * `NixStringContextElem::Built`. (`NixStringContextEleme::DrvDeep` + * is not permitted). + * + * The string is parsed based on the context --- the context is the + * source of truth, and ultimately tells us what we want, and then + * we ensure the string corresponds to it. + */ + DerivedPath coerceToDerivedPath(const PosIdx pos, Value & v, std::string_view errorCtx); + public: /** @@ -576,12 +599,37 @@ public: void mkThunk_(Value & v, Expr * expr); void mkPos(Value & v, PosIdx pos); - /* Create a string representing a store path. - - The string is the printed store path with a context containing a single - `Opaque` element of that store path. */ + /** + * Create a string representing a store path. + * + * The string is the printed store path with a context containing a single + * `NixStringContextElem::Opaque` element of that store path. + */ void mkStorePathString(const StorePath & storePath, Value & v); + /** + * Create a string representing a `DerivedPath::Built`. + * + * The string is the printed store path with a context containing a single + * `NixStringContextElem::Built` element of the drv path and output name. + * + * @param value Value we are settings + * + * @param drvPath Path the drv whose output we are making a string for + * + * @param outputName Name of the output + * + * @param optOutputPath Optional output path for that string. Must + * be passed if and only if output store object is input-addressed. + * Will be printed to form string if passed, otherwise a placeholder + * will be used (see `downstreamPlaceholder()`). + */ + void mkOutputString( + Value & value, + const StorePath & drvPath, + const std::string outputName, + std::optional<StorePath> optOutputPath); + void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx); /** diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 22ac780a5..6fbd66389 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -129,40 +129,31 @@ static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, co } } -/* Add and attribute to the given attribute map from the output name to - the output path, or a placeholder. - - Where possible the path is used, but for floating CA derivations we - may not know it. For sake of determinism we always assume we don't - and instead put in a place holder. In either case, however, the - string context will contain the drv path and output name, so - downstream derivations will have the proper dependency, and in - addition, before building, the placeholder will be rewritten to be - the actual path. - - The 'drv' and 'drvPath' outputs must correspond. */ +/** + * Add and attribute to the given attribute map from the output name to + * the output path, or a placeholder. + * + * Where possible the path is used, but for floating CA derivations we + * may not know it. For sake of determinism we always assume we don't + * and instead put in a place holder. In either case, however, the + * string context will contain the drv path and output name, so + * downstream derivations will have the proper dependency, and in + * addition, before building, the placeholder will be rewritten to be + * the actual path. + * + * The 'drv' and 'drvPath' outputs must correspond. + */ static void mkOutputString( EvalState & state, BindingsBuilder & attrs, const StorePath & drvPath, - const BasicDerivation & drv, const std::pair<std::string, DerivationOutput> & o) { - auto optOutputPath = o.second.path(*state.store, drv.name, o.first); - attrs.alloc(o.first).mkString( - optOutputPath - ? state.store->printStorePath(*optOutputPath) - /* Downstream we would substitute this for an actual path once - we build the floating CA derivation */ - /* FIXME: we need to depend on the basic derivation, not - derivation */ - : downstreamPlaceholder(*state.store, drvPath, o.first), - NixStringContext { - NixStringContextElem::Built { - .drvPath = drvPath, - .output = o.first, - } - }); + state.mkOutputString( + attrs.alloc(o.first), + drvPath, + o.first, + o.second.path(*state.store, Derivation::nameFromPath(drvPath), o.first)); } /* Load and evaluate an expression from path specified by the @@ -193,7 +184,7 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v state.mkList(outputsVal, drv.outputs.size()); for (const auto & [i, o] : enumerate(drv.outputs)) { - mkOutputString(state, attrs, *storePath, drv, o); + mkOutputString(state, attrs, *storePath, o); (outputsVal.listElems()[i] = state.allocValue())->mkString(o.first); } @@ -1100,7 +1091,7 @@ drvName, Bindings * attrs, Value & v) bool isImpure = false; std::optional<std::string> outputHash; std::string outputHashAlgo; - std::optional<FileIngestionMethod> ingestionMethod; + std::optional<ContentAddressMethod> ingestionMethod; StringSet outputs; outputs.insert("out"); @@ -1113,7 +1104,10 @@ drvName, Bindings * attrs, Value & v) auto handleHashMode = [&](const std::string_view s) { if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive; else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat; - else + else if (s == "text") { + experimentalFeatureSettings.require(Xp::DynamicDerivations); + ingestionMethod = TextIngestionMethod {}; + } else state.debugThrowLastTrace(EvalError({ .msg = hintfmt("invalid value '%s' for 'outputHashMode' attribute", s), .errPos = state.positions[noPos] @@ -1280,11 +1274,16 @@ drvName, Bindings * attrs, Value & v) })); /* Check whether the derivation name is valid. */ - if (isDerivation(drvName)) + if (isDerivation(drvName) && + !(ingestionMethod == ContentAddressMethod { TextIngestionMethod { } } && + outputs.size() == 1 && + *(outputs.begin()) == "out")) + { state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("derivation names are not allowed to end in '%s'", drvExtension), + .msg = hintfmt("derivation names are allowed to end in '%s' only if they produce a single derivation file", drvExtension), .errPos = state.positions[noPos] })); + } if (outputHash) { /* Handle fixed-output derivations. @@ -1300,21 +1299,15 @@ drvName, Bindings * attrs, Value & v) auto h = newHashAllowEmpty(*outputHash, parseHashTypeOpt(outputHashAlgo)); auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); - auto outPath = state.store->makeFixedOutputPath(drvName, FixedOutputInfo { - .hash = { - .method = method, - .hash = h, - }, - .references = {}, - }); - drv.env["out"] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign("out", - DerivationOutput::CAFixed { - .hash = FixedOutputHash { - .method = method, - .hash = std::move(h), - }, - }); + + DerivationOutput::CAFixed dof { + .ca = ContentAddress::fromParts( + std::move(method), + std::move(h)), + }; + + drv.env["out"] = state.store->printStorePath(dof.path(*state.store, drvName, "out")); + drv.outputs.insert_or_assign("out", std::move(dof)); } else if (contentAddressed || isImpure) { @@ -1332,13 +1325,13 @@ drvName, Bindings * attrs, Value & v) if (isImpure) drv.outputs.insert_or_assign(i, DerivationOutput::Impure { - .method = method, + .method = method.raw, .hashType = ht, }); else drv.outputs.insert_or_assign(i, DerivationOutput::CAFloating { - .method = method, + .method = method.raw, .hashType = ht, }); } @@ -1403,7 +1396,7 @@ drvName, Bindings * attrs, Value & v) NixStringContextElem::DrvDeep { .drvPath = drvPath }, }); for (auto & i : drv.outputs) - mkOutputString(state, result, drvPath, drv, i); + mkOutputString(state, result, drvPath, i); v.mkAttrs(result); } diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index d08672cfc..53ba70bdd 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -1,4 +1,5 @@ #include "print.hh" +#include <unordered_set> namespace nix { @@ -25,11 +26,26 @@ printLiteralBool(std::ostream & str, bool boolean) return str; } +// Returns `true' is a string is a reserved keyword which requires quotation +// when printing attribute set field names. +// +// This list should generally be kept in sync with `./lexer.l'. +// You can test if a keyword needs to be added by running: +// $ nix eval --expr '{ <KEYWORD> = 1; }' +// For example `or' doesn't need to be quoted. +bool isReservedKeyword(const std::string_view str) +{ + static const std::unordered_set<std::string_view> reservedKeywords = { + "if", "then", "else", "assert", "with", "let", "in", "rec", "inherit" + }; + return reservedKeywords.contains(str); +} + std::ostream & printIdentifier(std::ostream & str, std::string_view s) { if (s.empty()) str << "\"\""; - else if (s == "if") // FIXME: handle other keywords + else if (isReservedKeyword(s)) str << '"' << s << '"'; else { char c = s[0]; @@ -50,10 +66,10 @@ printIdentifier(std::ostream & str, std::string_view s) { return str; } -// FIXME: keywords static bool isVarName(std::string_view s) { if (s.size() == 0) return false; + if (isReservedKeyword(s)) return false; char c = s[0]; if ((c >= '0' && c <= '9') || c == '-' || c == '\'') return false; for (auto & i : s) diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh index f9cfc3964..3b72ae201 100644 --- a/src/libexpr/print.hh +++ b/src/libexpr/print.hh @@ -36,6 +36,12 @@ namespace nix { std::ostream & printAttributeName(std::ostream & o, std::string_view s); /** + * Returns `true' is a string is a reserved keyword which requires quotation + * when printing attribute set field names. + */ + bool isReservedKeyword(const std::string_view str); + + /** * Print a string as an identifier in the Nix expression language syntax. * * FIXME: "identifier" is ambiguous. Identifiers do not have a single diff --git a/src/libexpr/tests/derived-path.cc b/src/libexpr/tests/derived-path.cc new file mode 100644 index 000000000..8210efef2 --- /dev/null +++ b/src/libexpr/tests/derived-path.cc @@ -0,0 +1,65 @@ +#include <nlohmann/json.hpp> +#include <gtest/gtest.h> +#include <rapidcheck/gtest.h> + +#include "tests/derived-path.hh" +#include "tests/libexpr.hh" + +namespace nix { + +// Testing of trivial expressions +class DerivedPathExpressionTest : public LibExprTest {}; + +// FIXME: `RC_GTEST_FIXTURE_PROP` isn't calling `SetUpTestSuite` because it is +// no a real fixture. +// +// See https://github.com/emil-e/rapidcheck/blob/master/doc/gtest.md#rc_gtest_fixture_propfixture-name-args +TEST_F(DerivedPathExpressionTest, force_init) +{ +} + +RC_GTEST_FIXTURE_PROP( + DerivedPathExpressionTest, + prop_opaque_path_round_trip, + (const DerivedPath::Opaque & o)) +{ + auto * v = state.allocValue(); + state.mkStorePathString(o.path, *v); + auto d = state.coerceToDerivedPath(noPos, *v, ""); + RC_ASSERT(DerivedPath { o } == d); +} + +// TODO use DerivedPath::Built for parameter once it supports a single output +// path only. + +RC_GTEST_FIXTURE_PROP( + DerivedPathExpressionTest, + prop_built_path_placeholder_round_trip, + (const StorePath & drvPath, const StorePathName & outputName)) +{ + auto * v = state.allocValue(); + state.mkOutputString(*v, drvPath, outputName.name, std::nullopt); + auto [d, _] = state.coerceToDerivedPathUnchecked(noPos, *v, ""); + DerivedPath::Built b { + .drvPath = drvPath, + .outputs = OutputsSpec::Names { outputName.name }, + }; + RC_ASSERT(DerivedPath { b } == d); +} + +RC_GTEST_FIXTURE_PROP( + DerivedPathExpressionTest, + prop_built_path_out_path_round_trip, + (const StorePath & drvPath, const StorePathName & outputName, const StorePath & outPath)) +{ + auto * v = state.allocValue(); + state.mkOutputString(*v, drvPath, outputName.name, outPath); + auto [d, _] = state.coerceToDerivedPathUnchecked(noPos, *v, ""); + DerivedPath::Built b { + .drvPath = drvPath, + .outputs = OutputsSpec::Names { outputName.name }, + }; + RC_ASSERT(DerivedPath { b } == d); +} + +} /* namespace nix */ diff --git a/src/libexpr/tests/value/context.cc b/src/libexpr/tests/value/context.cc index 27d6920b0..0d9381577 100644 --- a/src/libexpr/tests/value/context.cc +++ b/src/libexpr/tests/value/context.cc @@ -95,13 +95,15 @@ Gen<NixStringContextElem::Built> Arbitrary<NixStringContextElem::Built>::arbitra Gen<NixStringContextElem> Arbitrary<NixStringContextElem>::arbitrary() { - switch (*gen::inRange<uint8_t>(0, 2)) { + switch (*gen::inRange<uint8_t>(0, std::variant_size_v<NixStringContextElem::Raw>)) { case 0: return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::Opaque>()); case 1: return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::DrvDeep>()); - default: + case 2: return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::Built>()); + default: + assert(false); } } |