Skip to content
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

Provide a way to bind the argument set *including* default values #1461

Closed
michaelpj opened this issue Jul 12, 2017 · 9 comments
Closed

Provide a way to bind the argument set *including* default values #1461

michaelpj opened this issue Jul 12, 2017 · 9 comments
Assignees
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc stale

Comments

@michaelpj
Copy link

I was somewhat surprised to find that this doesn't work:
(args@{a ? "a"}: ({a}: a) args) {}
error: anonymous function at (string):1:19 called without required argument ‘a’, at (string):1:18

I had expected that the @ binding would bind the argument set that was actually visible in the body of the function, which would include default values. But it looks like it binds the argument set that was actually passed.

Getting the set of all arguments as visible in the function seems pretty useful. There's a particularly obvious use case where you just want to call another function and pass on all of your arguments, which doesn't work currently.

I can see there are probably times when you want the current behaviour, but it would be nice to have a way to get the arguments including default values.

@shlevy shlevy added the backlog label Apr 1, 2018
jtojnar referenced this issue in NixOS/nixpkgs Dec 29, 2019
Bash takes an assignment of a string to an array variable:

local -a user_args
user_args="(foo bar)"

to mean appending the string to the array, not parsing the string into
an array as is the case when on the same line as the declaration:

local -a user_args="(foo bar)"

b063340 extracted the declaration before
the newly branched code block, causing string makeWrapperArgs being added
to the array verbatim.

Since local is function scoped, it does not matter if we move it inside
each of the branches so we fix it this way.
jtojnar added a commit to NixOS/nixpkgs that referenced this issue Dec 29, 2019
When `makeWrapperArgs` variable is not set, `declare -p makeWrapperArgs`
will return with 1 and print an error message to stderr.

I did not handle the non-existence case in b063340
because I thought `mk-python-derivation` will always define `makeWrapperArgs`
but `wrapProgram` can be called independently. And even with `mk-python-derivation`,
`makeWrappers` will not be set unless explicitly declared in the derivation
because of NixOS/nix#1461.

I was lead to believe that because the builds were succeeding and I confirmed
that the mechanism fails when the variable is not defined and `-o nounset` is enabled.
It appears that `wrapPython` setup hook is not running under `-o nounset`, though,
invaldating the assumption.

Now we are checking that the variable exists before checking its type, which
will get rid of the warning and also prevent future error when `-o nounset`
is enabled in the setup hook.

For more information, see the discussion at
a6bb2ed
FRidh pushed a commit to NixOS/nixpkgs that referenced this issue Dec 30, 2019
When `makeWrapperArgs` variable is not set, `declare -p makeWrapperArgs`
will return with 1 and print an error message to stderr.

I did not handle the non-existence case in b063340
because I thought `mk-python-derivation` will always define `makeWrapperArgs`
but `wrapProgram` can be called independently. And even with `mk-python-derivation`,
`makeWrappers` will not be set unless explicitly declared in the derivation
because of NixOS/nix#1461.

I was lead to believe that because the builds were succeeding and I confirmed
that the mechanism fails when the variable is not defined and `-o nounset` is enabled.
It appears that `wrapPython` setup hook is not running under `-o nounset`, though,
invaldating the assumption.

Now we are checking that the variable exists before checking its type, which
will get rid of the warning and also prevent future error when `-o nounset`
is enabled in the setup hook.

For more information, see the discussion at
a6bb2ed
FRidh pushed a commit to NixOS/nixpkgs that referenced this issue Dec 30, 2019
When `makeWrapperArgs` variable is not set, `declare -p makeWrapperArgs`
will return with 1 and print an error message to stderr.

I did not handle the non-existence case in b063340
because I thought `mk-python-derivation` will always define `makeWrapperArgs`
but `wrapProgram` can be called independently. And even with `mk-python-derivation`,
`makeWrappers` will not be set unless explicitly declared in the derivation
because of NixOS/nix#1461.

I was lead to believe that because the builds were succeeding and I confirmed
that the mechanism fails when the variable is not defined and `-o nounset` is enabled.
It appears that `wrapPython` setup hook is not running under `-o nounset`, though,
invaldating the assumption.

Now we are checking that the variable exists before checking its type, which
will get rid of the warning and also prevent future error when `-o nounset`
is enabled in the setup hook.

For more information, see the discussion at
a6bb2ed
@deliciouslytyped
Copy link

I really want this for some patterns...

@zimbatm
Copy link
Member

zimbatm commented Apr 15, 2020

related discussion: NixOS/rfcs#58 (comment)

@deliciouslytyped
Copy link

deliciouslytyped commented Apr 15, 2020

In fact, my use case would be something like

lib.makeOverridable ({ a ? 1, b ? ... , c ? ... }: { inherit a; })

which seems to me like a verbose way to get a decent pattern of overridable let expressions, which IMHO are sorely missed in the whole overridability story.
I just coded myself into a corner because I thought this would work.

For something like this, you have no way to override part of the initialCommand, because it disappears from view.

{ #...
    inspect-ast = _lib.makeOverridable (
        { viewer ? lib.viewer {}
        , initialCommand ? _lib.makeOverridable ( 
          { testFile ? lib.qt_aggregated_header
          , headers ? [ stdenv.glibc.dev lib.headers lib.llvmPackages.libcxx ]
          , args ? [ "-xc++" "-fsyntax-only" "-v" "-fPIC" "-Xclang" "-detailed-preprocessing-record" ]
          }:
            args ++ (builtins.map (v: " -I " + v) (_lib.makeSearchPath "/include" headers )) ++ [ testFile ] )
        , isTruthy ? v: ((v  != "") && (v != null) && (v != false) && (v != []))
        }:
          writeShellScriptBin "ast-viewer" ''
      echo ${viewer.passthru.bin} $@ ${ _lib.optionalString (isTruthy initialCommand) ("-- " + (_lib.concatStringsSep " " initialCommand)) }
      '');
}
{#...
    inspect-ast = (lib.inspect-ast {}).override (old: {
      initialCommand = old.initialCommand.override (old: old // { inherit headers; });
      });
}
attribute 'initialCommand' missing, at /path/default.nix:132:24

I think a builtin that returns the attrset lazy in its argument values as usual, but all its arguments available, would be sufficient.

Edit: It's been brought to my attention that functors might be a viable alternative to my approach.

@stale
Copy link

stale bot commented Feb 15, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Feb 15, 2021
@deliciouslytyped
Copy link

People continue to periodically run into this in various public fora including the IRC channels.

@stale stale bot removed the stale label Apr 16, 2021
@stale
Copy link

stale bot commented Oct 22, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Oct 22, 2021
@fricklerhandwerk fricklerhandwerk added feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc labels Sep 13, 2022
rvem pushed a commit to serokell/nixpkgs that referenced this issue Nov 8, 2022
When `makeWrapperArgs` variable is not set, `declare -p makeWrapperArgs`
will return with 1 and print an error message to stderr.

I did not handle the non-existence case in b063340
because I thought `mk-python-derivation` will always define `makeWrapperArgs`
but `wrapProgram` can be called independently. And even with `mk-python-derivation`,
`makeWrappers` will not be set unless explicitly declared in the derivation
because of NixOS/nix#1461.

I was lead to believe that because the builds were succeeding and I confirmed
that the mechanism fails when the variable is not defined and `-o nounset` is enabled.
It appears that `wrapPython` setup hook is not running under `-o nounset`, though,
invaldating the assumption.

Now we are checking that the variable exists before checking its type, which
will get rid of the warning and also prevent future error when `-o nounset`
is enabled in the setup hook.

For more information, see the discussion at
NixOS@a6bb2ed
@paulyoung
Copy link

I just ran into this and was quite confused.

@stale stale bot removed the stale label Dec 14, 2022
@stale stale bot added the stale label Jun 18, 2023
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Jun 23, 2023

Triaged in the Nix team meeting:

Closing, and deferring to RFC 137, which if accepted will allow for controlled breaking changes to the language. Making this a pure addition is likely to be very involved.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-06-22-nix-team-meeting-minutes-65/29643/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc stale
Projects
None yet
Development

No branches or pull requests

9 participants