-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nixos/nextcloud: fix eval of tests #393063
base: master
Are you sure you want to change the base?
Conversation
Currently failing with error: cannot coerce a set to a string: { __functionArgs = «thunk»; __functor = «thunk»; } This comes from the `extraTests` option I added to the test modules to compose certain tests a little nicer. It's of type either (functionTo ...) str and it seems like the `functionTo` part now returns a functor (i.e. an attr-set that can be invoked as function). This is caught by `lib.isFunction`, but `builtins.isFunction` returns `false`. Hence, switching to the former fixes this.
@@ -1,6 +1,7 @@ | |||
{ system ? builtins.currentSystem | |||
, config ? { } | |||
, pkgs ? import ../../.. { inherit system config; } | |||
, lib ? pkgs.lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
, lib ? pkgs.lib |
lib
is already in scope through with pkgs.lib
?
This breakage seems unfortunate. From a language perspective: An attributeSet with I expect this regression where introduced by: #386208 Previously it returned a lambda function now returns a functor, both of which are arguably @roberth Should we revert #386208 ? Revert no:
Revert yes:
|
Currently failing with
This comes from the
extraTests
option I added to the test modules to compose certain tests a little nicer. It's of typeand it seems like the
functionTo
part now returns a functor (i.e. an attr-set that can be invoked as function). This is caught bylib.isFunction
, butbuiltins.isFunction
returnsfalse
.Hence, switching to the former fixes this.
ccing lib maintainers @hsjobeki @roberth @infinisil: while I don't care enough about this change on lib-side, I figured it's still reasonable to let you know of this fallout given the stability requirements that lib has (IIRC).
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.