Document .override (old: ...)

[nh2]

.override for packages has 2 forms: .override {} and .override (old: {}).

These aren't properly documented in the nixpkgs manual, especially not the latter.

I think it's extra important to document what old contains in this case, because it has supriring behaviour:

It lacks e.g. defaulted boolean (enableX ? true) flags.

For example for curl, https://github.com/NixOS/nixpkgs/blob/f4af5f389925364f05e78445441dd17ef1ba567a/pkgs/tools/networking/curl/default.nix#L5

When you use curl.override (old: builtins.trace old.zlibSupport ...) it complains that zlibSupport does not exist on old. However old.zlib exists.

I suspect that what's happening is that only those things are in old that were passed from outside, and that ? defaultvalue arguments are not in there.

This badly surprised me here and probably also @oxij here.

We should document how it works, and recommend how to handle such cases (e.g. using old.enableX or true if the default value in the package is ? true), or "fix" the fact that default values are not present.

[oxij]

Alternatively, we can fix it to do the unsurprising thing. A quick hack that seems to work (a bit ugly, but a noop change that keeps all of nixpkgs evaluating)

diff --git a/lib/customisation.nix b/lib/customisation.nix
index aaaaaaaaaaa..bbbbbbbbbbb 100644
--- a/lib/customisation.nix
+++ b/lib/customisation.nix
@@ -67,7 +67,8 @@ rec {
   makeOverridable = f: origArgs:
     let
       ff = f origArgs;
-      overrideWith = newArgs: origArgs // (if lib.isFunction newArgs then newArgs origArgs else newArgs);
+      # builtins.isFunction is important here, one can't apply lib.functionArgs to things with `__functor`
+      overrideWith = newArgs: origArgs // (if lib.isFunction newArgs then newArgs (if builtins.isFunction f then lib.functionArgs f // origArgs else origArgs) else newArgs);
     in
       if builtins.isAttrs ff then (ff // {
         override = newArgs: makeOverridable f (overrideWith newArgs);

with the above the desired

+      gssSupport = !stdenv.isDarwin && old.gssSupport;

from the other PR evaluates as expected.

On to the issue of handling __functors I tried adding

+  extractFunction = f:
+    if builtins.isFunction f then f
+    else if f ? __functor then extractFunction (f.__functor f)
+    else throw "not a function";

near isFunction in lib/trivial.nix and doing this instead of the above

-      overrideWith = newArgs: origArgs // (if lib.isFunction newArgs then newArgs (if builtins.isFunction f then lib.functionArgs f // origArgs else origArgs) else newArgs);
+      overrideWith = newArgs: origArgs // (if lib.isFunction newArgs then newArgs (if lib.isFunction f then lib.functionArgs (lib.extractFunction f) // origArgs else origArgs) else newArgs);

but then something in the Haskell infra stops evaluating for mysterious reasons...

[nh2]

Alternatively, we can fix it to do the unsurprising thing

:+1: @oxij I'm not familiar enough with this to judge your fix, do you want to post it as a PR to get proper scrutiny, and perhaps some contributed fixes for the Haskell infra?

[stale[bot]]

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
2. Ask on the NixOS Discourse.
3. Ask on the #nixos channel on irc.freenode.net.

[nh2]

still important to me

[stale[bot]]

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

[nh2]

still important to me

[stale[bot]]

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