Skip to content
Open
17 changes: 15 additions & 2 deletions doc/manual/src/release-notes/rl-next.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
# Release X.Y (202?-??-??)

- Two new builtin functions,
- A pair of builtin functions,
[`builtins.parseFlakeRef`](@docroot@/language/builtins.md#builtins-parseFlakeRef)
and
[`builtins.flakeRefToString`](@docroot@/language/builtins.md#builtins-flakeRefToString),
have been added.
has been added.
These functions are useful for converting between flake references encoded as attribute sets and URLs.

- Function introspection in the language has been extended to allow more compatibility logic to be written.

- [`builtins.functionOpen`](@docroot@/language/builtins.md#builtins-functionOpen): whether arbitrary attributes can be passed to the function; a boolean, or `null` for plain lambdas like `x: x`.
- [`builtins.functionBindsAllAttrs`](@docroot@/language/builtins.md#builtins-functionBindsAllAttrs): whether the function puts the whole attrset into a variable with `@`.

The combination of being closed, but binding all attributes is not forward compatible and can now be reported as part of migrations that add an attribute to a function call.

- [`builtins.functionStrict`](@docroot@/language/builtins.md#builtins-functionStrict): whether the function is written using strict syntax, such as `{ ... }: foo`. The fact that this function is strict in its argument is often forgotten, so this allows library or DSL authors to detect and report it in places where it might be common to encounter this.

- [`builtins.toJSON`](@docroot@/language/builtins.md#builtins-parseFlakeRef) now prints [--show-trace](@docroot@/command-ref/conf-file.html#conf-show-trace) items for the path in which it finds an evaluation error.

- Error messages regarding malformed input to [`derivation add`](@docroot@/command-ref/new-cli/nix3-derivation-add.md) are now clearer and more detailed.
Expand All @@ -23,6 +32,10 @@
- Introduce a new [`outputOf`](@docroot@/language/builtins.md#builtins-outputOf) builtin.
It is part of the [`dynamic-derivations`](@docroot@/contributing/experimental-features.md#xp-feature-dynamic-derivations) experimental feature.

- The flake output function now accepts a parameter `meta`, which gives access to `sourceInfo` and such parameters in a robust manner, even when `self` can not be evaluated. This helps frameworks report error messages with locations, as trying to access the location previously resulted in an unhelpful infinite recursion when the error condition caused `self` to fail.

If an `outputs` function has a binding for all attributes (using `@`), you will be asked to add an ellipsis to avoid later confusion as to why `meta` is missing - which is necessary because of a limitation of Nix function semantics.

- Flake follow paths at depths greater than 2 are now handled correctly, preventing "follows a non-existent input" errors.

- [`nix-store --query`](@docroot@/command-ref/nix-store/query.md) gained a new type of query: `--valid-derivers`. It returns all `.drv` files in the local store that *can be* used to build the output passed in argument.
Expand Down
67 changes: 57 additions & 10 deletions src/libexpr/flake/call-flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,25 @@ lockFileStr: rootSrc: rootSubdir:

let

warn = msg: builtins.trace "${emphasize "warning"}: ${msg}";
emphasize = x: "${x}";
optional = cond: thing: if cond then [ thing ] else [];

lockFile = builtins.fromJSON lockFileStr;

allNodes =
builtins.mapAttrs
(key: node:
let
# Flakes should be interchangeable regardless of whether they're at the root, so use with care.
isRoot = key == lockFile.root;

sourceInfo =
if key == lockFile.root
if isRoot
then rootSrc
else fetchTree (node.info or {} // removeAttrs node.locked ["dir"]);

subdir = if key == lockFile.root then rootSubdir else node.locked.dir or "";
subdir = if isRoot then rootSubdir else node.locked.dir or "";

outPath = sourceInfo + ((if subdir == "" then "" else "/") + subdir);

Expand Down Expand Up @@ -43,7 +49,54 @@ let
(resolveInput lockFile.nodes.${nodeName}.inputs.${builtins.head path})
(builtins.tail path);

outputs = flake.outputs (inputs // { self = result; });
# Attributes that are added to the final representation of the flake.
# NB: `attrNames extraAttributes` must be lazy in `outputs` (tested). Values may depend on `outputs`.
extraAttributes =
sourceInfo
// {
# This shadows the sourceInfo.outPath
inherit outPath;

inherit inputs; inherit outputs; inherit sourceInfo; _type = "flake";
};

meta = {
# The source root, which may not correspond to the flake directory.
inherit sourceInfo;
# The base directory of the flake
inherit subdir;
# Extra attributes in the final representation of the flake, added to result of the output function
inherit extraAttributes;
# Extra inputs to the output function
inherit extraArguments;
};

# NB: `attrNames arguments` must be lazy in `outputs` (tested).
arguments = inputs // extraArgumentsCompat;

# Find out if we have a conflict between a missing meta argument and the need for `meta` attr to be added to the @ binding.
# If possible, fix it up without complaining.
extraArgumentsCompat = builtins.removeAttrs extraArguments (
let
# NB: some of these builtins can return null, hence the ==
isClosed = builtins.functionOpen flake.outputs == false;
acceptsMeta = !isClosed || (builtins.functionArgs flake.outputs)?meta;
canRemove = isClosed && (builtins.functionBindsAllAttrs flake.outputs == false);

removals = if acceptsMeta then [] else [ "meta" ];
checked = if isRoot && !acceptsMeta && !canRemove then warning else x: x;
warning = warn
"in flake ${toString outPath}: The flake's ${emphasize "outputs"} function does not accept the ${emphasize "meta"} argument.\nThis will become an error.\nPlease add ellipsis (${emphasize "..."}) to the function header for it to be compatible with both dated and upcoming versions of Flakes. Example use of ellipsis: ${emphasize "outputs = { self, ... }: "}.";
in
checked removals
);

extraArguments = {
self = result;
inherit meta;
};

outputs = flake.outputs arguments;

result =
outputs
Expand All @@ -52,13 +105,7 @@ let
# sourceInfo does not necessarily match the outPath of the flake,
# as the flake may be in a subdirectory of a source.
# This is shadowed in the next //
// sourceInfo
// {
# This shadows the sourceInfo.outPath
inherit outPath;

inherit inputs; inherit outputs; inherit sourceInfo; _type = "flake";
};
// extraAttributes;

in
if node.flake or true then
Expand Down
6 changes: 5 additions & 1 deletion src/libexpr/flake/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
expectType(state, nAttrs, *value, pos);

for (nix::Attr & inputAttr : *(*value).attrs) {

if (inputAttr.name == state.sSelf || inputAttr.name == state.sMeta)
throw Error("flake input name '%s' is reserved", state.symbols[inputAttr.name]);

inputs.emplace(state.symbols[inputAttr.name],
parseFlakeInput(state,
state.symbols[inputAttr.name],
Expand Down Expand Up @@ -244,7 +248,7 @@ static Flake getFlake(

if (outputs->value->isLambda() && outputs->value->lambda.fun->hasFormals()) {
for (auto & formal : outputs->value->lambda.fun->formals->formals) {
if (formal.name != state.sSelf)
if (!(formal.name == state.sSelf || formal.name == state.sMeta))
flake.inputs.emplace(state.symbols[formal.name], FlakeInput {
.ref = parseFlakeRef(state.symbols[formal.name])
});
Expand Down
103 changes: 103 additions & 0 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2750,6 +2750,109 @@ static RegisterPrimOp primop_catAttrs({
.fun = prim_catAttrs,
});

static void prim_functionStrict(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
state.forceValue(*args[0], pos);
if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) {
v.mkNull();
return;
}

state.forceFunction(*args[0], pos, "while evaluating the argument passed to builtins.functionStrict");
const auto fun = args[0]->lambda.fun;

if (args[0]->isLambda() && fun->hasFormals()) {
v.mkBool(true);
} else {
v.mkNull();
}
}

static RegisterPrimOp primop_functionStrict({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be functionHasFormals or whatever. It is not immediately obvious to the casual user what a “strict function” is supposed to be (and it is a bit ambiguous as well, I'd say). A function with formals is immediately syntactically obvious, so there is no confusion what this builtin would check for.

Additionally, not all functions that are strict in their argument have formals, as the documentation also admits, e.g.:

foo: builtins.seq foo (/* body … */)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't care less about the syntax of a function, which is why I've named these primops after the semantics.

By calling it has formals we can't go back and add a syntax to make functions with formals lazy. What matters is that the function definition is trivially strict, so that's what the name reflects. Should it be functionTriviallyStrict?

If this function is too contentious, I might remove it because it turned out that this one wasn't necessary for the use case I'm trying to solve. I think it's good to have, but low impact, so I don't want to waste time on it.

not all functions that are strict in their argument have formals

This is for improving error messages in a few cases, and nothing more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the semantics are fundamentally not introspectable, so the naming is very deceptive.

we can't go back and add a syntax to make functions with formals lazy

It wouldn't really matter, the introspection would still do its job as advertised—there would be a breaking change in the semantics of the language, but that would be a problem regardless of the existence of that builtin.

.name = "__functionStrict",
.args = {"f"},
.doc = R"(
Return `true` if *f* is a function that is defined using strict function
syntax, which is to say, using an argument attribute set declaration such
as `{ a }:` or `{ ... }:`.

Proper strictness analysis is undecidable, so for all other functions the
return value is `null`. `false` is not returned.
)",
.fun = prim_functionStrict,
});

static void prim_functionOpen(EvalState & state, const PosIdx pos, Value * * args, Value & v)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open is new terminology that has not appeared so far, as far as I am aware. I'd stick to something with ellipsis which is also terminology used by builtins.toXML.

{
state.forceValue(*args[0], pos);
if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) {
v.mkNull();
return;
}

state.forceFunction(*args[0], pos, "while evaluating the argument passed to builtins.functionOpen");
const auto fun = args[0]->lambda.fun;

if (args[0]->isLambda()) {
v.mkNull();
}

if (fun->hasFormals()) {
v.mkBool(fun->formals->ellipsis);
} else {
v.mkNull();
}
}

static RegisterPrimOp primop_functionOpen({
.name = "__functionOpen",
.args = {"f"},
.doc = R"(
Return `true` if *f* is a function that is defined using the ellipsis syntax, such as `{ ... }:` or `{ foo, ... }:`.

Return `false` for functions defined with an attribute list but no ellipsis, such as `{ foo, bar }:`.

Return `null` for functions defined using plain lambdas, such as `x: ...`, as well as for built-in functions.
)",
.fun = prim_functionOpen,
});

static void prim_functionBindsAllAttrs(EvalState & state, const PosIdx pos, Value * * args, Value & v)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this needed for? I don't really see an use case for this right away.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in this PR to report a warning when a flake's outputs function is invoked. This PR adds an attribute, which causes somewhat of a problem when the function is of the form

args@{ self }:

The lack of ellipsis means that we'd have to remove the attribute for compatibility, while args@ exposes that workaround, which would be a source of confusion. Detecting this situation and reporting a warning means that we get a reasonable upgrade story for this feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just check for an ellipsis (functionOpen)? Whether the user also binds the whole attribute set, is not really interesting for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it has ellipsis, but no binding for the whole attrset, there's no conflict and the backcompat logic can comply with the old interface and it doesn't have to warn about anything.

I don't want it to warn about something that isn't a problem, because too many warnings lead people to ignore them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but why not just warn if it lacks an ellipsis. The full attribute set binding should be irrelevant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The full attribute set binding should be irrelevant.

The ambiguity of #8908 (comment) does not occur if the full attrset binding is missing. So to keep warnings to a minimum, I want to know whether the full attrset binding exists or not.

I can agree that it should but unfortunately it would require me to warn in cases where it doesn't matter.

{
state.forceValue(*args[0], pos);
if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) {
v.mkNull();
return;
}

state.forceFunction(*args[0], pos, "while evaluating the argument passed to builtins.functionBindsAllAttrs");
const auto fun = args[0]->lambda.fun;

if (!args[0]->isLambda()) {
v.mkNull();
return;
}

if (fun->hasFormals()) {
v.mkBool(fun->arg != Symbol {});
} else {
v.mkNull();
}
}

static RegisterPrimOp primop_functionBindsAllAttrs({
.name = "__functionBindsAllAttrs",
.args = {"f"},
.doc = R"(
If the function is not defined with an argument list, return `null`.

Return `true` if *f* is a function that is defined using the `@` syntax, which binds an identifier to the original attribute set.

Return `false` for a function that does not use the `@` syntax.
)",
.fun = prim_functionBindsAllAttrs,
});

static void prim_functionArgs(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
state.forceValue(*args[0], pos);
Expand Down
13 changes: 13 additions & 0 deletions src/nix/flake.md
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,19 @@ way. Most flakes provide their functionality through Nixpkgs overlays
or NixOS modules, which are composed into the top-level flake's
`nixpkgs` input; so their own `nixpkgs` input is usually irrelevant.

# Other arguments

As discussed, `self` is an example of an `outputs` argument that is not derived from `inputs`.

The following are the extra attributes that are always passed to `outputs`:

* `meta`: An attribute set relating to the flake. It contains the attributes:
* `sourceInfo`: Equal to `self.sourceInfo`, but accessible even when `self` is broken due to an evaluation error.
* `subdir`: The subdirectory within `sourceInfo`, where `flake.nix` resides. This is `""` when `flake.nix` is at `${sourceInfo.outPath}/flake.nix`.
* `extraAttributes`: Attributes that are added to the flake outputs by `getFlake` or the internal `inputs` logic.
* `extraArguments`: A reference to the attributes documented here - essentially the `outputs` arguments that aren't `inputs`.
* `self`: This flake, including the attributes returned by `outputs` and `meta.extraAttributes`. It has the same shape as the result of `getFlake` or the inputs that are flakes.

# Lock files

Inputs specified in `flake.nix` are typically "unlocked" in the sense
Expand Down
97 changes: 96 additions & 1 deletion tests/flakes/check.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
source common.sh

flakeDir=$TEST_ROOT/flake3
mkdir -p $flakeDir
depDir=$TEST_ROOT/flakedep
depDirB=$TEST_ROOT/flakedep2
mkdir -p $flakeDir $depDir $depDirB

cat > $depDir/flake.nix <<EOF
{
outputs = { ... }: {
};
}
EOF

cat > $flakeDir/flake.nix <<EOF
{
Expand Down Expand Up @@ -89,3 +98,89 @@ nix flake check $flakeDir
checkRes=$(nix flake check --all-systems --keep-going $flakeDir 2>&1 && fail "nix flake check --all-systems should have failed" || true)
echo "$checkRes" | grepQuiet "packages.system-1.default"
echo "$checkRes" | grepQuiet "packages.system-2.default"

cat > $flakeDir/flake.nix <<EOF
{
inputs = {
dep.url = "$depDir";
};
outputs = { self, dep }: {
};
}
EOF

nix flake check $flakeDir

cat > $flakeDir/flake.nix <<EOF
{
inputs = {
self.url = "$depDir";
};
outputs = { self }: {
};
}
EOF

expectStderr 1 nix flake check $flakeDir | grep -F "flake input name 'self' is reserved"


cat > $flakeDir/flake.nix <<EOF
{
inputs = {
meta.url = "$depDir";
};
outputs = args@{ self, ... }: {
};
}
EOF

expectStderr 1 nix flake check $flakeDir | grep -F "flake input name 'meta' is reserved"

echo cat \> $depDirB/flake.nix
cat > $depDirB/flake.nix <<EOF
{
inputs = {
};
outputs = args@{ self }:
# args won't have meta, as required by function semantics, but it is rather unfortunate,
# so Nix should warn about it, when this is the root flake.
assert !args?meta; # implied by function semantics
{
packages.$system = { };
};
}
EOF

nix flake check $depDirB 2>&1 | grep -F "Please add ellipsis"

cat $depDirB/flake.nix

# However it should not warn when the flake is used as a dependency, because in that case the user may not own the flake and can't change it. If they do own it, they only need to know about it when working on the flake itself.
cat > $flakeDir/flake.nix <<EOF
{
inputs = {
dep.url = "$depDirB";
};
outputs = { self, dep }:
builtins.seq dep.packages
{
};
}
EOF

nix flake check $flakeDir 2>&1 | grepQuietInverse "Please add ellipsis"


cat > $flakeDir/flake.nix <<EOF
{
inputs = {
};
outputs = args:
assert args?meta;
assert args?self;
{
};
}
EOF

nix flake check $flakeDir 2>&1 | grepQuietInverse "Please add ellipsis"
Loading