From 8a539424c823c44ab6f6869d3d1bd2b02e5d30a3 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 11 May 2025 22:50:32 +0200 Subject: [PATCH] libexpr: drop support for dyn-drv string context string context doesn't need any tests because it's never persisted or shown to the user. getting rid of recursive string context means that the context string parsers can be a lot simpler from here on forward. Change-Id: I58443679ad76c0f28ea5f4eb8bfb3874f270e764 --- lix/libexpr/value/context.cc | 72 ++++++++++------------------- lix/libexpr/value/context.hh | 6 +-- lix/libstore/derived-path.cc | 6 +-- tests/unit/libexpr/value/context.cc | 37 --------------- tests/unit/libstore/derived-path.cc | 35 +------------- 5 files changed, 30 insertions(+), 126 deletions(-) diff --git a/lix/libexpr/value/context.cc b/lix/libexpr/value/context.cc index eca91cdd0..479cc1720 100644 --- a/lix/libexpr/value/context.cc +++ b/lix/libexpr/value/context.cc @@ -2,33 +2,10 @@ namespace nix { -NixStringContextElem NixStringContextElem::parse( - std::string_view s0, - const ExperimentalFeatureSettings & xpSettings) +NixStringContextElem NixStringContextElem::parse(std::string_view s0) { std::string_view s = s0; - std::function parseRest; - parseRest = [&]() -> SingleDerivedPath { - // Case on whether there is a '!' - size_t index = s.find("!"); - if (index == std::string_view::npos) { - return SingleDerivedPath::Opaque { - .path = StorePath { s }, - }; - } else { - std::string output { s.substr(0, index) }; - // Advance string to parse after the '!' - s = s.substr(index + 1); - auto drv = make_ref(parseRest()); - drvRequireExperiment(*drv, xpSettings); - return SingleDerivedPath::Built { - .drvPath = std::move(drv), - .output = std::move(output), - }; - } - }; - if (s.size() == 0) { throw BadNixStringContextElem(s0, "String context element should never be an empty string"); @@ -40,14 +17,20 @@ NixStringContextElem NixStringContextElem::parse( s = s.substr(1); // Find *second* '!' - if (s.find("!") == std::string_view::npos) { + size_t index = s.find("!"); + if (index == std::string_view::npos) { throw BadNixStringContextElem(s0, "String content element beginning with '!' should have a second '!'"); } - return std::visit( - [&](auto x) -> NixStringContextElem { return std::move(x); }, - parseRest()); + std::string output { s.substr(0, index) }; + // Advance string to parse after the '!' + s = s.substr(index + 1); + auto drv = make_ref(SingleDerivedPath::Opaque{StorePath{s}}); + return SingleDerivedPath::Built{ + .drvPath = std::move(drv), + .output = std::move(output), + }; } case '=': { return NixStringContextElem::DrvDeep { @@ -60,9 +43,9 @@ NixStringContextElem NixStringContextElem::parse( throw BadNixStringContextElem(s0, "String content element not beginning with '!' should not have a second '!'"); } - return std::visit( - [&](auto x) -> NixStringContextElem { return std::move(x); }, - parseRest()); + return SingleDerivedPath::Opaque{ + .path = StorePath{s}, + }; } } } @@ -71,27 +54,22 @@ std::string NixStringContextElem::to_string() const { std::string res; - std::function toStringRest; - toStringRest = [&](auto & p) { - std::visit(overloaded { - [&](const SingleDerivedPath::Opaque & o) { - res += o.path.to_string(); - }, - [&](const SingleDerivedPath::Built & o) { - res += o.output; - res += '!'; - toStringRest(*o.drvPath); - }, - }, p.raw()); - }; - std::visit(overloaded { [&](const NixStringContextElem::Built & b) { res += '!'; - toStringRest(b); + res += b.output; + res += '!'; + std::visit(overloaded { + [&](const SingleDerivedPath::Opaque & o2) { + res += o2.path.to_string(); + }, + [&](const SingleDerivedPath::Built & o) { + assert(false && "dynamic derivations shouldn't exist in string context any more"); + }, + }, b.drvPath->raw()); }, [&](const NixStringContextElem::Opaque & o) { - toStringRest(o); + res += o.path.to_string(); }, [&](const NixStringContextElem::DrvDeep & d) { res += '='; diff --git a/lix/libexpr/value/context.hh b/lix/libexpr/value/context.hh index bc48b66fa..b7024d0fa 100644 --- a/lix/libexpr/value/context.hh +++ b/lix/libexpr/value/context.hh @@ -69,12 +69,8 @@ struct NixStringContextElem { * - ‘’ * - ‘=’ * - ‘!!’ - * - * @param xpSettings Stop-gap to avoid globals during unit tests. */ - static NixStringContextElem parse( - std::string_view s, - const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); + static NixStringContextElem parse(std::string_view s); std::string to_string() const; }; diff --git a/lix/libstore/derived-path.cc b/lix/libstore/derived-path.cc index ff34d719f..66dc664df 100644 --- a/lix/libstore/derived-path.cc +++ b/lix/libstore/derived-path.cc @@ -210,11 +210,9 @@ static DerivedPathT parseDerivedPath( return DerivedPathT::Opaque::parse(store, s); } else { auto path = DerivedPathT::Built::parse(store, - make_ref(parseDerivedPath( + make_ref(DerivedPathT::Opaque::parse( store, - s.substr(0, n), - separator, - xpSettings)), + s.substr(0, n))), s.substr(n + 1), xpSettings); diff --git a/tests/unit/libexpr/value/context.cc b/tests/unit/libexpr/value/context.cc index 8ea4cd1d4..1e7857aa8 100644 --- a/tests/unit/libexpr/value/context.cc +++ b/tests/unit/libexpr/value/context.cc @@ -84,43 +84,6 @@ TEST(NixStringContextElemTest, built_opaque) { ASSERT_EQ(elem.to_string(), built); } -/** - * Round trip (string <-> data structure) test for a more complex, - * inductive `NixStringContextElem::Built`. - */ -TEST(NixStringContextElemTest, built_built) { - /** - * We set these in tests rather than the regular globals so we don't have - * to worry about race conditions if the tests run concurrently. - */ - ExperimentalFeatureSettings mockXpSettings; - mockXpSettings.experimentalFeatures.override( - ExperimentalFeatures{} | Xp::DynamicDerivations | Xp::CaDerivations - ); - - std::string_view built = "!foo!bar!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv"; - auto elem = NixStringContextElem::parse(built, mockXpSettings); - auto * p = std::get_if(&elem.raw); - ASSERT_TRUE(p); - ASSERT_EQ(p->output, "foo"); - auto * drvPath = std::get_if(&*p->drvPath); - ASSERT_TRUE(drvPath); - ASSERT_EQ(drvPath->output, "bar"); - ASSERT_EQ(*drvPath->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque { - .path = StorePath { built.substr(9) }, - })); - ASSERT_EQ(elem.to_string(), built); -} - -/** - * Without the right experimental features enabled, we cannot parse a - * complex inductive string context element. - */ -TEST(NixStringContextElemTest, built_built_xp) { - ASSERT_THROW( - NixStringContextElem::parse("!foo!bar!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv"), MissingExperimentalFeature); -} - #ifndef COVERAGE RC_GTEST_PROP( diff --git a/tests/unit/libstore/derived-path.cc b/tests/unit/libstore/derived-path.cc index 9f296a557..3c9219bdc 100644 --- a/tests/unit/libstore/derived-path.cc +++ b/tests/unit/libstore/derived-path.cc @@ -44,42 +44,11 @@ TEST_F(DerivedPathTest, built_opaque) { ASSERT_EQ(elem.to_string(*store), built); } -/** - * Round trip (string <-> data structure) test for a more complex, - * inductive `DerivedPath::Built`. - */ +// dynamic derivations are no longer supported TEST_F(DerivedPathTest, built_built) { - /** - * We set these in tests rather than the regular globals so we don't have - * to worry about race conditions if the tests run concurrently. - */ - ExperimentalFeatureSettings mockXpSettings; - mockXpSettings.experimentalFeatures.override( - ExperimentalFeatures{} | Xp::DynamicDerivations | Xp::CaDerivations - ); - - std::string_view built = "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv^foo^bar,baz"; - auto elem = DerivedPath::parse(*store, built, mockXpSettings); - auto * p = std::get_if(&elem); - ASSERT_TRUE(p); - ASSERT_EQ(p->outputs, ((OutputsSpec) OutputsSpec::Names { "bar", "baz" })); - auto * drvPath = std::get_if(&*p->drvPath); - ASSERT_TRUE(drvPath); - ASSERT_EQ(drvPath->output, "foo"); - ASSERT_EQ(*drvPath->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque { - .path = store->parseStorePath(built.substr(0, 49)), - })); - ASSERT_EQ(elem.to_string(*store), built); -} - -/** - * Without the right experimental features enabled, we cannot parse a - * complex inductive derived path. - */ -TEST_F(DerivedPathTest, built_built_xp) { ASSERT_THROW( DerivedPath::parse(*store, "/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv^foo^bar,baz"), - MissingExperimentalFeature); + BadStorePath); } /**