Add builtins.addDrvOutputDependencies

The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>

doc/manual/src/SUMMARY.md.in

@@ -31,6 +31,7 @@
   - [Language Constructs](language/constructs.md)
     - [String interpolation](language/string-interpolation.md)
     - [Lookup path](language/constructs/lookup-path.md)
+    - [String context](language/string-context.md)
   - [Operators](language/operators.md)
   - [Derivations](language/derivations.md)
     - [Advanced Attributes](language/advanced-attributes.md)

doc/manual/src/glossary.md

@@ -216,6 +216,18 @@
 
   [output path]: #gloss-output-path
 
+- [derived path]{#gloss-derived-path}
+
+  A [store]-level expression (not Nix language expression) that denotes a [store object].
+  There are two forms:
+
+  - *constant*: A constant derived path is just a [store path]
+
+  - *output*: An output derived path is a pair of a [store path] to a [derivation] and an [output] name.
+
+  Derived paths are necessary because in general, we do not know what the [output path] path of an [output] will buntil it is [realise]d.
+  We need a way to refer to unrelised outputs in order to be able to tell Nix to realise them!
+
 - [deriver]{#gloss-deriver}
 
   The [store derivation] that produced an [output path].

doc/manual/src/language/string-context.md

@@ -0,0 +1,137 @@
+# String context
+
+Strings in the Nix language are not just a sequences of characters like strings in other languages.
+They are actually pairs of sequences of characters and what is known as a *string context*.
+The string context is an (unordered) set of *string context elements*.
+
+String contexts are not intended to be intentionally manipulated.
+Rather, they are supposed to collect non-string values combined with strings via
+[string concatenation](./operators.md#string-concatenation),
+[string interpolation](./string-interpolation.md),
+and similar operations.
+The idea is that a user can combine together values to create a build recipe without manually keeping track of where the "ingredients" come from, and then the Nix language does that bookkeeping implicitly to come up with the right derivation inputs.
+In this idiom, string with context (i.e. with a non-empty string context) are concatenated together without their contents being inspected.
+
+String context elements come in 3 forms:
+
+- [*constant*]{#string-context-element-constant}
+- [*output*]{#string-context-element-output}
+- [*derivation deep*]{#string-context-element-derivation-deep}
+
+*Constant* and *output* string contexts elements are just
+[derived paths](@docroot@/glossary.md#gloss-derived-path);
+those are just the names of the two kinds of derived path.
+See the documentation on derived paths for further details.
+
+*derivation deep* is an advanced feature intended to be used with the
+[`exportReferencesGraph`](./advanced-attributes.html#adv-attr-exportReferencesGraph)
+advanced derivation feature.
+A *derivation deep* string context element is a derivation path, and refers to both its outputs and the entire build closure of that derivation:
+all its outputs, all the other derivations the given derivation depends on, and all their outputs too.
+
+## Inspecting string contexts
+
+Most basically, [`builtins.hasContext`] will whether a string as a non-empty context.
+
+When more granualr information than merely whether the string context is empty is needed, [`builtins.getContext`] can be used.
+It creates an [attribute set] representing the string context; the attribute set can be inspected normally.
+
+[`builtins.hasContext`]: ./builtins.md#builtins-hasContext
+[`builtins.getContext`]: ./builtins.md#builtins-getContext
+[attribute set]: ./values.md#attribute-set
+
+## Clearing string contexts
+
+[`buitins.unsafeDiscardStringContext`](./builtins.md#) will make a copy of another string, but with an empty string context.
+The new string can be inspected in more ways, e.g. by operators that require that the string context is empty.
+Explicitly discarding the string context and then expecting it helps ensure that string context elements are not lost by mistake.
+
+## Constructing string contexts
+
+[`builtins.appendContext`] will create copy of a string but with additional string context elements.
+The context is specified explicitly by an [attribute set] in the format that [`builtins.hasContext`] produces.
+We there can create strings arbitrary contexts in 3 steps:
+
+1. Create strings with the string context elements we want
+2. Dump their contexts with [`builtins.getContext`]
+3. Combine them together with a base string and repeated [`builtins.appendContext`] calls.
+
+The remainder of this section will focus on step 1: making strings with individual string context elements on which to apply `builtins.getContext`.
+
+[`builtins.appendContext`]: ./builtins.md#builtins-appendContext
+
+## Constant string context elements
+
+A constant string context element is just a constant [derived path];
+a constant derived path is just a [store path].
+We therefore want to use [`builtins.storePath`] to create a string with a single constant string context element:
+
+> **Example**
+>
+> ```nix
+> builtins.getContext (builtins.storePath "/nix/store/wkhdf9jinag5750mqlax6z2zbwhqb76n-hello-2.10")
+> ```
+> evaluates to
+> ```nix
+> {
+>   "/nix/store/wkhdf9jinag5750mqlax6z2zbwhqb76n-hello-2.10" = {
+>     path = true;
+>   };
+> }
+> ```
+
+[derived path]: @docroot@/glossary.md#gloss-derived-path
+[store path]: @docroot@/glossary.md#gloss-store-path
+[`builtins.storePath`]: ./builtins.md#builtins-storePath
+
+## Output string context elements
+
+The best way to illustrate this with a builtin function that is still experimental: [`builtins.ouputOf`].
+This example will *not* work the stable Nix!
+
+> **Example**
+>
+> ```nix
+> builtins.getContext
+>   (builtins.outputOf
+>     (builtins.storePath "/nix/store/fvchh9cvcr7kdla6n860hshchsba305w-hello-2.12.drv")
+>     "out")
+> ```
+> evaluates to
+> ```nix
+> {
+>   "/nix/store/fvchh9cvcr7kdla6n860hshchsba305w-hello-2.12.drv" = {
+>     outputs = [ "out" ];
+>   };
+> }
+> ```
+
+[`builtins.outputOf`]: ./builtins.md#builtins-outputOf
+
+## "Derivation deep" string context elements
+
+These best way to illustrate this is with [`builtins.addDrvOutputDependencies`].
+We take a regular constant string context element pointing to a derivation, and transform it unto a "Derivation deep" string context element.
+
+> **Example**
+>
+> ```nix
+> builtins.getContext
+>   (builtins.addDrvOutputDependencies
+>     (builtins.storePath "/nix/store/fvchh9cvcr7kdla6n860hshchsba305w-hello-2.12.drv"))
+> ```
+> evaluates to
+> ```nix
+> {
+>   "/nix/store/fvchh9cvcr7kdla6n860hshchsba305w-hello-2.12.drv" = {
+>     allOutputs = true;
+>   };
+> }
+> ```
+
+[`builtins.unsafeDiscardOutputDependency`] does this the opposite of [`builtins.addDrvOutputDependencies`], but is not prefered because it "weakens" rather than "strengens" the string context.
+What is meant by that is that since the "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" one is a safe operation that just enlargens the string context without forgetting anything.
+
+[`builtins.addDrvOutputDependencies`]: ./builtins.md#builtins-addDrvOutputDependencies
+[`builtins.unsafeDiscardOutputDependency`]: ./builtins.md#builtins-unsafeDiscardOutputDependency

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
-      context can be obtained with
+      Return `true` if string *s* has a non-empty context.
+      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,86 @@ 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](@docroot@/language/string-context.md#string-context-element-derivation-deep)
+      string context element is turned into a
+      [constant](@docroot@/language/string-context.md#string-context-element-constant)
+      string context element.
+
+      This is unsafe because it allows us to "forgot" store objects we would have otherwise refered to with the string context.
+      Safe operations "grow" but never "shrink" string contexts.
+
+      Opposite of [`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 derivation outputs"),
+                    .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
+      [constant](@docroot@/language/string-context.md#string-context-element-constant)
+      string context element is turned into a
+      [derivation deep](@docroot@/language/string-context.md#string-context-element-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.
+
+      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/build.sh

@@ -66,7 +66,7 @@ nix build -f multiple-outputs.nix --json 'e.a_a.outPath' --no-link | jq --exit-s
 '
 
 # Illegal type of string context
-expectStderr 1 nix build -f multiple-outputs.nix 'e.a_a.drvPath' \
+expectStderr 1 nix build --impure --expr 'builtins.addDrvOutputDependencies (import ./multiple-outputs.nix).e.a_a.drvPath' \
   | grepQuiet "has a context which refers to a complete source and binary closure."
 
 # No string context
@@ -77,7 +77,7 @@ expectStderr 1 nix build --expr '""' --no-link \
 expectStderr 1 nix build --impure --expr 'with (import ./multiple-outputs.nix).e.a_a; "${drvPath}${outPath}"' --no-link \
   | grepQuiet "has 2 entries in its context. It should only have exactly one entry"
 
-nix build --impure --json --expr 'builtins.unsafeDiscardOutputDependency (import ./multiple-outputs.nix).e.a_a.drvPath' --no-link | jq --exit-status '
+nix build --json -f multiple-outputs.nix e.a_a.drvPath --no-link | jq --exit-status '
   (.[0] | match(".*multiple-outputs-e.drv"))
 '
 

tests/functional/config.nix.in

@@ -16,15 +16,32 @@ rec {
 
   shared = builtins.getEnv "_NIX_TEST_SHARED";
 
-  mkDerivation = args:
-    derivation ({
+  mkDerivation = args: let
+
+    drv = derivation ({
       inherit system;
       builder = shell;
       args = ["-e" args.builder or (builtins.toFile "builder-${args.name}.sh" ''
         if [ -e "$NIX_ATTRS_SH_FILE" ]; then source $NIX_ATTRS_SH_FILE; fi;
         eval "$buildCommand"
       '')];
       PATH = path;
-    } // caArgs // removeAttrs args ["builder" "meta"])
-    // { meta = args.meta or {}; };
+    } // caArgs // removeAttrs args ["builder" "meta"]);
+
+    fixUp = value: value // outputsMap // {
+      # Not unsafe, and we will add back if we need it.
+      # Nixpkgs should do this too, when `addDrvOutputDependencies` makes it into the NixOS stable release.
+      drvPath = (builtins.unsafeDiscardOutputDependency value.drvPath);
+    };
+
+    outputsMap = builtins.listToAttrs (map
+      (name: {
+        inherit name;
+        value = fixUp drv.${name};
+      })
+      (drv.outputs or []));
+
+  in fixUp drv // {
+    meta = args.meta or {};
+  };
 }

tests/functional/dyn-drv/eval-outputOf.sh

@@ -20,15 +20,15 @@ expectStderr 1 nix --experimental-features 'nix-command dynamic-derivations' eva
 #
 # Like the above, this is a restriction we could relax later.
 expectStderr 1 nix --experimental-features 'nix-command dynamic-derivations' eval --impure --expr \
-    'builtins.outputOf (import ../dependencies.nix {}).drvPath "out"' \
+    'builtins.outputOf (builtins.addDrvOutputDependencies (import ../dependencies.nix {}).drvPath) "out"' \
     | grepQuiet "has a context which refers to a complete source and binary closure. This is not supported at this time"
 
 # Test using `builtins.outputOf` with static derivations
 testStaticHello () {
     nix eval --impure --expr \
         'with (import ./text-hashed-output.nix); let
            a = hello.outPath;
-           b = builtins.outputOf (builtins.unsafeDiscardOutputDependency hello.drvPath) "out";
+           b = builtins.outputOf hello.drvPath "out";
          in builtins.trace a
            (builtins.trace b
              (assert a == b; null))'
@@ -53,7 +53,7 @@ NIX_TESTS_CA_BY_DEFAULT=1 testStaticHello
 nix eval --impure --expr \
     'with (import ./text-hashed-output.nix); let
        a = producingDrv.outPath;
-       b = builtins.outputOf (builtins.builtins.unsafeDiscardOutputDependency producingDrv.drvPath) "out";
+       b = builtins.outputOf producingDrv.drvPath "out";
      in builtins.trace a
        (builtins.trace b
          (assert a == b; null))'
@@ -67,7 +67,7 @@ testDynamicHello () {
     nix eval --impure --expr \
         'with (import ./text-hashed-output.nix); let
            a = builtins.outputOf producingDrv.outPath "out";
-           b = builtins.outputOf (builtins.outputOf (builtins.unsafeDiscardOutputDependency producingDrv.drvPath) "out") "out";
+           b = builtins.outputOf (builtins.outputOf producingDrv.drvPath "out") "out";
          in builtins.trace a
            (builtins.trace b
              (assert a == b; null))'

tests/functional/dyn-drv/recursive-mod-json.nix

@@ -8,7 +8,7 @@ mkDerivation rec {
 
   requiredSystemFeatures = [ "recursive-nix" ];
 
-  drv = builtins.unsafeDiscardOutputDependency (import ./text-hashed-output.nix).hello.drvPath;
+  drv = (import ./text-hashed-output.nix).hello.drvPath;
 
   buildCommand = ''
     export NIX_CONFIG='experimental-features = nix-command ca-derivations'

tests/functional/dyn-drv/text-hashed-output.nix

@@ -17,7 +17,7 @@ rec {
     name = "hello.drv";
     buildCommand = ''
       echo "Copying the derivation"
-      cp ${builtins.unsafeDiscardOutputDependency hello.drvPath} $out
+      cp ${hello.drvPath} $out
     '';
     __contentAddressed = true;
     outputHashMode = "text";