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

don't re-instantiate nixpkgs in flake and module #67

Merged
merged 4 commits into from
Feb 5, 2025

Conversation

getchoo
Copy link
Contributor

@getchoo getchoo commented Feb 4, 2025

This is expensive on evaluation, but can be easily avoided here as we aren't using unfree packages or special configurations

See https://zimbatm.com/notes/1000-instances-of-nixpkgs

The only systems this will affect are those not listed here, as the overlay will no longer be automatically applied and an assertion will be triggered. They will need to add nixpkgs.overlays = [ comin.overlays.default ] to their configuration; users on other systems will have overlayed packages used by default as well

@nlewo
Copy link
Owner

nlewo commented Feb 4, 2025

Hello,
The CI fails because you added an option without running nix run .#generate-module-options to regenerate the markdown file describing the options.

So, you need to

  • run nix run .#generate-module-options
  • commit markdown file modifications

This will allow the overlay to still easily be used by those on systems
not directly supported by the Flake, or those who just prefer to use
them over pure attributes
and stop using the now removed `lib.mdDoc`

• Updated input 'nixpkgs':
    'path:/nix/store/f3rv70nrmkfya581f1j2z9a6yx0b83np-source?lastModified=1717281328&narHash=sha256-evZPzpf59oNcDUXxh2GHcxHkTEG4fjae2ytWP85jXRo%3D&rev=b3b2b28c1daa04fe2ae47c21bb76fd226eac4ca1' (2024-06-01)
  → 'github:NixOS/nixpkgs/fecfeb86328381268e29e998ddd3ebc70bbd7f7c?narHash=sha256-rvyfF49e/k6vkrRTV4ILrWd92W%2BnmBDfRYZgctOyolQ%3D' (2025-02-03)
@getchoo
Copy link
Contributor Author

getchoo commented Feb 4, 2025

Done!

I've updated the Nixpkgs revision as well, since it seems it was still on the now EOL 24.05 branch. nix flake check worked on x86_64-linux

@nlewo
Copy link
Owner

nlewo commented Feb 5, 2025

Thank you!

@nlewo nlewo merged commit 563b287 into nlewo:main Feb 5, 2025
2 checks passed
@getchoo getchoo deleted the dont-instantiate branch February 5, 2025 21:47
@nlewo
Copy link
Owner

nlewo commented Feb 6, 2025

hm, looks like i merged it a bit too quickly: it no longer evaluates on my machines:

       error: attribute 'packages' missing
       at /nix/store/4ipj96bhdy4wh6c4fxqik4kd05knkm1m-source/nix/module.nix:26:71:
           25|       # If the package is null and our `system` isn't supported by the Flake, it's probably safe to show this error message
           26|       { assertion = package == null -> lib.elem system (lib.attrNames self.packages); message = "comin: ${system} is not supported by the Flake."; }
             |                                                                       ^
           27|     ];

No time ATM to fix this.

@@ -1,4 +1,4 @@
overlay: { config, pkgs, lib, ... }:
self: { config, pkgs, lib, ... }:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this line is the culprit

self: should be { self }:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants