about summary refs log tree commit diff
diff options
context:
space:
mode:
authorsterni <sternenseemann@systemli.org>2021-01-31T18·36+0100
committersterni <sternenseemann@systemli.org>2021-02-02T10·52+0000
commit87091539739e8ee30217e6483a8b13b27f5bc31e (patch)
tree2ed15bbf5459783449b82b3bc3f941dca9447dcc
parentc6b243a7a5d7ca9af6764844e820f3b3911b1019 (diff)
fix(tvix/libexpr): backport fix for functionArgs failing on primops r/2178
Nix internally differentiates between lambdas and primops, but their
type in the nix expression language is the same (lambda). The
implementation of builtins.functionArgs only checks if the given
expression is of type tLambda and fails if the type is tPrimop or
tPrimopApp which are also functions. This most notably breaks
lib.generators.toPretty when called on a builtin making for example
yants fail if a primop is typechecked and an error message is
generated.

This fix generates an empty set for primops like for plain lambdas
and is based upstream commit b2748c6e99239ff6803ba0da76c362790c8be192.

Additionally we add to two tests:

* eval-okay-functionargs now includes a few test cases checking that
  builtins.functionArgs always returns an empty set for builtins and
  also works as expected for normal functions.
* eval-okay-types now also checks if builtins are functions.

Future work would be to make builtins.functionArgs work as users would
expect for builtins like builtins.fetchurl, builtins.fetchGit etc. which
take a set as an argument. These currently don't register as formal
arguments, but it would be an usability improvement at least if they
did.

See also https://github.com/NixOS/nix/pull/3626#issuecomment-698546704

Change-Id: I2bf4cb80d44a4b72ade13d3e0dbd7dfb1d049f32
Reviewed-on: https://cl.tvl.fyi/c/depot/+/2477
Tested-by: BuildkiteCI
Reviewed-by: Profpatsch <mail@profpatsch.de>
Reviewed-by: glittershark <grfn@gws.fyi>
-rw-r--r--third_party/nix/src/libexpr/primops.cc5
-rw-r--r--third_party/nix/src/tests/lang/eval-okay-functionargs.exp2
-rw-r--r--third_party/nix/src/tests/lang/eval-okay-functionargs.nix11
-rw-r--r--third_party/nix/src/tests/lang/eval-okay-types.exp2
-rw-r--r--third_party/nix/src/tests/lang/eval-okay-types.nix1
5 files changed, 18 insertions, 3 deletions
diff --git a/third_party/nix/src/libexpr/primops.cc b/third_party/nix/src/libexpr/primops.cc
index 49c43556fe..710d214ce7 100644
--- a/third_party/nix/src/libexpr/primops.cc
+++ b/third_party/nix/src/libexpr/primops.cc
@@ -1369,6 +1369,11 @@ static void prim_catAttrs(EvalState& state, const Pos& pos, Value** args,
 static void prim_functionArgs(EvalState& state, const Pos& pos, Value** args,
                               Value& v) {
   state.forceValue(*args[0]);
+  if (args[0]->type == tPrimOpApp || args[0]->type == tPrimOp) {
+    // TODO(sterni): return set of formal arguments for fetch* primops
+    state.mkAttrs(v, 0);
+    return;
+  }
   if (args[0]->type != tLambda) {
     throw TypeError(format("'functionArgs' requires a function, at %1%") % pos);
   }
diff --git a/third_party/nix/src/tests/lang/eval-okay-functionargs.exp b/third_party/nix/src/tests/lang/eval-okay-functionargs.exp
index c1c9f8ffaf..f8ddea8e0b 100644
--- a/third_party/nix/src/tests/lang/eval-okay-functionargs.exp
+++ b/third_party/nix/src/tests/lang/eval-okay-functionargs.exp
@@ -1 +1 @@
-[ "stdenv" "fetchurl" "aterm-stdenv" "aterm-stdenv2" "libX11" "libXv" "mplayer-stdenv2.libXv-libX11" "mplayer-stdenv2.libXv-libX11_2" "nix-stdenv-aterm-stdenv" "nix-stdenv2-aterm2-stdenv2" ]
+[ { aterm = false; fetchurl = false; stdenv = false; } { color = false; name = true; } { } { } { } "stdenv" "fetchurl" "aterm-stdenv" "aterm-stdenv2" "libX11" "libXv" "mplayer-stdenv2.libXv-libX11" "mplayer-stdenv2.libXv-libX11_2" "nix-stdenv-aterm-stdenv" "nix-stdenv2-aterm2-stdenv2" ]
diff --git a/third_party/nix/src/tests/lang/eval-okay-functionargs.nix b/third_party/nix/src/tests/lang/eval-okay-functionargs.nix
index 68dca62ee1..205bbf307a 100644
--- a/third_party/nix/src/tests/lang/eval-okay-functionargs.nix
+++ b/third_party/nix/src/tests/lang/eval-okay-functionargs.nix
@@ -6,6 +6,14 @@ let
   atermFun = { stdenv, fetchurl }: { name = "aterm-${stdenv.name}"; };
   aterm2Fun = { stdenv, fetchurl }: { name = "aterm2-${stdenv.name}"; };
   nixFun = { stdenv, fetchurl, aterm }: { name = "nix-${stdenv.name}-${aterm.name}"; };
+
+  trivialFunctionArgsUsage = [
+    (builtins.functionArgs nixFun)
+    (builtins.functionArgs ({ name ? "Karl", color }: "${name} is ${color}"))
+    (builtins.functionArgs (x: y: x + y))
+    (builtins.functionArgs builtins.map)
+    (builtins.functionArgs builtins.fetchurl)
+  ];
   
   mplayerFun =
     { stdenv, fetchurl, enableX11 ? false, xorg ? null, enableFoo ? true, foo ? null  }:
@@ -67,7 +75,8 @@ let
   
 in
 
-  [ pkgs.stdenv.name
+  trivialFunctionArgsUsage ++ [
+    pkgs.stdenv.name
     pkgs.fetchurl.name
     pkgs.aterm.name
     pkgs2.aterm.name
diff --git a/third_party/nix/src/tests/lang/eval-okay-types.exp b/third_party/nix/src/tests/lang/eval-okay-types.exp
index 92a1532993..882c16dbfe 100644
--- a/third_party/nix/src/tests/lang/eval-okay-types.exp
+++ b/third_party/nix/src/tests/lang/eval-okay-types.exp
@@ -1 +1 @@
-[ true false true false true false true false true true true true true true true true true true true false true true true false "int" "bool" "string" "null" "set" "list" "lambda" "lambda" "lambda" "lambda" ]
+[ true false true true false true false true false true true true true true true true true true true true false true true true false "int" "bool" "string" "null" "set" "list" "lambda" "lambda" "lambda" "lambda" ]
diff --git a/third_party/nix/src/tests/lang/eval-okay-types.nix b/third_party/nix/src/tests/lang/eval-okay-types.nix
index 9b58be5d1d..cc51d8cb7a 100644
--- a/third_party/nix/src/tests/lang/eval-okay-types.nix
+++ b/third_party/nix/src/tests/lang/eval-okay-types.nix
@@ -3,6 +3,7 @@ with builtins;
 [ (isNull null)
   (isNull (x: x))
   (isFunction (x: x))
+  (isFunction functionArgs)
   (isFunction "fnord")
   (isString ("foo" + "bar"))
   (isString [ "x" ])