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.

Sign up for GitHub

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

Jump to bottom

Add pkgs.callOverlay function #180557

Closed
wants to merge 4 commits into from
Closed

Add pkgs.callOverlay function #180557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10e0ac7
Add pkgs.callOverlay and pkgs.callOverlays funcitons
gytis-ivaskevicius Jul 7, 2022
879591c
Improve pkgs.callOverlay performance
gytis-ivaskevicius Jul 8, 2022
9e7c795
Fix 'pkgs.callOverlay' docs
gytis-ivaskevicius Jul 8, 2022
8babe0e
Fix 'pkgs.callOverlay' docs
gytis-ivaskevicius Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions pkgs/pkgs-lib/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,35 @@
formats = import ./formats.nix {
inherit lib pkgs;
};

/*
Return attribute set of packages from overlay

Type: callOverlay :: (pkgs -> pkgs -> attrs) -> attrs

Example:
callOverlay (final: prev: { a = 42; })
=> { a = 42 }
*/
callOverlay = overlay:
let
newPkgs = overlay newPkgs pkgs // {
pkgs = newPkgs;
callPackage = pkgs.lib.callPackageWith newPkgs;
Comment on lines +23 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the other package sets like pkgsStatic or the cross ones?
As a general rule, overriding can not be added externally like you're attempting here. It needs to be integrated in the original structure because of all the possible recursive definitions. In other words, this needs changes in top-level/stage.nix in order to work well.
We haven't even made pkg.override work entirely correctly with cross splicing and overrideAttrs yet, and this change has a lot in common with a new "pkg.override".
For this functionality to not be another series of bugs like we've had with overriding, we need better support for sub-packagesets in stage.nix.
One idea is to make its nixpkgsFun overridable, although that might re-evaluate too much. Another is to put all the extra package sets in their own overlay, so that not just pkgs but all the package sets are easily modifiable.

Here's the general idea. It probably contains some mistakes, but it conveys the idea better than English.

let
  # adaptPackageSet :: attrs -> attrs -> attrs
  # adaptPackageSet = pkgs: definition: ...
  #     Internal to stage.nix.
  #     A function that defines how the package set is extended.
  #     - `pkgs`         an already evaluated (or at least shared) instance of the Nixpkgs fixpoint.
  #     - `definition`   a fresh redefinition of the package set according to the rules of e.g. `pkgsLLVM`, `pkgsCross.<x>`, etc.
  otherPackageSets = adaptPackageSet: self: super: {
    pkgsLLVM = adaptPackageSet super.pkgsLLVM (
      # the existing definition
      nixpkgsFun { /* ... */ }
    );
    # pkgsCross etc
    # pkgs?
  };
  callOverlayOverlay = otherPackageSets (super: definition: callOverlay' super);
  callOverlay' = overlay: callOverlay (/* flip ? */ extends callOverlayOverlay overlay); 
  callOverlay = your definition;
in

# toFix = ... [
# ...
  (otherPackageSets (super: definition: definition))
# ...
  (_: _: { callOverlay = callOverlay'; }) # though not sure if this should be separate (and add a somewhat expensive // on pkgs)
# ...

extend should probably be defined similarly, but ignore the super argument of wrapPackageSet.

I think there's also a possibility of allowing actual overlays to extend the otherPackageSets function, but I'm not sure and that's over-engineering it for the first design step, but it would fix another problem. First priority would be to make something like the above work.

Copy link
Member

@roberth roberth Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB this still doesn't cover interactions between extend and callOverlay.

Also how about affixOverlay or attachOverlay? They're not properly merged into self like an overlay normally would, so I think it would be good to add an appropriate verb. extend already doesn't really suggest merging, so I tried to think of something that is even less "merge"-like.

EDIT: add "into self"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related? https://github.com/nixpkgs-architecture/issues/issues/21

};
in
overlay newPkgs pkgs;

/*
Return attribute set of packages from overlays

Type: callOverlays :: [pkgs -> pkgs -> attrs] -> attrs

Example:
callOverlays [(final: prev: { a = 42; }) (final: prev: { b = 123; })]
=> { a = 42; b = 123; }
*/
callOverlays = overlays:
pkgs.callOverlay (lib.composeManyExtensions overlays);
}

2 changes: 1 addition & 1 deletion pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,7 @@ with pkgs;
writers = callPackage ../build-support/writers {};

# lib functions depending on pkgs
inherit (import ../pkgs-lib { inherit lib pkgs; }) formats;
inherit (import ../pkgs-lib { inherit lib pkgs; }) formats callOverlay callOverlays;

testers = callPackage ../build-support/testers {};

Expand Down