From 88d8f6ac4817e44fba0a4f086ea0b8c06234cdd7 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 21 Jan 2023 23:50:09 -0500 Subject: [PATCH 1/2] Expand tests to reproduce #7655 The original `builtins.getContext` test from 1d757292d0cb78beec32fcdfe15c2944a4bc4a95 would have caught this. The problem is that b30be6b450f872f8be6dc8afa28f4b030fa8d1d1 adding `builtins.appendContext` modified that test to make it test too much at once, rather than adding a separate test. We now have isolated tests for both functions, and also a property test showing everything put together (in the form of an eta rule for strings with context). This is better coverage and properly reproduces the bug. --- .../lang/eval-okay-context-introspection.exp | 2 +- .../lang/eval-okay-context-introspection.nix | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/tests/lang/eval-okay-context-introspection.exp b/tests/lang/eval-okay-context-introspection.exp index 27ba77dda..03b400cc8 100644 --- a/tests/lang/eval-okay-context-introspection.exp +++ b/tests/lang/eval-okay-context-introspection.exp @@ -1 +1 @@ -true +[ true true true true true true ] diff --git a/tests/lang/eval-okay-context-introspection.nix b/tests/lang/eval-okay-context-introspection.nix index 43178bd2e..50a78d946 100644 --- a/tests/lang/eval-okay-context-introspection.nix +++ b/tests/lang/eval-okay-context-introspection.nix @@ -18,7 +18,24 @@ let }; }; - legit-context = builtins.getContext "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}"; + combo-path = "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}"; + legit-context = builtins.getContext combo-path; - constructed-context = builtins.getContext (builtins.appendContext "" desired-context); -in legit-context == constructed-context + reconstructed-path = builtins.appendContext + (builtins.unsafeDiscardStringContext combo-path) + desired-context; + + # Eta rule for strings with context. + etaRule = str: + str == builtins.appendContext + (builtins.unsafeDiscardStringContext str) + (builtins.getContext str); + +in [ + (legit-context == desired-context) + (reconstructed-path == combo-path) + (etaRule "foo") + (etaRule drv.drvPath) + (etaRule drv.foo.outPath) + (etaRule (builtins.unsafeDiscardOutputDependency drv.drvPath)) +] From 0afdf4084cc866610d0a0b6f46221680c8420cbf Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 21 Jan 2023 23:53:23 -0500 Subject: [PATCH 2/2] Fix #7655 We had some local variables left over from the older (more complicated) implementation of this function. They should all be unused, but one wasn't by mistake. Delete them all, and replace the one that was still in use as intended. --- src/libexpr/primops/context.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 4b7357495..4da648c3d 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -83,15 +83,13 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args, state.forceString(*args[0], context, pos); auto contextInfos = std::map(); for (const auto & p : context) { - Path drv; - std::string output; NixStringContextElem ctx = NixStringContextElem::parse(*state.store, p); std::visit(overloaded { [&](NixStringContextElem::DrvDeep & d) { contextInfos[d.drvPath].allOutputs = true; }, [&](NixStringContextElem::Built & b) { - contextInfos[b.drvPath].outputs.emplace_back(std::move(output)); + contextInfos[b.drvPath].outputs.emplace_back(std::move(b.output)); }, [&](NixStringContextElem::Opaque & o) { contextInfos[o.path].path = true;