From 12efe1d2b86d51896d17008d620059c05fa2af3b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 27 Jun 2023 12:10:36 -0400 Subject: [PATCH] Add `builtins.addDrvOutputDependencies` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://github.com/NixOS/nix/issues/7910#issuecomment-1544134386 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 Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com> Co-authored-by: Valentin Gagarin --- doc/manual/src/SUMMARY.md.in | 1 + doc/manual/src/glossary.md | 13 ++ doc/manual/src/language/string-context.md | 145 ++++++++++++++++++ src/libexpr/primops/context.cc | 100 ++++++++++-- ...rvOutputDependencies-empty-context.err.exp | 10 ++ ...addDrvOutputDependencies-empty-context.nix | 1 + ...putDependencies-multi-elem-context.err.exp | 11 ++ ...vOutputDependencies-multi-elem-context.nix | 18 +++ ...putDependencies-wrong-element-kind.err.exp | 11 ++ ...vOutputDependencies-wrong-element-kind.nix | 9 ++ .../lang/eval-okay-context-introspection.exp | 2 +- .../lang/eval-okay-context-introspection.nix | 22 ++- 12 files changed, 331 insertions(+), 12 deletions(-) create mode 100644 doc/manual/src/language/string-context.md create mode 100644 tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.err.exp create mode 100644 tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.nix create mode 100644 tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.err.exp create mode 100644 tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.nix create mode 100644 tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.err.exp create mode 100644 tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.nix diff --git a/doc/manual/src/SUMMARY.md.in b/doc/manual/src/SUMMARY.md.in index 60ebeb1384a8..97c2478e24df 100644 --- a/doc/manual/src/SUMMARY.md.in +++ b/doc/manual/src/SUMMARY.md.in @@ -30,6 +30,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) diff --git a/doc/manual/src/glossary.md b/doc/manual/src/glossary.md index d49d5e52e4b5..9d872464974f 100644 --- a/doc/manual/src/glossary.md +++ b/doc/manual/src/glossary.md @@ -217,6 +217,19 @@ [output path]: #gloss-output-path +- [deriving path]{#gloss-derived-path} + + A [store]-level expression (not Nix language expression) that denotes a [store object]. + There are two forms: + + - *constant*: A constant deriving path is just a [store path] + It can be made valid by copying it into the store, from the evaluator, command line interface or another store. + + - *output*: An output deriving path is a pair of a [store path] to a [derivation] and an [output] name. + + Deriving paths are necessary because in general, we do not know what the [output path] path of an [output] will be until it is [realise]d. + Deriving paths provide a way to refer to unrealised outputs in order to be able to tell Nix to realise them. + - [deriver]{#gloss-deriver} The [store derivation] that produced an [output path]. diff --git a/doc/manual/src/language/string-context.md b/doc/manual/src/language/string-context.md new file mode 100644 index 000000000000..58278b771f6f --- /dev/null +++ b/doc/manual/src/language/string-context.md @@ -0,0 +1,145 @@ +# String context + +> **Note** +> +> This is an advanced topic. +> The Nix language is designed to be used without the programmer consciously dealing with string contexts or even knowing what they are. + +A string in the Nix language is not just a sequence of characters like strings in other languages. +It is actually a pair of a sequence of characters and a *string context*. +The string context is an (unordered) set of *string context elements*. + +The purpose of string contexts is to collect non-string values attached to 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 line with this goal, string contexts are *not* explicitly manipulated in idiomatic Nix code. +> Strings with non-empty context are only concatenated and eventually passed to `builtins.derivation`. +> Plain strings with empty contexts are the only ones to be inspected, e.g. using comparison with `==`. + +String context elements come in different 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 +[deriving paths](@docroot@/glossary.md#gloss-deriving-path); +those are just the names of the two kinds of deriving path. +See the documentation on deriving 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 tell whether a string has a non-empty context. + +When more granular information is needed, [`builtins.getContext`] can be used. +It creates an [attribute set] representing the string context, which can be inspected as usual. + +[`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 a string, but with an empty string context. +The returned string can be used in more ways, e.g. by operators that require the string context to be empty. +The requirement to explicitly discard the string context in such use cases helps ensure that string context elements are not lost by mistake. +The "unsafe" marker is only there to remind that Nix normally guarantees that dependencies are tracked, whereas the returned string has lost them. + +## Constructing string contexts + +[`builtins.appendContext`] will create a 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. +A string with arbitrary contexts can be made like this: + +1. Create a string with the desired string context elements. +2. Dump its context with [`builtins.getContext`]. +3. Combine it 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 [deriving path]; +a constant deriving 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; +> }; +> } +> ``` + +[deriving path]: @docroot@/glossary.md#gloss-deriving-path +[store path]: @docroot@/glossary.md#gloss-store-path +[`builtins.storePath`]: ./builtins.md#builtins-storePath + +## Output string context elements + +> **Example** +> +> This is best illustrated with a built-in function that is still experimental: [`builtins.ouputOf`]. +> This example will *not* work the stable Nix! +> +> ```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 + +The best way to illustrate this is with [`builtins.addDrvOutputDependencies`]. +We take a regular constant string context element pointing to a derivation, and transform it into 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 marked unsafe because it "forgets" the dependencies on the outputs, whereas Nix normally tracks all dependencies consistently. +[`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.md#builtins-addDrvOutputDependencies +[`builtins.unsafeDiscardOutputDependency`]: ./builtins.md#builtins-unsafeDiscardOutputDependency diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index e8542503a42b..5793b89f6120 100644 --- a/src/libexpr/primops/context.cc +++ b/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 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 + [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 diff --git a/tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.err.exp b/tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.err.exp new file mode 100644 index 000000000000..ad91a22aa5b3 --- /dev/null +++ b/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 diff --git a/tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.nix b/tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.nix new file mode 100644 index 000000000000..dc9ee3ba2e59 --- /dev/null +++ b/tests/functional/lang/eval-fail-addDrvOutputDependencies-empty-context.nix @@ -0,0 +1 @@ +builtins.addDrvOutputDependencies "" diff --git a/tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.err.exp b/tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.err.exp new file mode 100644 index 000000000000..bb389db4e4c6 --- /dev/null +++ b/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 diff --git a/tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.nix b/tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.nix new file mode 100644 index 000000000000..dbde264dfaeb --- /dev/null +++ b/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 diff --git a/tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.err.exp b/tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.err.exp new file mode 100644 index 000000000000..07038111822f --- /dev/null +++ b/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' diff --git a/tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.nix b/tests/functional/lang/eval-fail-addDrvOutputDependencies-wrong-element-kind.nix new file mode 100644 index 000000000000..e379e1d9598b --- /dev/null +++ b/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 diff --git a/tests/functional/lang/eval-okay-context-introspection.exp b/tests/functional/lang/eval-okay-context-introspection.exp index 03b400cc8862..a136b0035e0a 100644 --- a/tests/functional/lang/eval-okay-context-introspection.exp +++ b/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 ] diff --git a/tests/functional/lang/eval-okay-context-introspection.nix b/tests/functional/lang/eval-okay-context-introspection.nix index 50a78d946e76..8886cf32e94b 100644 --- a/tests/functional/lang/eval-okay-context-introspection.nix +++ b/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) ]