Make overridePackages extend rather than replace existing overrides

[Ericson2314]

It makes the function more useful as one can always re-import nixpkgs to start afresh.

CC @cstrahan (your new abstraction!) @nbp (held off on doing this because breaking change IIRC) @Mathnerd314 (wanted this)

[mention-bot]

@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nbp, @cstrahan and @edolstra to be potential reviewers.

[aneeshusa]

I never realized overridePackages replaced any previous overrides, so +1 from me for more intuitive semantics.

[aneeshusa]

Not sure about space usage/complexity, but using foldl or foldl' would let this this from top to bottom, which seems easier to read.

[Ericson2314]

Good point. There could be some constant factor cost in the extra flip needed for extends, but nothing to worry about without benchmarks.

[cstrahan]

Hah, I recently thought about doing just this, as I figured someone might need a different name, perhaps for backwards compatibility somewhere or avoiding name clashes.

[cstrahan]

I've wanted this ever since I discovered @roconnor's dynamic-binding/object system[1][2], as seen in the wonderful haskellPackages infrastructure (from which I extracted the makeExtensible function).

The current solution isn't ideal, and the behavior clearly isn't what we'd like (as is evident in #18483).

# The way we currently override packages:
nixpkgs.config.packageOverrides = super: let self = super.pkgs; in
 rec {
   # ... overrides ...
 };

I believe this approach would be an improvement from both a UI and semantic point of view.

👍

[1] http://r6.ca/blog/20140422T142911Z.html
[2] http://lists.science.uu.nl/pipermail/nix-dev/2014-May/013272.html

[Ericson2314]

Glad you all like it! What further review/testing/etc is needed before somebody feels comfortable merging this? 😄

[cstrahan]

The only hesitance I have is that (if I'm reading this right) it'll break existing uses of packageOverrides (as, up to this point, that function expects as its argument a unary function, as opposed to a binary function). I'm not sure what the best way is to proceed with this just yet, though I think the transition should be made somehow.

Would it be possible to, perhaps with more verbose code, provide a backwards compatible interface, and warn the user to supply the (self: super: { ... }) style argument? We could consider the old style deprecated and remove support for it in the future.

Alternatively, maybe the error that people would see with the present code would be sufficient to clue them in. I feel like I might be over stepping my privileges in making that judgement call, though.

[cstrahan]

One option might be: expose the "extender" function as extend instead of packageOverrides, and provide a warning that packageOverrides is deprecated and extend should be used instead, that way we can preserve the old interface for a while, and we can start to standardize on extend for extensions (as we have standardized on override being the function that overrides package arguments).

[Ericson2314]

See https://github.com/NixOS/nixpkgs/pull/19496/files#diff-017a38a631b06991d33857f7681874b3L71 config's packageOverrides is one-arg but overridePackages is already two-arg.

The only compatibility hazard is relying on overridePackages to purge any prior config / previous calls to overridePackages, which is much more niche.

[Ericson2314]

@cstrahan I agree it would be nice to standardize on extend, but I fear its less obvious without the packages that extend isn't itself some package. I think the real long term solution is not sticking with the current name, but separating actual packages from utility functions in nixpkgs's top-level, in which case we can and should use extend for consistency.

[cstrahan]

@Ericson2314 Ah, I didn't catch the distinction between overridePackages and packageOverrides -- kept reading them as if they were one in the same. In fact, I didn't know about overridePackages, which was apparently introduced here: 7f5f9072ad.

In that case, I think this is probably fine as is. I'll put it through its paces tonight and merge.

[Ericson2314]

@cstrahan yeah, typing that last comment I realized just how confusing those identifiers are. All the more reason for standardizing on extend long-term :).

[cstrahan]

Hey, sorry -- oversubscribed myself. I'll give this a quick sanity check right now and merge. Pinkie-promise.

[FRidh]

Since this commit I get

error: attribute ‘overridePackages’ missing, at /tmp/example_inkscape.nix:4:13

when using the following expression

with import <nixpkgs> { };

let
  newpkgs = pkgs.overridePackages ( self: super: {
  } );
in newpkgs.inkscape

and a .nixpkgs/config.nix that doesn't contain any packageOverrides.

[FRidh]

Checking the manual I saw the following has to be used.

let
  x= import <nixpkgs> { };
  newpkgs = x.overridePackages ( self: super: {
  } );
in newpkgs.inkscape

What I wrote in my previous comment used to work but not anymore. Maybe its worth sending out an e-mail.

[Ericson2314]

@FRidh Sorry this was not an intended breaking change. I'll look into it.