Skip to content

Draft for nixpkgs flake module#126

Closed
ghost wants to merge 6 commits intohercules-ci:mainfrom
Criome:overlayedFlakes
Closed

Draft for nixpkgs flake module#126
ghost wants to merge 6 commits intohercules-ci:mainfrom
Criome:overlayedFlakes

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Mar 21, 2023

Uses NixOS/nixpkgs#222155
This is shared currently to document and test.
Done from a live coding on youtube

Sajban-style-preview commit message:

(added (naiveImplementationFor (nixpkgs flakeModule)))

Copy link
Copy Markdown
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Hi Li, how's it going?

I think it should be possible to turn most if not all changes here into additions to the nixpkgs.flakeModules.default module. Perhaps we could add a deprecation message to the old default, suggesting that they import the nixpkgs module themselves. The idea is to reinforce the modular architecture and give users more choice.
What do you think?

Comment thread modules/nixpkgs.nix
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think additions to this module are necessary.

Comment thread modules/nixpkgs.nix
);
};
};
imports = [ inputs.nixpkgs.flakeModules.default ];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We shouldn't make assumptions about the inputs, especially not in the core of flake parts.

Comment thread modules/nixpkgs.nix
imports = [ inputs.nixpkgs.flakeModules.default ];
_file = ./nixpkgs.nix;
config = {
_module.args.pkgs = lib.mkForce finalPkgs;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would give this responsibility to nixpkgs.flakeModules.default, and leave the original default in place for the time being.

Comment thread modules/nixpkgs.nix

in
{
imports = [ inputs.nixpkgs.flakeModules.default ];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a general rule, modules are imported at the top level only. This makes importing more consistent, not having to wonder where to import it.
Maybe this was added by mistake? Maybe it doesn't even raise an error, as the laziness can mask problems sometimes; a blessing and a curse.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I thought this would be needed to expose the nixpkgs option to the perSystem evaluation scope. Will experiment with removing it now.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see you've avoided the perSystem option there. I think users would expect the Nixpkgs to be in perSystem.

See my comment above. Am I wrong? I'm using nixpkgs option inside the perSystem scope here

@roberth
Copy link
Copy Markdown
Member

roberth commented Apr 1, 2023

I think it should be possible to turn most if not all changes here into additions to the nixpkgs.flakeModules.default module.

I see you've avoided the perSystem option there. I think users would expect the Nixpkgs to be in perSystem.
Maybe you've avoided it because that might not be allowed architecturally, but I don't think this should be a concern. It's very common for modules to make assumptions about the environment they're in, and for a flake-parts module that environment includes perSystem.
Another reason I suppose is that it would be nice to have a very pure module that's all about calling the Nixpkgs function and very little else. I think such a module would be great to have, and it would be probably be something that you could import into the flakeModules.default module, inside perSystem.
This creates a bit different division of labor, where it's also possible to for example have an overlay depend on an option that's in perSystem.

Here's what I think that might look like.

A general nixpkgs invocation module, not specific to any context (where flake-parts is such a context):

pkgs/top-level/modules/pkgs.nix

{ config, lib, ... }: {
  options.nixpkgs = {
    overlays = ...;
    system = ...; # or even hostPlatform + buildPlatform options
    config = ...;
  }
  config._module.args = lib.mkDefault (import .. { ... });
}

The module for nixpkgs.flakeModules.default:

pkgs/top-level/modules/flakeModules.nix

{
   options.perSystem = mkPerSystemOption ({ config, system, ... }: {
     imports = [ ./pkgs.nix ];
     nixpkgs.system = system;
   };
}

@LiGoldragon
Copy link
Copy Markdown

This creates a bit different division of labor, where it's also possible to for example have an overlay depend on an option that's in perSystem.

Beat you to it: clojix does that

@LiGoldragon
Copy link
Copy Markdown

pkgs/top-level/modules/flakeModules.nix

{
   options.perSystem = mkPerSystemOption ({ config, system, ... }: {
     imports = [ ./pkgs.nix ];
     nixpkgs.system = system;
   };
}

That would make nixpkgs depent on flake-parts for mkPerSystemOption, right?

After reconsidering this from a few steps back, I realized none of this really needs to go in either flake-parts or nixpkgs. In fact, I'm rather of the opinion that nothing should go in nixpkgs anymore, since flakes is partly about splitting giant mono-repos. I have a rather funny story related to that; a few years ago when I began to play with flakes, I was going to make a flake for every major function, only to realize (after creating like 50 repos), that functions can't use caching very well, if at all.

So I might just create a repo and call it 'nixpkgs-flake' or something. Stay tuned.

And thanks for all the help.

Off-topic: I would love to know your birth time and location, I would give you a quick astrology reading one of these days. Get the exact birth time if possible, to the minute. Just looking at your profile pic and from reading your comments and code, my first bet is Virgo. Did I get that right? Lol! Cheers! ;)

@roberth
Copy link
Copy Markdown
Member

roberth commented Apr 12, 2023

After reconsidering this from a few steps back, I realized none of this really needs to go in either flake-parts or nixpkgs.

That is true; nothing needs to go anywhere. Flake-parts is designed to be flexible like that, because of the purposely limited scope of its core.

So I might just create a repo and call it 'nixpkgs-flake' or something. Stay tuned.

Sounds good! You can send a PR to the sister repo https://github.com/hercules-ci/flake.parts-website if you'd like it to be listed on flake.parts.

And thanks for all the help.

You're welcome.

Do you imply that you're discontinuing the nixpkgs PR as well?
Would you mind if I took the overlay type logic and added it to Nixpkgs lib with some tests?

I'd still like to have a fairly simple module in Nixpkgs that does the right thing for most people; just a little bit more advanced than the placeholder that I want to remove from flake-parts, reinforcing the idea that it certainly doesn't have to go into flake-parts, by not even providing any built-in solution in the near future. I'll probably add it myself soon, because I keep getting requests about overlays and such.

@roberth
Copy link
Copy Markdown
Member

roberth commented Apr 12, 2023

Off-topic: I would love to know

The universe is far more mysterious than that... 🌟

@LiGoldragon
Copy link
Copy Markdown

I'll close as unattended, will make a new one if I take another shot, as it would be different now. Related to a module in nixpkgs, which should share some code as you said. Lib sounds right, since that is nixpkgs's lib, and overlays are one of its API types. I want to move away from overlays for sure though. I think a github-like namespace would do a lot, especially with pre-typed subnames: hub.LiGoldragon.website is a website type, allowing for safer jobs for shared servers, etc. I have a repo called hob where I'll be doing a complete rewrite in clojure soon. Stay tuned.

@ghost ghost closed this Apr 27, 2023
@LiGoldragon
Copy link
Copy Markdown

LiGoldragon commented Apr 28, 2023

Off-topic: I would love to know

The universe is far more mysterious than that... star2

I'm trying to see if that means Virgo or not ...
More mysterious than anyone can imagine in his wildest fantasy.
But I can still see people's Sun. If I miss it there will be a powerful ascendant or first house hiding it. I cannot ignore the results of my research and experimentation; astrology works better than a brand new Mercedez engine. No science can rival it. And mor importantly, it does not make the world any less mysterious; quite the contrary. The currently dominating cosmology is being exposed; it knows less than nothing. We know the Sun is some kind of portal and Earth is stationary from all kinds of experiments, but mainstream is very afraid of that topic. So I have more questions than anyone who doesnt want to look at astrology with a modicum of respect and curiosity. Read Arthur Young's work on astrology if you want to see it from the man who invented modern helicopter designs and found an astrology book one day and got curious... You could be blown away. "Geometry of Meaning" and "Science, Astrology, ${longerSubtitle}" (A short essay) are the centerpieces. Watch Eric Dollard, an emminent physicist explain why he knows the sun is some kind of portal, and enjoy Eric Dubay's "200 proofs" or Dave Murphy debating the best Physics and Astro-physics people who want to defend the current cosmology in various videos, or in his famous late-night-show interview he gave with Milenko. You can laugh too if it helps, but I strongly suggest you eventually take a look.

Much thanks and gratitude!

This pull request was closed.
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