pkgs/top-level/stage.nix: move most nixpkgs sets to variants by RossComputerGuy · Pull Request #400351 · NixOS/nixpkgs

RossComputerGuy · 2025-04-20T15:27:18Z

Things done

Add separate nixpkgs variants which can be disabled or enabled as needed.

Also add mkOptionalOverlay, this allows us to throw an error when an attribute from an overlay isn't available unless enabled.

Add a 👍 reaction to pull requests you find important.

doc/release-notes/rl-2505.section.md

pkgs/top-level/config.nix

pkgs/top-level/variants.nix

wolfgangwalther · 2025-04-24T12:40:49Z

I like the basic idea, which is to make sure that no references to those package sets can sneak into other nixpkgs code. In the best case, all the remaining package sets like pkgsStatic etc. would also be variants. But I guess they are currently referenced in nixpkgs, so that won't work. And also, the breaking change would be much bigger.

But that still doesn't convince me, that adding those variants like pkgsLLVMLibc or pkgsBolt etc. is a good idea. We will quickly end up with a pkgsXXX variant of every available config option.

I think we are conflating two issues here:

Top-level package sets make it possible to create a combination of various config options. pkgsExtraHardening or pkgsStatic, for example. Nobody would expect users to set those up manually, not everyone needs to know about all the different flags that go in there.
Top-level package sets provide better UX than config options.

We should only add package sets for the first reason, not "for better UX". Maybe we can think about better UX separately.

Why is passing those via system on the command line so hard?

OK, I tried it. I need:

pkgsBolt: nix-build --arg crossSystem '{ system="x86_64-linux"; useBolt=true; }' -A hello
pkgsLLVM: nix-build --arg crossSystem '{ system="x86_64-linux"; useLLVM=true; linker="lld"; }' -A hello

Really annoying that I have to pass my own system as well. Doing nix-build -A pkgsBolt.hello... is so much easier.

Maybe we can introduce some kind of attribute-based-config system for CLI UX? I'm thinking of doing something like:

nix-build -A <something>.useLLVM.withLLD.pkgs.hello
nix-build -A <something>.useBolt.pkgs.hello

Where:

We use a prefix (not sure about the name, <something> here) to avoid calling nixpkgsFun before we need to - and also to keep the packages namespace clean.
Config options are exposed as attributes where sensible.
pkgs actually calls nixpkgsFun, thus "ends" the configuration part.

This way, we have better UX, but don't need to add package sets all the time. Maybe we can also do shortcuts like nix-build -A <something>.static.pkgs.hello, where static would be a collection of different options.

RossComputerGuy · 2025-04-24T14:04:21Z

OK, I tried it. I need:
* `pkgsBolt`: `nix-build --arg crossSystem '{ system="x86_64-linux"; useBolt=true; }' -A hello`

* `pkgsLLVM`: `nix-build --arg crossSystem '{ system="x86_64-linux"; useLLVM=true; linker="lld"; }' -A hello`
Really annoying that I have to pass my own system as well. Doing nix-build -A pkgsBolt.hello... is so much easier.

I feel like this kinda answers itself. Plus, once #365057 is merged and we stop supporting useLLVM it'll be even more complicated since you will have to supply each attribute specifically.

Maybe we can introduce some kind of attribute-based-config system for CLI UX?

I agree but I think we should stick to the variants approach until we're able to design a better system. It'll also likely be some amount of time we want both for compatibility.

Ericson2314 · 2025-04-24T16:53:12Z

Yeah this is good. I look forward to everything being moved!

wolfgangwalther · 2025-04-24T17:06:34Z

This is a breaking change for everyone using one of those package sets right now, so I say we can't merge this before branch-off anymore.

RossComputerGuy · 2025-04-24T23:13:13Z

Is it a breaking change? It only really effects people if they turn off allowVariants which we never do right now. The plan is to change that after the release this PR lands in.

alyssais

Love the direction. Glad somebody is finally getting around to this.

alyssais · 2025-04-26T15:53:10Z

pkgs/top-level/config.nix

They're not really subsets. They contain just as much as the original set.

What's a better way to explaining it?

They're differently instantiated package sets.

I've reworded things slightly, idk if that's any better.

alyssais · 2025-04-26T15:55:35Z

pkgs/top-level/stage.nix

Given how simple this was before I'm not sure the quite complex mkOptionalOverlay function is worth it?

Ok, I've dropped the commit which did that. We can figure something out later.

alyssais · 2025-04-26T15:56:43Z

doc/release-notes/rl-2505.section.md

I don't think that's a desirable change — one of the reasons to disable aliases is to get rid of throws when iterating over package sets. Usually that's only relevant for sets below the top level but I don't think we'd want the top level to work confusingly differently.

Hmm, would we want to add another option for turning off the optional overlay?

alyssais · 2025-04-27T08:03:06Z

pkgs/top-level/config.nix

Suggested change

This allows for using different toolchains, libc's, or global build changes across nixpkgs.

This allows for using different toolchains, libcs, or global build changes across nixpkgs.

And maybe it would be good to explain why you'd want to enable/disable this?

alyssais · 2025-04-27T08:03:44Z

pkgs/top-level/stage.nix

This complicated function is still here and is now only used once.

Ok, removed

Ericson2314 · 2025-05-19T15:32:20Z

I am still a bit confused, because I hate variants but like this PR.

If we want to provide short-hands, that can still be in the hydra jobset, but a new attribute set. Likewise for the CLI. Perhaps making those new entry points is sort of the stuff that @wolfgangwalther is already planning.

I don't really worry about drift in the meaning of "aliases" as that also seems like something we can clear up after the fact?

RossComputerGuy · 2025-05-19T15:45:12Z

Yeah, and I don't see how we could get to @wolfgangwalther's work without doing something like this at least for the interim. The goal here is to move packages in nixpkgs off of using pkgs*.

wolfgangwalther · 2025-05-19T15:49:58Z

The goal here is to move packages in nixpkgs off of using pkgs*.

And I fully agree with that goal. I just think we should use allowAliases, not introduce allowVariants.

RossComputerGuy · 2025-05-19T15:51:57Z

I've explained that that isn't really optimal since it's not really an alias, it'll break existing things if people turned aliases off, and if someone wants it for CI then every throw in aliases will show up.

wolfgangwalther · 2025-05-19T16:01:43Z

I understand your position, because you don't want any breaking changes for existing use-cases.

My position is different, because I am not only OK with breaking changes, in small steps - in this case, I actively want them. If we ever want to get away from those variants, we must break them eventually. This seems like a reasonable first step.

Ericson2314 · 2025-05-19T16:30:53Z

Can we do allowVariants for 25.05, and no allowVariants for 25.11?

wolfgangwalther · 2025-05-19T16:34:53Z

Can we do allowVariants for 25.05, and no allowVariants for 25.11?

As said earlier, I'm not a big fan of introducing it now, if we already have the intent to remove it again.

But to be clear: I am not blocking this PR, otherwise I would have "Requested Changes". So if the three of you agree with the current approach (and it looks like this, given the two approvals), then you might still want to go ahead with it in this form. I think I have voiced all my concerns :D

RossComputerGuy · 2025-05-19T16:51:45Z

Can we do allowVariants for 25.05, and no allowVariants for 25.11?

That's been my plan.

But to be clear: I am not blocking this PR, otherwise I would have "Requested Changes".

Sounds good.

Ericson2314 · 2025-05-19T16:55:47Z

@RossComputerGuy Can you add modify the docs to say we expect allowVariants to be temporary, ideally removed by 25.11?

RossComputerGuy · 2025-05-19T16:58:03Z

After work I can, about 8 hours from now. Unless someone is able to do it sooner.

RossComputerGuy · 2025-05-21T02:56:22Z

expect allowVariants to be temporary, ideally removed by 25.11

Just now getting to this, I don't think 25.11 would remove it based on this:

Can we do allowVariants for 25.05, and no allowVariants for 25.11?

Would it be better to note that it is likely to be dropped after 26.05?

Of course it could be removed after 25.11 but it really depends on the pace of how we can get things through.

Ericson2314 · 2025-05-21T03:20:36Z

Just leave it vague and say it will be removed in a future stable release.

RossComputerGuy · 2025-05-21T04:09:21Z

Done in #409265

Ericson2314 · 2025-05-21T17:00:47Z

@RossComputerGuy uh is that the PR you meant to link?

RossComputerGuy · 2025-05-21T17:06:10Z

No apparently the PR didn't get created even though I remember making it, it should be #409484

RossComputerGuy · 2025-05-24T00:54:12Z

As per @Ericson2314's request (#409484 (comment)), we're backporting this to 25.05

nixpkgs-ci · 2025-05-24T00:54:27Z

Successfully created backport PR for release-25.05:

[Backport release-25.05] pkgs/top-level/stage.nix: move most nixpkgs sets to variants #410362

wolfgangwalther · 2025-07-18T19:40:31Z

pkgs/top-level/stage.nix

+  variants = import ./variants.nix {
+    inherit
+      lib
+      nixpkgsFun
+      stdenv
+      overlays
+      makeMuslParsedPlatform
+      ;
+  };


So we now have an allowVariants config option - but it doesn't do anything. Its value is not used anywhere. All the variants are still available when setting it to false.

What was the intent to introduce this again, when apparently nobody has used it, yet?

The plan is to set it to true in CI once we have a competent replacement. #410056 is one that I've worked on. We're kinda blocked on that with GCC NG, @Ericson2314 and I are working on it.

The plan is to set it to true in CI

I think this is just a typo and you mean false.

I think you missed my point: When I use allowVariants = false, then it doesn't work. It doesn't do anything. The PR body mentions this:

Also add mkOptionalOverlay, this allows us to throw an error when an attribute from an overlay isn't available unless enabled.

But apparently this has been lost during later refactors of the PR. Now all variants are unconditionally included all the time.

We currently have a prominently placed release note (including a backport) which mentions that we have a new config option allowVariants, but this config option is disfunctional.

I hope that clarifies it.

I think this is just a typo and you mean false.

Yes, I meant false. It was a typo.

I think you missed my point: When I use allowVariants = false, then it doesn't work. It doesn't do anything. The PR body mentions this:

Also add mkOptionalOverlay, this allows us to throw an error when an attribute from an overlay isn't available unless enabled.

But apparently this has been lost during later refactors of the PR. Now all variants are unconditionally included all the time.

mkOptionalOverlay was removed due to a review.

We currently have a prominently placed release note (including a backport) which mentions that we have a new config option allowVariants, but this config option is disfunctional.

I hope that clarifies it.

Huh, lemme check if this works. I remember testing it and it did work.

It's easy to confirm that it can't work. Just grep for allowVariants. It's only used in top-level/config and the release notes :)

Oh now I see what's going on, after #400351 (comment) I didn't add in lib.optionalAttrs for the variants attribute in pkgs/top-level/stage.nix.

RossComputerGuy mentioned this pull request Apr 20, 2025

llvmPackages.bolt: add experimental bolt hook #375435

Open

13 tasks

RossComputerGuy force-pushed the feat/nixpkgs-variants branch from 5e41f62 to 3f3fcf0 Compare April 20, 2025 15:31

RossComputerGuy force-pushed the feat/nixpkgs-variants branch from 3f3fcf0 to 4585e60 Compare April 20, 2025 15:59

github-actions bot added 8.has: documentation This PR adds or changes documentation and removed 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Apr 20, 2025

RossComputerGuy marked this pull request as ready for review April 20, 2025 16:11

nix-owners bot requested a review from Ericson2314 April 20, 2025 16:12

wolfgangwalther reviewed Apr 24, 2025

View reviewed changes

doc/release-notes/rl-2505.section.md Outdated Show resolved Hide resolved

pkgs/top-level/config.nix Outdated Show resolved Hide resolved

pkgs/top-level/variants.nix Outdated Show resolved Hide resolved

wolfgangwalther requested a review from alyssais April 24, 2025 12:40

RossComputerGuy force-pushed the feat/nixpkgs-variants branch from a48535d to f3f6e5c Compare April 24, 2025 13:54

ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 24, 2025

RossComputerGuy force-pushed the feat/nixpkgs-variants branch from f3f6e5c to e12e396 Compare April 24, 2025 14:36

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 24, 2025

Ericson2314 approved these changes Apr 24, 2025

View reviewed changes

wegank added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Apr 24, 2025

alyssais requested changes Apr 26, 2025

View reviewed changes

RossComputerGuy force-pushed the feat/nixpkgs-variants branch 2 times, most recently from b662e26 to 58b5540 Compare April 27, 2025 05:05

alyssais requested changes Apr 27, 2025

View reviewed changes

wolfgangwalther mentioned this pull request Apr 27, 2025

Add Nixpkgs entrypoint lib, <flake>.lib.nixpkgs.configure, (import <nixpkgs>).configure #402298

Open

13 tasks

RossComputerGuy merged commit 4eb6667 into NixOS:master May 19, 2025
20 of 21 checks passed

RossComputerGuy deleted the feat/nixpkgs-variants branch May 19, 2025 16:52

risicle mentioned this pull request May 19, 2025

cc-wrapper: add support for strictflexarrays1 & strictflexarrays3 hardening flags #400408

Merged

13 tasks

RossComputerGuy mentioned this pull request May 23, 2025

pkgs/top-level/all-packages.nix: add cc & ccChooser #409851

Open

13 tasks

RossComputerGuy added the backport release-25.05 label May 24, 2025

nixpkgs-ci bot mentioned this pull request May 24, 2025

[Backport release-25.05] pkgs/top-level/stage.nix: move most nixpkgs sets to variants #410362

Merged

1 task

nixpkgs-ci bot added the 8.has: port to stable This PR already has a backport to the stable release. label May 24, 2025

RossComputerGuy mentioned this pull request May 24, 2025

pkgs/top-level/stage.nix: add crossStdenv #410056

Draft

13 tasks

wolfgangwalther reviewed Jul 18, 2025

View reviewed changes

RossComputerGuy mentioned this pull request Jul 18, 2025

pkgs/top-level/stage.nix: fix allowVariants with variants set #426490

Merged

13 tasks

wolfgangwalther mentioned this pull request Aug 6, 2025

Provide middle-ground overlay between pkgsMusl and pkgsStatic #61575

Open

siraben mentioned this pull request Sep 1, 2025

stdenv: add uutils-coreutils stdenv #2 #407301

Open

13 tasks

wolfgangwalther mentioned this pull request Sep 29, 2025

pkgsUnfree: init #435082

Closed

	This allows for using different toolchains, libc's, or global build changes across nixpkgs.
	This allows for using different toolchains, libcs, or global build changes across nixpkgs.

Uh oh!

Conversation

RossComputerGuy commented Apr 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfgangwalther commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RossComputerGuy commented Apr 24, 2025

Uh oh!

Ericson2314 commented Apr 24, 2025

Uh oh!

wolfgangwalther commented Apr 24, 2025

Uh oh!

RossComputerGuy commented Apr 24, 2025

Uh oh!

alyssais left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ericson2314 commented May 19, 2025

Uh oh!

RossComputerGuy commented May 19, 2025

Uh oh!

wolfgangwalther commented May 19, 2025

Uh oh!

RossComputerGuy commented May 19, 2025

Uh oh!

wolfgangwalther commented May 19, 2025

Uh oh!

Ericson2314 commented May 19, 2025

Uh oh!

wolfgangwalther commented May 19, 2025

Uh oh!

RossComputerGuy commented May 19, 2025

Uh oh!

Uh oh!

Ericson2314 commented May 19, 2025

Uh oh!

RossComputerGuy commented May 19, 2025

Uh oh!

RossComputerGuy commented May 21, 2025

Uh oh!

Ericson2314 commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RossComputerGuy commented May 21, 2025

Uh oh!

Ericson2314 commented May 21, 2025

Uh oh!

RossComputerGuy commented May 21, 2025

Uh oh!

RossComputerGuy commented May 24, 2025

RossComputerGuy commented Apr 20, 2025 •

edited

Loading

wolfgangwalther commented Apr 24, 2025 •

edited

Loading

Ericson2314 commented May 21, 2025 •

edited

Loading