diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9177b0a2f3e7..9a7b8ed68d35 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1104,10 +1104,10 @@ static void prim_second(EvalState & state, const PosIdx pos, Value * * args, Val * Derivations *************************************************************/ -static void derivationStrictInternal( +static void derivationStrictInternalBuildResult( EvalState & state, const std::string & name, - const Bindings * attrs, + const Derivation & drv, Value & v); /* Construct (as a unobservable side effect) a Nix derivation @@ -1119,25 +1119,14 @@ static void derivationStrictInternal( derivation. */ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.derivationStrict"); - - auto attrs = args[0]->attrs(); - - /* Figure out the name first (for stack backtraces). */ - auto nameAttr = getAttr(state, state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict"); - - std::string drvName; - try { - drvName = state.forceStringNoCtx(*nameAttr->value, pos, "while evaluating the `name` attribute passed to builtins.derivationStrict"); - } catch (Error & e) { - e.addTrace(state.positions[nameAttr->pos], "while evaluating the derivation attribute 'name'"); - throw; - } + auto [attrs, drvName, namePos] = derivationStrictInternalGetNameAndAttrs(state, pos, *args[0]); try { - derivationStrictInternal(state, drvName, attrs, v); + auto pos = v.determinePos(noPos); + auto drv = derivationStrictInternalReturning(state, drvName, attrs, pos); + derivationStrictInternalBuildResult(state, drvName, drv, v); } catch (Error & e) { - Pos pos = state.positions[nameAttr->pos]; + Pos pos = state.positions[namePos]; /* * Here we make two abuses of the error system * @@ -1163,16 +1152,41 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * * } } -static void derivationStrictInternal( +std::tuple +derivationStrictInternalGetNameAndAttrs( EvalState & state, - const std::string & drvName, + const PosIdx pos, + Value & arg) +{ + state.forceAttrs(arg, pos, "while evaluating the argument passed to builtins.derivationStrict"); + + auto attrs = arg.attrs(); + + /* Figure out the name first (for stack backtraces). */ + auto nameAttr = getAttr(state, state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict"); + + std::string drvName; + try { + return std::tuple{ + attrs, + state.forceStringNoCtx(*nameAttr->value, pos, "while evaluating the `name` attribute passed to builtins.derivationStrict"), + nameAttr->pos, + }; + } catch (Error & e) { + e.addTrace(state.positions[nameAttr->pos], "while evaluating the derivation attribute 'name'"); + throw; + } +} + +Derivation derivationStrictInternalReturning( + EvalState & state, + std::string_view drvName, const Bindings * attrs, - Value & v) + const PosIdx pos) { /* Check whether attributes should be passed as a JSON file. */ using nlohmann::json; std::optional jsonObject; - auto pos = v.determinePos(noPos); auto attr = attrs->find(state.sStructuredAttrs); if (attr != attrs->end() && state.forceBool(*attr->value, pos, @@ -1215,7 +1229,7 @@ static void derivationStrictInternal( } catch (UsageError &) { state.error( "invalid value '%s' for 'outputHashMode' attribute", s - ).atPos(v).debugThrow(); + ).atPos(pos).debugThrow(); } if (ingestionMethod == TextIngestionMethod {}) experimentalFeatureSettings.require(Xp::DynamicDerivations); @@ -1228,7 +1242,7 @@ static void derivationStrictInternal( for (auto & j : ss) { if (outputs.find(j) != outputs.end()) state.error("duplicate derivation output '%1%'", j) - .atPos(v) + .atPos(pos) .debugThrow(); /* !!! Check whether j is a valid attribute name. */ @@ -1237,13 +1251,13 @@ static void derivationStrictInternal( the resulting set (see state.sDrvPath). */ if (j == "drvPath") state.error("invalid derivation output name 'drvPath'") - .atPos(v) + .atPos(pos) .debugThrow(); outputs.insert(j); } if (outputs.empty()) state.error("derivation cannot have an empty set of outputs") - .atPos(v) + .atPos(pos) .debugThrow(); }; @@ -1367,12 +1381,12 @@ static void derivationStrictInternal( /* Do we have all required attributes? */ if (drv.builder == "") state.error("required attribute 'builder' missing") - .atPos(v) + .atPos(pos) .debugThrow(); if (drv.platform == "") state.error("required attribute 'system' missing") - .atPos(v) + .atPos(pos) .debugThrow(); /* Check whether the derivation name is valid. */ @@ -1384,7 +1398,7 @@ static void derivationStrictInternal( state.error( "derivation names are allowed to end in '%s' only if they produce a single derivation file", drvExtension - ).atPos(v).debugThrow(); + ).atPos(pos).debugThrow(); } if (outputHash) { @@ -1395,7 +1409,7 @@ static void derivationStrictInternal( if (outputs.size() != 1 || *(outputs.begin()) != "out") state.error( "multiple outputs are not supported in fixed-output derivations" - ).atPos(v).debugThrow(); + ).atPos(pos).debugThrow(); auto h = newHashAllowEmpty(*outputHash, outputHashAlgo); @@ -1415,7 +1429,7 @@ static void derivationStrictInternal( else if (contentAddressed || isImpure) { if (contentAddressed && isImpure) state.error("derivation cannot be both content-addressed and impure") - .atPos(v).debugThrow(); + .atPos(pos).debugThrow(); auto ha = outputHashAlgo.value_or(HashAlgorithm::SHA256); auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); @@ -1459,7 +1473,7 @@ static void derivationStrictInternal( state.error( "derivation produced no hash for output '%s'", i - ).atPos(v).debugThrow(); + ).atPos(pos).debugThrow(); auto outPath = state.store->makeOutputPath(i, *h, drvName); drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign( @@ -1477,6 +1491,15 @@ static void derivationStrictInternal( } } + return drv; +} + +static void derivationStrictInternalBuildResult( + EvalState & state, + const std::string & drvName, + const Derivation & drv, + Value & v) +{ /* Write the resulting term into the Nix store directory. */ auto drvPath = writeDerivation(*state.store, drv, state.repair); auto drvPathS = state.store->printStorePath(drvPath); diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh index 9f76975db8dc..c1cf6a987566 100644 --- a/src/libexpr/primops.hh +++ b/src/libexpr/primops.hh @@ -53,4 +53,28 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v); void makePositionThunks(EvalState & state, const PosIdx pos, Value & line, Value & column); +struct Derivation; + +/** + * Get the `name` field and attribute set of `builtins.derivationStrict` + * arguments + * + * Exposed for unit testing purposes. + */ +std::tuple +derivationStrictInternalGetNameAndAttrs( + EvalState & state, + const PosIdx pos, + Value & arg); + +/** + * @returns The derivation produced by `builtins.derivationStrict` + * + * Exposed for unit testing purposes. + */ +Derivation derivationStrictInternalReturning( + EvalState & state, + std::string_view name, + const Bindings * attrs, + const PosIdx pos); } diff --git a/tests/unit/libexpr-support/tests/libexpr.hh b/tests/unit/libexpr-support/tests/libexpr.hh index 1a4313990828..32e138bfac0a 100644 --- a/tests/unit/libexpr-support/tests/libexpr.hh +++ b/tests/unit/libexpr-support/tests/libexpr.hh @@ -19,6 +19,7 @@ namespace nix { static void SetUpTestSuite() { LibStoreTest::SetUpTestSuite(); initGC(); + settings.readOnlyMode = true; evalSettings.nixPath = {}; } diff --git a/tests/unit/libexpr/data/derivation/advanced-attributes-defaults.nix b/tests/unit/libexpr/data/derivation/advanced-attributes-defaults.nix index 51a8d0e7e1a9..be42c1720d02 100644 --- a/tests/unit/libexpr/data/derivation/advanced-attributes-defaults.nix +++ b/tests/unit/libexpr/data/derivation/advanced-attributes-defaults.nix @@ -1,4 +1,4 @@ -derivation { +{ name = "advanced-attributes-defaults"; system = "my-system"; builder = "/bin/bash"; diff --git a/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs-defaults.nix b/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs-defaults.nix index 0c13a76911f5..5b6de3623d5e 100644 --- a/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs-defaults.nix +++ b/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs-defaults.nix @@ -1,4 +1,4 @@ -derivation { +{ name = "advanced-attributes-structured-attrs-defaults"; system = "my-system"; builder = "/bin/bash"; diff --git a/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs.nix b/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs.nix index 0044b65fd410..6c71e3b6e46e 100644 --- a/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs.nix +++ b/tests/unit/libexpr/data/derivation/advanced-attributes-structured-attrs.nix @@ -13,7 +13,7 @@ let args = ["-c" "echo bar > $out"]; }; in -derivation { +{ inherit system; name = "advanced-attributes-structured-attrs"; builder = "/bin/bash"; diff --git a/tests/unit/libexpr/data/derivation/advanced-attributes.nix b/tests/unit/libexpr/data/derivation/advanced-attributes.nix index ff680c5677f5..5cb0ba521ba4 100644 --- a/tests/unit/libexpr/data/derivation/advanced-attributes.nix +++ b/tests/unit/libexpr/data/derivation/advanced-attributes.nix @@ -13,7 +13,7 @@ let args = ["-c" "echo bar > $out"]; }; in -derivation { +{ inherit system; name = "advanced-attributes"; builder = "/bin/bash"; diff --git a/tests/unit/libexpr/derivation.cc b/tests/unit/libexpr/derivation.cc index e0b594db5ada..9fa4ac35fc98 100644 --- a/tests/unit/libexpr/derivation.cc +++ b/tests/unit/libexpr/derivation.cc @@ -3,6 +3,7 @@ #include "tests/characterization.hh" #include "tests/libexpr.hh" #include "derivations.hh" +#include "primops.hh" namespace nix { @@ -17,29 +18,24 @@ class DerivationTest : public CharacterizationTest, public LibExprTest } }; -#define EXPR_DRV_TEST(NAME, STEM) \ - TEST_F(DerivationTest, Derivation_##NAME) \ - { \ - writeTest( \ - STEM ".drv", \ - [&]() -> Derivation { \ - Value v = eval(readFile(goldenMaster(STEM ".nix"))); \ - Symbol s = state.symbols.create("drvPath"); \ - auto attr = v.attrs() -> get(s); \ - state.forceValueDeep(*attr->value); \ - NixStringContext context; \ - auto storePath = state.coerceToStorePath(attr->pos, *attr->value, context, ""); \ - \ - return store->readDerivation(storePath); \ - }, \ - [&](const auto & file) { \ - auto s = readFile(file); \ - return parseDerivation(*store, std::move(s), STEM); \ - }, \ - [&](const auto & file, const auto & got) { \ - auto s = got.unparse(*store, false); \ - return writeFile(file, std::move(s)); \ - }); \ +#define EXPR_DRV_TEST(NAME, STEM) \ + TEST_F(DerivationTest, Derivation_##NAME) \ + { \ + writeTest( \ + STEM ".drv", \ + [&]() -> Derivation { \ + Value v = eval(readFile(goldenMaster(STEM ".nix"))); \ + auto [attrs, drvName, _namePos] = derivationStrictInternalGetNameAndAttrs(state, noPos, v); \ + return derivationStrictInternalReturning(state, drvName, attrs, noPos); \ + }, \ + [&](const auto & file) { \ + auto s = readFile(file); \ + return parseDerivation(*store, std::move(s), STEM); \ + }, \ + [&](const auto & file, const auto & got) { \ + auto s = got.unparse(*store, false); \ + return writeFile(file, std::move(s)); \ + }); \ } EXPR_DRV_TEST(advancedAttributes, "advanced-attributes"); diff --git a/tests/unit/libexpr/nix_api_expr.cc b/tests/unit/libexpr/nix_api_expr.cc index 92a6a1175378..6efed0348b4c 100644 --- a/tests/unit/libexpr/nix_api_expr.cc +++ b/tests/unit/libexpr/nix_api_expr.cc @@ -63,6 +63,11 @@ TEST_F(nix_api_expr_test, nix_expr_eval_drv) TEST_F(nix_api_expr_test, nix_build_drv) { + // FIXME this is ugly and will prevent running the tests in + // parallel. #5638 tracks + auto old = nix::settings.readOnlyMode; + nix::settings.readOnlyMode = false; + auto expr = R"(derivation { name = "myname"; system = builtins.currentSystem; builder = "/bin/sh"; @@ -103,6 +108,9 @@ TEST_F(nix_api_expr_test, nix_build_drv) // Clean up nix_store_path_free(drvStorePath); nix_store_path_free(outStorePath); + + // FIXME remove. see above + nix::settings.readOnlyMode = old; } TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_value) @@ -118,6 +126,11 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_value) TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build) { + // FIXME this is ugly and will prevent running the tests in + // parallel. #5638 tracks + auto old = nix::settings.readOnlyMode; + nix::settings.readOnlyMode = false; + auto expr = R"( derivation { name = "letsbuild"; system = builtins.currentSystem; @@ -131,10 +144,18 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build) ASSERT_EQ(nullptr, r); ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("failed with exit code 1"))); + + // FIXME remove. see above + nix::settings.readOnlyMode = old; } TEST_F(nix_api_expr_test, nix_expr_realise_context) { + // FIXME this is ugly and will prevent running the tests in + // parallel. #5638 tracks + auto old = nix::settings.readOnlyMode; + nix::settings.readOnlyMode = false; + // TODO (ca-derivations): add a content-addressed derivation output, which produces a placeholder auto expr = R"( '' @@ -189,6 +210,9 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context) EXPECT_THAT(names[2], testing::StrEq("not-actually-built-yet.drv")); nix_realised_string_free(r); + + // FIXME remove. see above + nix::settings.readOnlyMode = old; } const char * SAMPLE_USER_DATA = "whatever";