Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 51 additions & 9 deletions lib/customisation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,18 @@ rec {
f = if isFunction fn then fn else import fn;
fargs = functionArgs f;

aliasArgs = filterAttrs (name: _: autoArgs._isAlias or (_: false) name) fargs;

aliasErrorForArg =
arg:
let
loc = builtins.unsafeGetAttrPos arg fargs;
in
"Function expects alias \"${arg}\" at ${loc.file}:${toString loc.line}.";

# Only show the error for the first alias
aliasError = aliasErrorForArg (head (attrNames aliasArgs));

# All arguments that will be passed to the function
# This includes automatic ones and ones passed explicitly
allArgs = intersectAttrs fargs autoArgs // args;
Expand Down Expand Up @@ -300,7 +312,7 @@ rec {
else
", did you mean ${concatStringsSep ", " (lib.init suggestions)} or ${lib.last suggestions}?";

errorForArg =
missingErrorForArg =
arg:
let
loc = builtins.unsafeGetAttrPos arg fargs;
Expand All @@ -309,17 +321,18 @@ rec {
+ "${loc.file}:${toString loc.line}${prettySuggestions (getSuggestions arg)}";

# Only show the error for the first missing argument
error = errorForArg (head (attrNames missingArgs));
missingError = missingErrorForArg (head (attrNames missingArgs));

in
if missingArgs == { } then
makeOverridable f allArgs
# This needs to be an abort so it can't be caught with `builtins.tryEval`,
# which is used by nix-env and ofborg to filter out packages that don't evaluate.
# This way we're forced to fix such errors in Nixpkgs,
# which is especially relevant with allowAliases = false
# These need to be abort so they can't be caught with `builtins.tryEval`,
# which is used by nix-env and CI to filter out packages that don't evaluate.
# This way we're forced to fix such errors in Nixpkgs.
if aliasArgs != { } then
abort "lib.customisation.callPackageWith: ${aliasError}"
Comment on lines +327 to +331
Copy link
Member

Choose a reason for hiding this comment

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

If we made aliases become throws when !config.allowAliases, would we need this? i.e., they would be filtered out by CI, but would it also just filter out packages that use those aliases? (Just thinking about if we can minimize the number of places that have to know about aliases, for both cleanliness and performance.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would still be possible to add aliases to the package arguments, if eval doesn't hit them.

We had this case for python3Packages, where this was already tried - somone tried to somehow override python3Packages in the python scope with aliases turned on or so, to prevent this from being used - but it didn't work, because Ci is swallowing those errors.

This could be hacked by putting abort in that place, I guess - but here I just demonstrated how we could use the structured alias information to implement such a check. This has a tiny effect on performance, so we can also entirely leave this part out, if we want to.

else if missingArgs != { } then
abort "lib.customisation.callPackageWith: ${missingError}"
else
abort "lib.customisation.callPackageWith: ${error}";
makeOverridable f allArgs;

/**
Like callPackage, but for a function that returns an attribute
Expand Down Expand Up @@ -365,6 +378,35 @@ rec {
else
mapAttrs mkAttrOverridable pkgs;

makeAlias =
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We should call this mkAlias for consistency with mkDerivation, I think.

Also, I feel that “alias” may be a confusing word in general. llvmPackages = llvmPackages_21; is an “alias”, but it’s distinct from both the end‐user aliases.nix aliases we have for convenience (like llvmPackages_latest), temporary compatibility aliases, and from removed‐package throws. I do not know what a better word would be; maybe having helper functions will reduce the need here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I started in customisation.nix, I had makeOverridablenext door, so... but yeah, I'm fine with mkX, too.

let
currentRelease = lib.versions.majorMinor lib.version;
in
self: name:
{
type,
package ? null,
reason ? null,
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to support multiple reasons, if we can ensure the interface remains convenient. I also think that we should ensure the reasons are structured by default rather than free‐form strings, because our existing throws are wildly inconsistent, and it’s much more convenient to write unmaintained than unmaintained in Nixpkgs but can give the wrong idea about upstream status.

Vague draft: reason.broken = true; reason.unmaintained = true; → “it was broken and unmaintained in Nixpkgs”? And then we could also have reason.insecure = true; or reason.insecure = ["CVE1" "CVE2"]; or such, and of course reason.custom = "I just really don’t like it";.

release ? null,
Copy link
Member

Choose a reason for hiding this comment

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

I have a mild preference for since over release here – it seems less ambiguous that it’s when the alias was added, rather than when it’s due for complete removal.

}:
assert type == "alias";
Copy link
Member

Choose a reason for hiding this comment

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

Seems like busywork to have to set this? It should be relatively low‐ceremony to stub out a package. lib.optionalAttrs (!config.allowAliases) is already annoying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, that's the part that we need to keep the typing a bit stronger than freeform. If we ever want to extend this to other things like scopes or so, we better have type here, so we're not "an attrset means alias" deadend.

if release == null then
self.${package}
else if release != currentRelease then
if package == null then
abort "Throw for '${name}' was added in ${release}. The alias must now be removed."
Copy link
Member

Choose a reason for hiding this comment

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

A warning seems like it may be better? Then you can scoop them all up at once, and perhaps do something with unsafeGetAttrPos to list all the files that need changing.

else
abort "Warning for '${name}' was added in ${release}. The alias must now be converted to a throw."
Comment on lines +398 to +399
Copy link
Member

Choose a reason for hiding this comment

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

Why? We know what the current release is and when the warning was added. We can automatically turn it into a throwing alias without making the release manager do busywork. Complete removal can’t be done in this way, of course, but the automatic transition from warning to throw is IMO one of the nicest advantages of a more structured alias system.

You can say that it causes issues if the package = …; stops working, but that’s okay; it’s not a big burden to just drop that line on such changes. I suppose the worry is that this offsets the removal period by one release cycle? But we could just say that things that immediately become throws stay for two releases and things that point to packages are a warning for one cycle and throw for the second – that way it’s consistent and package = …; or not doesn’t affect the timing.

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 consider Nixpkgs versions mostly freeform, so we can only say "still matches the current release" or "was added in a previous release".

Now, if we don't want to stick with these forever, then we need two points where things change:

  • One from alias+warning to error
  • One removing the error

If we only have one piece of information, we can't identify both.

To be able to identify both, we must put "structure" into the Nixpkgs version, too. Aka we must give it a semantic meaning, saying "25.11" is "25.05 + 1" and "26.05" is "25.05 + 2". We can do that, but I'm not sure if and how, yet.

else if package == null || !self.config.allowAliases then
{
type = "error";
Copy link
Member

Choose a reason for hiding this comment

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

We use _type in a fair number of places, which seems like it might be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is to say we already have type = "derivation", so let's stick to that?

Aka we never want something like a derivation with a different _type. So arguably _type would weaken the typing.

meta = throw "'${name}' was removed because it was ${reason}.";
Copy link
Member

Choose a reason for hiding this comment

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

Why the choice of meta specifically here? To ensure that even nix-env etc. are unhappy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

meta is specifically for lib.meta.availableOn to fail.

(we should probably also add __toString, just to make sure)

}
else
lib.warnOnInstantiate
"'${name}' was renamed to '${package}'. The alias will be removed in the next release."
Copy link
Member

Choose a reason for hiding this comment

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

We should at least say “the next release after […]”.

self.${package};

/**
Add attributes to each output of a derivation without changing
the derivation itself and check a given condition when evaluating.
Expand Down
1 change: 1 addition & 0 deletions lib/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ let
makeOverridable
callPackageWith
callPackagesWith
makeAlias
extendDerivation
hydraJob
makeScope
Expand Down
5 changes: 5 additions & 0 deletions pkgs/by-name/do/docker-sync/package.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
type = "alias";
release = "25.11";
reason = "broken and unmaintained";
}
5 changes: 5 additions & 0 deletions pkgs/by-name/fo/follow/package.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
type = "alias";
release = "25.11";
reason = "renamed to folo";
}
5 changes: 5 additions & 0 deletions pkgs/by-name/op/open-timeline-io/package.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
type = "alias";
release = "25.11";
package = "opentimelineio";
}
Comment on lines +1 to +5
Copy link
Member

Choose a reason for hiding this comment

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

Ah… so the reason for the mandatory type = "alias"; is that we don’t have a wrapper function here at all.

But this is no good: you can have fooFull replaced with foo.override { withStuff = true; } and this does happen.

I know this ties in to supporting more non‐package stuff in by-name, but I think this would at least have to be package = { opentimelineio }: opentimelineio; or something. (Also, we should probably call it target or something rather than package, because we can have aliases for package sets too.)

Is the idea that this makes .override work properly, compared to { mkAlias, opentimelineio }: mkAlias { release = "25.11"; target = opentimelineio; }? Or is there a more fundamental reason that won’t work with this approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there are a couple of things going on here:

  • We can't callPackage, because then the "argument is an alias" check won't work - it hits infinite recursion, because we have no way of telling whether something is an alias before we do callPackage. If we're happy to give up on that check, which I am not sold on, yet, we can probably also put this behind callPackage.
  • If we wanted to have overrides, we could still have this as an attrset, but have a function inside, like override = { foo, bar }: bar.override { foo } or whatever (don't worry the details, just meant to say that we can have a function in the attrset, too)
  • Yes, this will cause fewer complications with .override if we want overriden aliases to be done via this system as well.

4 changes: 0 additions & 4 deletions pkgs/top-level/aliases.nix
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,6 @@ mapAliases {
docker_27 = throw "'docker_27' has been removed because it has been unmaintained since May 2025. Use docker_28 or newer instead."; # Added 2025-06-15
docker-compose_1 = throw "'docker-compose_1' has been removed because it has been unmaintained since May 2021. Use docker-compose instead."; # Added 2024-07-29
docker-distribution = distribution; # Added 2023-12-26
docker-sync = throw "'docker-sync' has been removed because it was broken and unmaintained"; # Added 2025-08-26
dolphin-emu-beta = dolphin-emu; # Added 2023-02-11
dolphinEmu = throw "'dolphinEmu' has been renamed to/replaced by 'dolphin-emu'"; # Converted to throw 2024-10-17
dolphinEmuMaster = throw "'dolphinEmuMaster' has been renamed to/replaced by 'dolphin-emu-beta'"; # Converted to throw 2024-10-17
Expand Down Expand Up @@ -926,7 +925,6 @@ mapAliases {
fmt_8 = throw "fmt_8 has been removed as it is obsolete and was no longer used in the tree"; # Added 2024-11-12
fntsample = throw "fntsample has been removed as it is unmaintained upstream"; # Added 2025-04-21
foldingathome = throw "'foldingathome' has been renamed to/replaced by 'fahclient'"; # Converted to throw 2024-10-17
follow = lib.warnOnInstantiate "follow has been renamed to folo" folo; # Added 2025-05-18
forgejo-actions-runner = forgejo-runner; # Added 2024-04-04
fornalder = throw "'fornalder' has been removed as it is unmaintained upstream"; # Added 2025-01-25
foundationdb71 = throw "foundationdb71 has been removed; please upgrade to foundationdb73"; # Added 2024-12-28
Expand Down Expand Up @@ -1839,7 +1837,6 @@ mapAliases {
openssl_3_0 = openssl_3; # Added 2022-06-27
opensycl = lib.warnOnInstantiate "'opensycl' has been renamed to 'adaptivecpp'" adaptivecpp; # Added 2024-12-04
opensyclWithRocm = lib.warnOnInstantiate "'opensyclWithRocm' has been renamed to 'adaptivecppWithRocm'" adaptivecppWithRocm; # Added 2024-12-04
open-timeline-io = lib.warnOnInstantiate "'open-timeline-io' has been renamed to 'opentimelineio'" opentimelineio; # Added 2025-08-10
opentofu-ls = lib.warnOnInstantiate "'opentofu-ls' has been renamed to 'tofu-ls'" tofu-ls; # Added 2025-06-10
openvdb_11 = throw "'openvdb_11' has been removed in favor of the latest version'"; # Added 2025-05-03
opera = throw "'opera' has been removed due to lack of maintenance in nixpkgs"; # Added 2025-05-19
Expand Down Expand Up @@ -1958,7 +1955,6 @@ mapAliases {
pgroonga = throw "'pgroonga' has been removed. Use 'postgresqlPackages.pgroonga' instead."; # Added 2025-07-19
pgtap = throw "'pgtap' has been removed. Use 'postgresqlPackages.pgtap' instead."; # Added 2025-07-19
plv8 = throw "'plv8' has been removed. Use 'postgresqlPackages.plv8' instead."; # Added 2025-07-19
postcss-cli = throw "postcss-cli has been removed because it was broken"; # added 2025-03-24
Copy link
Member

Choose a reason for hiding this comment

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

Is it intentional that this was removed in this PR without replacement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because it's exactly the case that would hit abort. The throw as added in 2025-03, so needs to be removed in 25.11 - so right now. This is done in separate commits, it is first moved to an alias, which is then confirmed to hit the abort - and then later removed in a second commit to clean that up. This shows how this workflow would work at branch-off.

postgis = throw "'postgis' has been removed. Use 'postgresqlPackages.postgis' instead."; # Added 2025-07-19
tegaki-zinnia-japanese = throw "'tegaki-zinnia-japanese' has been removed due to lack of maintenance"; # Added 2025-09-10
tet = throw "'tet' has been removed for lack of maintenance"; # Added 2025-10-12
Expand Down
26 changes: 19 additions & 7 deletions pkgs/top-level/by-name-overlay.nix
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@ let
mergeAttrsList
;

# Package files for a single shard
# Type: String -> String -> AttrsOf Path
inherit (lib.customisation)
makeAlias
;

# Package imports for a single shard
# Type: String -> String -> AttrsOf Any
namesForShard =
shard: type:
if type != "directory" then
Expand All @@ -32,27 +36,35 @@ let
# Additionally in either of those alternatives, we would have to duplicate the hardcoding of "README.md"
{ }
else
mapAttrs (name: _: baseDirectory + "/${shard}/${name}/package.nix") (
mapAttrs (name: _: import (baseDirectory + "/${shard}/${name}/package.nix")) (
readDir (baseDirectory + "/${shard}")
);

# The attribute set mapping names to the package files defining them
# The attribute set mapping names to the package expressions defining them
# This is defined up here in order to allow reuse of the value (it's kind of expensive to compute)
# if the overlay has to be applied multiple times
packageFiles = mergeAttrsList (mapAttrsToList namesForShard (readDir baseDirectory));
packages = mergeAttrsList (mapAttrsToList namesForShard (readDir baseDirectory));
in
# TODO: Consider optimising this using `builtins.deepSeq packageFiles`,
# which could free up the above thunks and reduce GC times.
# Currently this would be hard to measure until we have more packages
# and ideally https://github.com/NixOS/nix/pull/8895
self: super:
{
_isAlias = name: packages.${name}.type or null == "alias";
Copy link
Member

Choose a reason for hiding this comment

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

I think we should use double underscores for this, to match __spliced and the like.

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 think double underscores are wrong, my understanding is that double underscores mean "internal to Nix", while single underscore means "internal to Nixpkgs". __spliced is just.. bad.


# This attribute is necessary to allow CI to ensure that all packages defined in `pkgs/by-name`
# don't have an overriding definition in `all-packages.nix` with an empty (`{ }`) second `callPackage` argument.
# It achieves that with an overlay that modifies both `callPackage` and this attribute to signal whether `callPackage` is used
# and whether it's defined by this file here or `all-packages.nix`.
# TODO: This can be removed once `pkgs/by-name` can handle custom `callPackage` arguments without `all-packages.nix` (or any other way of achieving the same result).
# Because at that point the code in ./stage.nix can be changed to not allow definitions in `all-packages.nix` to override ones from `pkgs/by-name` anymore and throw an error if that happens instead.
_internalCallByNamePackageFile = file: self.callPackage file { };
_internalCallByNamePackageFile = fn: self.callPackage fn { };
}
// mapAttrs (name: self._internalCallByNamePackageFile) packageFiles
// mapAttrs (
name: value:
if value.type or null == "alias" then
makeAlias self name value
else
self._internalCallByNamePackageFile value
) packages
135 changes: 110 additions & 25 deletions pkgs/top-level/linux-kernels.nix
Original file line number Diff line number Diff line change
Expand Up @@ -290,31 +290,116 @@ in

linux_hardened = hardenedKernelFor packageAliases.linux_default.kernel { };
}
// lib.optionalAttrs config.allowAliases {
linux_4_19 = throw "linux 4.19 was removed because it will reach its end of life within 24.11";
linux_6_9 = throw "linux 6.9 was removed because it has reached its end of life upstream";
linux_6_10 = throw "linux 6.10 was removed because it has reached its end of life upstream";
linux_6_11 = throw "linux 6.11 was removed because it has reached its end of life upstream";
linux_6_13 = throw "linux 6.13 was removed because it has reached its end of life upstream";
linux_6_14 = throw "linux 6.14 was removed because it has reached its end of life upstream";
linux_6_15 = throw "linux 6.15 was removed because it has reached its end of life upstream";

linux_5_10_hardened = throw "linux_hardened on nixpkgs only contains latest stable and latest LTS";
linux_5_15_hardened = throw "linux_hardened on nixpkgs only contains latest stable and latest LTS";
linux_6_1_hardened = throw "linux_hardened on nixpkgs only contains latest stable and latest LTS";
linux_6_6_hardened = throw "linux_hardened on nixpkgs only contains latest stable and latest LTS";

linux_4_19_hardened = throw "linux 4.19 was removed because it will reach its end of life within 24.11";
linux_5_4_hardened = throw "linux_5_4_hardened was removed because it was broken";
linux_6_9_hardened = throw "linux 6.9 was removed because it has reached its end of life upstream";
linux_6_10_hardened = throw "linux 6.10 was removed because it has reached its end of life upstream";
linux_6_11_hardened = throw "linux 6.11 was removed because it has reached its end of life upstream";
linux_6_13_hardened = throw "linux 6.13 was removed because it has reached its end of life upstream";
linux_6_14_hardened = throw "linux 6.14 was removed because it has reached its end of life upstream";
linux_6_15_hardened = throw "linux 6.15 was removed because it has reached its end of life upstream";

linux_ham = throw "linux_ham has been removed in favour of the standard kernel packages";
}
// (
let
aliases = {
linux_4_19 = {
type = "alias";
release = "25.11";
reason = "it will reach its end of life within 24.11";
};
linux_6_9 = {
type = "alias";
release = "25.11";
reason = "it has reached its end of life upstream";
};
linux_6_10 = {
type = "alias";
release = "25.11";
reason = "it has reached its end of life upstream";
};
linux_6_11 = {
type = "alias";
release = "25.11";
reason = "it has reached its end of life upstream";
};
linux_6_13 = {
type = "alias";
release = "25.11";
reason = "it has reached its end of life upstream";
};
linux_6_14 = {
type = "alias";
release = "25.11";
reason = "it has reached its end of life upstream";
};
linux_6_15 = {
type = "alias";
release = "25.11";
reason = "it has reached its end of life upstream";
};

linux_5_10_hardened = {
type = "alias";
release = "25.11";
reason = "linux_hardened on nixpkgs only contains latest stable and latest LTS";
};
linux_5_15_hardened = {
type = "alias";
release = "25.11";
reason = "linux_hardened on nixpkgs only contains latest stable and latest LTS";
};
linux_6_1_hardened = {
type = "alias";
release = "25.11";
reason = "linux_hardened on nixpkgs only contains latest stable and latest LTS";
};
linux_6_6_hardened = {
type = "alias";
release = "25.11";
reason = "linux_hardened on nixpkgs only contains latest stable and latest LTS";
};

linux_4_19_hardened = {
type = "alias";
release = "25.11";
reason = "linux 4.19 was removed because it will reach its end of life within 24.11";
};
linux_5_4_hardened = {
type = "alias";
release = "25.11";
reason = "linux_5_4_hardened was removed because it was broken";
};
linux_6_9_hardened = {
type = "alias";
release = "25.11";
reason = "linux 6.9 was removed because it has reached its end of life upstream";
};
linux_6_10_hardened = {
type = "alias";
release = "25.11";
reason = "linux 6.10 was removed because it has reached its end of life upstream";
};
linux_6_11_hardened = {
type = "alias";
release = "25.11";
reason = "linux 6.11 was removed because it has reached its end of life upstream";
};
linux_6_13_hardened = {
type = "alias";
release = "25.11";
reason = "linux 6.13 was removed because it has reached its end of life upstream";
};
linux_6_14_hardened = {
type = "alias";
release = "25.11";
reason = "linux 6.14 was removed because it has reached its end of life upstream";
};
linux_6_15_hardened = {
type = "alias";
release = "25.11";
reason = "linux 6.15 was removed because it has reached its end of life upstream";
};

linux_ham = {
type = "alias";
release = "25.11";
reason = "in favour of the standard kernel packages";
};
};
in
(lib.mapAttrs (lib.makeAlias kernels) aliases) // { _isAlias = name: aliases ? ${name}; }
Copy link
Member

Choose a reason for hiding this comment

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

Yuck. This is too much ceremony – I think we have to find something better here. Whether that’s just defining these under an __aliases sub‐attribute‐set, or having these in line as mkAlias calls adjacent to the regular packages, or whatever. (I haven’t reviewed the implementation strategy enough to predict the issues these will cause, but I think this is no good, and I believe we can do better by making our scope creation functions etc. smarter.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I am not happy with this, yet, either. I added this to have a look at how this would look like... but yeah, don't like it, yet.

My current idea would be: These kinds of aliases don't need to support non by-name structures at all. Let's implement them with by-name in mind, and then work on migrating everything to by-name. This renders this case void.

)
)
);
/*
Expand Down
Loading