Merge pull request #9216 from obsidiansystems/addDrvOutputDependencies-pre

Add `builtins.addDrvOutputDependencies`

src/libexpr/primops/context.cc

@@ -30,20 +30,27 @@ static RegisterPrimOp primop_hasContext({
     .name = "__hasContext",
     .args = {"s"},
     .doc = R"(
-      Return `true` if string *s* has a non-empty context. The
+      Return `true` if string *s* has a non-empty context.
-      context can be obtained with
+      The context can be obtained with
       [`getContext`](#builtins-getContext).
+
+      > **Example**
+      >
+      > Many operations require a string context to be empty because they are intended only to work with "regular" strings, and also to help users avoid unintentionally loosing track of string context elements.
+      > `builtins.hasContext` can help create better domain-specific errors in those case.
+      >
+      > ```nix
+      > name: meta:
+      >
+      > if builtins.hasContext name
+      > then throw "package name cannot contain string context"
+      > else { ${name} = meta; }
+      > ```
     )",
     .fun = prim_hasContext
 });
 
 
-/* Sometimes we want to pass a derivation path (i.e. pkg.drvPath) to a
-   builder without causing the derivation to be built (for instance,
-   in the derivation that builds NARs in nix-push, when doing
-   source-only deployment).  This primop marks the string context so
-   that builtins.derivation adds the path to drv.inputSrcs rather than
-   drv.inputDrvs. */
 static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx pos, Value * * args, Value & v)
 {
     NixStringContext context;
@@ -66,11 +73,83 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p
 
 static RegisterPrimOp primop_unsafeDiscardOutputDependency({
     .name = "__unsafeDiscardOutputDependency",
-    .arity = 1,
+    .args = {"s"},
+    .doc = R"(
+      Create a copy of the given string where every "derivation deep" string context element is turned into a constant string context element.
+
+      This is the opposite of [`builtins.addDrvOutputDependencies`](#builtins-addDrvOutputDependencies).
+
+      This is unsafe because it allows us to "forget" store objects we would have otherwise refered to with the string context,
+      whereas Nix normally tracks all dependencies consistently.
+      Safe operations "grow" but never "shrink" string contexts.
+      [`builtins.addDrvOutputDependencies`] in contrast is safe because "derivation deep" string context element always refers to the underlying derivation (among many more things).
+      Replacing a constant string context element with a "derivation deep" element is a safe operation that just enlargens the string context without forgetting anything.
+
+      [`builtins.addDrvOutputDependencies`]: #builtins-addDrvOutputDependencies
+    )",
     .fun = prim_unsafeDiscardOutputDependency
 });
 
 
+static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, Value * * args, Value & v)
+{
+    NixStringContext context;
+    auto s = state.coerceToString(pos, *args[0], context, "while evaluating the argument passed to builtins.addDrvOutputDependencies");
+
+	auto contextSize = context.size();
+    if (contextSize != 1) {
+        throw EvalError({
+            .msg = hintfmt("context of string '%s' must have exactly one element, but has %d", *s, contextSize),
+            .errPos = state.positions[pos]
+        });
+    }
+    NixStringContext context2 {
+        (NixStringContextElem { std::visit(overloaded {
+            [&](const NixStringContextElem::Opaque & c) -> NixStringContextElem::DrvDeep {
+                if (!c.path.isDerivation()) {
+                    throw EvalError({
+                        .msg = hintfmt("path '%s' is not a derivation",
+                            state.store->printStorePath(c.path)),
+                        .errPos = state.positions[pos],
+                    });
+                }
+                return NixStringContextElem::DrvDeep {
+                    .drvPath = c.path,
+                };
+            },
+            [&](const NixStringContextElem::Built & c) -> NixStringContextElem::DrvDeep {
+                throw EvalError({
+                    .msg = hintfmt("`addDrvOutputDependencies` can only act on derivations, not on a derivation output such as '%1%'", c.output),
+                    .errPos = state.positions[pos],
+                });
+            },
+            [&](const NixStringContextElem::DrvDeep & c) -> NixStringContextElem::DrvDeep {
+                /* Reuse original item because we want this to be idempotent. */
+                return std::move(c);
+            },
+        }, context.begin()->raw) }),
+    };
+
+    v.mkString(*s, context2);
+}
+
+static RegisterPrimOp primop_addDrvOutputDependencies({
+    .name = "__addDrvOutputDependencies",
+    .args = {"s"},
+    .doc = R"(
+      Create a copy of the given string where a single consant string context element is turned into a "derivation deep" string context element.
+
+      The store path that is the constant string context element should point to a valid derivation, and end in `.drv`.
+
+      The original string context element must not be empty or have multiple elements, and it must not have any other type of element other than a constant or derivation deep element.
+      The latter is supported so this function is idempotent.
+
+      This is the opposite of [`builtins.unsafeDiscardOutputDependency`](#builtins-addDrvOutputDependencies).
+    )",
+    .fun = prim_addDrvOutputDependencies
+});
+
+
 /* Extract the context of a string as a structured Nix value.
 
    The context is represented as an attribute set whose keys are the

tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.err.exp

@@ -0,0 +1,10 @@
+error:
+       … while calling the 'addDrvOutputDependencies' builtin
+
+         at /pwd/lang/eval-fail-addDrvOutputDependencies-empty-context.nix:1:1:
+
+            1| builtins.addDrvOutputDependencies ""
+             | ^
+            2|
+
+       error: context of string '' must have exactly one element, but has 0

tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.nix

@@ -0,0 +1 @@
+builtins.addDrvOutputDependencies ""

tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.err.exp

@@ -0,0 +1,11 @@
+error:
+       … while calling the 'addDrvOutputDependencies' builtin
+
+         at /pwd/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.nix:18:4:
+
+           17|
+           18| in builtins.addDrvOutputDependencies combo-path
+             |    ^
+           19|
+
+       error: context of string '/nix/store/pg9yqs4yd85yhdm3f4i5dyaqp5jahrsz-fail.drv/nix/store/2dxd5frb715z451vbf7s8birlf3argbk-fail-2.drv' must have exactly one element, but has 2

tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.nix

@@ -0,0 +1,18 @@
+let
+  drv0 = derivation {
+    name = "fail";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "foo" ];
+  };
+
+  drv1 = derivation {
+    name = "fail-2";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "foo" ];
+  };
+
+  combo-path = "${drv0.drvPath}${drv1.drvPath}";
+
+in builtins.addDrvOutputDependencies combo-path

tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.err.exp

@@ -0,0 +1,11 @@
+error:
+       … while calling the 'addDrvOutputDependencies' builtin
+
+         at /pwd/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.nix:9:4:
+
+            8|
+            9| in builtins.addDrvOutputDependencies drv.outPath
+             |    ^
+           10|
+
+       error: `addDrvOutputDependencies` can only act on derivations, not on a derivation output such as 'out'

tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.nix

@@ -0,0 +1,9 @@
+let
+  drv = derivation {
+    name = "fail";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "foo" ];
+  };
+
+in builtins.addDrvOutputDependencies drv.outPath

tests/functional/lang/eval-okay-context-introspection.exp

@@ -1 +1 @@
-[ true true true true true true ]
+[ true true true true true true true true true true true true true ]

tests/functional/lang/eval-okay-context-introspection.nix

@@ -31,11 +31,29 @@ let
       (builtins.unsafeDiscardStringContext str)
       (builtins.getContext str);
 
+  # Only holds true if string context contains both a `DrvDeep` and
+  # `Opaque` element.
+  almostEtaRule = str:
+    str == builtins.addDrvOutputDependencies
+      (builtins.unsafeDiscardOutputDependency str);
+
+  addDrvOutputDependencies_idempotent = str:
+    builtins.addDrvOutputDependencies str ==
+    builtins.addDrvOutputDependencies (builtins.addDrvOutputDependencies str);
+
+  rules = str: [
+    (etaRule str)
+    (almostEtaRule str)
+    (addDrvOutputDependencies_idempotent str)
+  ];
+
 in [
   (legit-context == desired-context)
   (reconstructed-path == combo-path)
   (etaRule "foo")
-  (etaRule drv.drvPath)
   (etaRule drv.foo.outPath)
-  (etaRule (builtins.unsafeDiscardOutputDependency drv.drvPath))
+] ++ builtins.concatMap rules [
+  drv.drvPath
+  (builtins.addDrvOutputDependencies drv.drvPath)
+  (builtins.unsafeDiscardOutputDependency drv.drvPath)
 ]