Incidentally I have no idea why this shows as a rebuild in #440273 when it fails to evaluate before and fails to evaluate after. @wolfgangwalther? :)

Looks like this only fails with aliases enabled. In CI, aliases are disabled, so it passes both before and after.

Aha, that makes sense. It would be nice if there was a reliable way to detect aliases, but I suppose this is the reasonable fix for now.

I have a marvellous design for a better alias system that this margin is too small to contain that would make this easy to do. Although really the whole “filtering the entire set of package names” thing here is silly and we should just have an explicit attribute set of GCC and LLVM versions, also.

I have a marvellous design for a better alias system that this margin is too small to contain that would make this easy to do.

Please draw it up elsewhere! I started writing something up only to realize that this wouldn't fit here either - but it wasn't fully thought through either... it all started with representing an alias as a dummy derivation...

How about I try to fit the rough outline into the margin and if you think it’s reasonable that’ll incentivize me to put it up in a proper issue :)

Problems with the current system:

1. We mix “permanent aliases for convenience” and “aliases with an expiry date” in a fairly unprincipled manner.
2. Users of non‐throwing expiring aliases do not get warnings about them, so they have no way to know they’re meant to migrate until they become hard errors.
3. We write ad‐hoc messages for all of our throws even though 90% of them are down to like three or four causes.
4. The script to delete old aliases works line‐by‐line and can break evaluation. It also goes by date, rather than release, even though commits don’t know when they’ll get merged and it really makes no sense to have a cut‐off that isn’t based on the release an alias was added.

So:

1. Let’s separate out convenience aliases and expiring ones, and represent the latter as structured data, including the release the alias was added, and either the renamed package it points to, or a reason for removal (with stock options like insecure, unmaintained, and so on, to cover the vast majority of cases).
2. Renames get transformed into lib.warnOnInstantiate "foo has been renamed to bar, please update" bar; removals get transformed into throws.
3. The removal script operates on the structured data and applies the process of “turn warnings into throws after one release”, “drop throws a release after they are added or become throws”.

This is as far as I got thinking about this previously. We could then expose the alias data as pkgs.aliases or something and consume it here. But insofar as this still involves config.allowAliases conditionals and doesn’t handle aliases in package sets, it’s certainly still not ideal.

So how about, taking it a bit further: we have a mkAlias function to handle the structured data → alias conversion; if it’s a rename and config.allowAliases, it behaves as described above; otherwise, it produces a throw. This is a slight downgrade in “alias protection” from the current config.allowAliases: stuff can still take the alias as an input as long as it never forces its evaluation, which would then break when the alias is later removed. Not sure how big a deal that is – other than making the removal process less automated, I can imagine it breaking a passthru.* thing that CI doesn’t evaluate, for instance.

The temptation is to have callPackage detect when it’s passing along an alias when !config.allowAliases and break it. That would require callPackage to become stricter in the values it passes along… which should be fine for derivations, although I’m not sure how expensive it is to do a simple builtins.seq like that, but could be dodgy for things like flag values. I don’t know if there’s a good way we could have mkAlias signal to callPackage when it’s going to be passing along an alias other than through its return value, because you can have aliases that aren’t in proper scopes or anything. Perhaps we could do the alias‐filtering at scope creation time to encourage the use of explicit scopes – mkAlias always returns a structured value, putting it in a scope gets detected, rewrites the value as appropriate, and leaves a record of the structure ddata in __aliases? And then this could look at lib.removeAttrs llvmPackageSets (lib.attrNames llvmPackageSets.__aliases), or such.

4. The script to delete old aliases works line‐by‐line and can break evaluation. It also goes by date, rather than release, even though commits don’t know when they’ll get merged and it really makes no sense to have a cut‐off that isn’t based on the release an alias was added.
[...]
3. The removal script operates on the structured data and applies the process of “turn warnings into throws after one release”, “drop throws a release after they are added or become throws”.

Different ideas for these in #441492. Based on this PR, I would like to:

• Load the whole "latest drop" file in "warning or throw" mode, where renames are turned into warnings automatically.
• Load all other drop files in "only throw" mode, where renames are turned into throws, too.
• Only drop file-by-file.

Should make the whole script stuff considerably simpler - because we don't need any. Creating the next drop file and removing the old one can be part of the release process instead.

So how about, taking it a bit further: we have a mkAlias function to handle the structured data → alias conversion; if it’s a rename and config.allowAliases, it behaves as described above; otherwise, it produces a throw.

This will break CI's Eval during the attrpath generation step. During that step, aliases can't be present, otherwise they will break.

That was where I thought we might use a dummy derivation instead. One that will throw* when its outpath is being evaluated, but one that can return meta.available = false;, essentially.

* abort might be better to not be swallowed by the outpaths step.

This will break CI's Eval during the attrpath generation step. During that step, aliases can't be present, otherwise they will break.

Here is my proposal that I think works:

Structured data for aliases goes in __aliases at top level or in a scope; e.g.

__aliases = {
  hottest-program-of-1999 = mkBrokenAlias "25.11";
  abandoned-upstream = mkDeprecatedAlias "25.11";
  libpointless = mkUnusedAlias "25.11";
  unloved = mkUnmaintainedAlias "25.11";
  firefox_3 = mkInsecureAlias "27.11"; # lone remaining user was very passionate
  # These warn in 25.11, throw in 26.05 or if `!config.allowAliases`.
  renamedPackage = mkRenameAlias "25.11" "renamed-package";
  complicatedFull = mkAlias {
    since = "25.11";
    message = "use `complicated.override { … }` to set feature flags";
    # Without `package`, this would be an unconditional `throw`.
    package = complicated.override {
      withBells = true;
      withWhistles = true;
      withKitchenSink = true;
    };
  };
};

If we do the per‐version files, then they can get versions of these functions that pre‐apply the versions.

These functions don’t actually return things that throw or warn, just structured data representing the alias. When we tie together a scope to inject splicing and so on, we also populate it with the appropriate values corresponding to the contents of __aliases. Ideally, we include throws even if !config.allowAliases, and ensure that callPackage detects __aliases ? name && !config.allowAliases and give a useful error; the only issue I can imagine this having is perf. We’d probably want a lib.removeAliases someScope convenience function; in an ideal world, tests.cc-wrapper.supported would do something like lib.filter (lib.meta.availableOn stdenv.hostPlatform) (lib.removeAliases llvmPackageSets).

Then, we’d also ensure that any aliases with a since that are due for removal will fail CI – I guess we could simply include them even if config.allowAliases, or something? – and document dropping them in the release wiki.

How does that sound? I think this makes the common case convenient and easy, and prevents ugly config.allowAlias conditionals in package sets. It won’t work if you’re doing package sets in ways that also break splicing etc., but that seems like a reasonable nudge to have.

The other alternative, for package sets, would be to have a way to inject aliases into package sets in aliases.nix. That sounds kind of hard, though; we do it in nixos/rename.nix, but the module system is a lot more structured than our package sets. Maybe it can work if we assume everyone is doing the right thing with scopes along the way.

I do think that we should improve the !config.allowAliases UX by having the aliases throw/abort with their helpful message whenever they are used, directly or in callPackage, if possible. “Why is this package that totally exists not available?” is a not uncommon point of confusion.

Structured data for aliases goes in __aliases at top level or in a scope; e.g.

__aliases = {
  hottest-program-of-1999 = mkBrokenAlias "25.11";
  abandoned-upstream = mkDeprecatedAlias "25.11";
  libpointless = mkUnusedAlias "25.11";
  unloved = mkUnmaintainedAlias "25.11";
  firefox_3 = mkInsecureAlias "27.11"; # lone remaining user was very passionate
  # These warn in 25.11, throw in 26.05 or if `!config.allowAliases`.
  renamedPackage = mkRenameAlias "25.11" "renamed-package";
  complicatedFull = mkAlias {
    since = "25.11";
    message = "use `complicated.override { … }` to set feature flags";
    # Without `package`, this would be an unconditional `throw`.
    package = complicated.override {
      withBells = true;
      withWhistles = true;
      withKitchenSink = true;
    };
  };
};

I like this in principle. package might not be correct for all cases, we do have aliases for package sets, too. Quite often, because a lot of package sets are passthru's of other derivations. For example emacsPackages = emacs.pkgs etc.

These functions don’t actually return things that throw or warn, just structured data representing the alias. When we tie together a scope to inject splicing and so on, we also populate it with the appropriate values corresponding to the contents of __aliases. Ideally, we include throws even if !config.allowAliases, and ensure that callPackage detects __aliases ? name && !config.allowAliases and give a useful error; the only issue I can imagine this having is perf.

There is another practical problem: The callPackage stuff is in lib/, but at this stage, you don't have access to config.

Then, we’d also ensure that any aliases with a since that are due for removal will fail CI – I guess we could simply include them even if config.allowAliases, or something? – and document dropping them in the release wiki.

This part should work, yes.

That doesn't address the part that I meant with "this won't work for CI" though: I still read "when aliases are disabled, add throws". But that won't work for CI, at least for the foreseeable future.

Let's assume we do this:

__aliases = {
  hottest-program-of-1999 = mkBrokenAlias "25.11";
};

When config.allowAliases = true;, we will surely have a top-level attribute that has throw "removed because it was broken" as its value. That's what we currently do. I do understand your proposal to say that we also do this when allowAliases = false, though.

But this immediately breaks Eval, because it will hit this throw when going through all packages.

The only way for the first Eval step to succeed is:

• not have the attribute at all (that's currently the case, but gives us bad error messages exactly along the lines of "WTF, I know this package exists")
• have the attribute return null (probably even worse)
• have the attribute return a regular attrset without recurseForDerivations (also not helpful at all)
• have the attribute return a derivation that is meta.available = false - this will be filtered out by Eval, nix-env -qa etc. - yet it can still carry a throw in it, when something else than meta is evaluated.

This dummy derivation could probably have an internal attribute that carries the throw as well - and callPackage could just try to evaluate that attribute if present. This would keep all the logic about config.allowAliases in the function generating this dummy derivation from a structured alias.

The other alternative, for package sets, would be to have a way to inject aliases into package sets in aliases.nix. That sounds kind of hard, though; we do it in nixos/rename.nix, but the module system is a lot more structured than our package sets.

I thought about this, too, but rejected it because of complexity.

Maybe it can work if we assume everyone is doing the right thing with scopes along the way.

I think that's a stretch. But yes... we need to standardize all package sets to use the same interfaces and implementation.... big task :)

I do think that we should improve the !config.allowAliases UX by having the aliases throw/abort with their helpful message whenever they are used, directly or in callPackage, if possible. “Why is this package that totally exists not available?” is a not uncommon point of confusion.

Fully agree.

With dummy derivation, I mean something like this:

{
  type = "derivation";
  drvPath = throw "foo has been removed because it was broken";
  meta.available = false;
  # this is checked by callPackage
  _alias = true;
}

We might not even need to replicate the throw in _alias - a boolean should be enough. The error message from callPackage would then just be "You are using an alias here, but you shouldn't be!".

(far in the future, if something like NixOS/nix#8187 is ever implemented in Nix, we could throw the error on other attributes of that derivation, too - catching some cases of people trying to do pkgs.foo.somePassthru, for which they'd currently just get a "does not exist" error)

Edit: We'd also tack the _alias = true thing on rename-aliases, so that callPackage can also fail on these.

We might not even need to replicate the throw in _alias - a boolean should be enough. The error message from callPackage would then just be "You are using an alias here, but you shouldn't be!".

The whole _alias magic value doesn't work. It leads to infinite recursion, because callPackage needs to evaluate all arguments at least as far as knowing whether _alias even exists...

I like this in principle. package might not be correct for all cases, we do have aliases for package sets, too. Quite often, because a lot of package sets are passthru's of other derivations. For example emacsPackages = emacs.pkgs etc.

That’s tricky because lib.warnOnInstantiate won’t work in that case. But I guess we can do a gross thing to traverse down and add it wherever is relevant. (Using lib.warnOnInstantiate means you don’t get warnings whenever you specifically avoid instantiation, either, but that’s probably a better UX trade‐off than nix search spewing a bunch of them.)

There is another practical problem: The callPackage stuff is in lib/, but at this stage, you don't have access to config.

Eh – we’re already relying on the scoping/splicing machinery to make __aliases real and determine whether to add a warning or a throw, so it can just inject __enableAliases or something else gross like that. Doesn’t seem like a huge problem to me.

That doesn't address the part that I meant with "this won't work for CI" though: I still read "when aliases are disabled, add throws". But that won't work for CI, at least for the foreseeable future.

Let's assume we do this:

__aliases = {
  hottest-program-of-1999 = mkBrokenAlias "25.11";
};

When config.allowAliases = true;, we will surely have a top-level attribute that has throw "removed because it was broken" as its value. That's what we currently do. I do understand your proposal to say that we also do this when allowAliases = false, though.

But this immediately breaks Eval, because it will hit this throw when going through all packages.

The only way for the first Eval step to succeed is:

• not have the attribute at all (that's currently the case, but gives us bad error messages exactly along the lines of "WTF, I know this package exists")
• have the attribute return null (probably even worse)
• have the attribute return a regular attrset without recurseForDerivations (also not helpful at all)
• have the attribute return a derivation that is meta.available = false - this will be filtered out by Eval, nix-env -qa etc. - yet it can still carry a throw in it, when something else than meta is evaluated.

This dummy derivation could probably have an internal attribute that carries the throw as well - and callPackage could just try to evaluate that attribute if present. This would keep all the logic about config.allowAliases in the function generating this dummy derivation from a structured alias.

It seems easy for it to ignore these throws when __aliases ? ${path}. Then the “obsolete aliases that need removing” check can just make them aborts instead, or eval could check __aliases.${path}.since, or…

I am not sure how Hydra would feel about this, however; it’s possible we’d need to be smarter for that, or else just have a noReallyDisallowAliases that does the worse thing (but I don’t prefer this).

I don’t love the meta.available thing, because it means lib.meta.availableOn stdenv.hostPlatform renamedPackage isn’t caught on Hydra and isn’t warned about in user configs until it becomes a throw. (Although lib.warnOnInstantiate already causes that problem :( )

I don’t love the meta.available thing, because it means lib.meta.availableOn stdenv.hostPlatform renamedPackage isn’t caught on Hydra and isn’t warned about in user configs until it becomes a throw.

Shouldn't this be caught where renamedPackage is passed into the expression via callPackage already?

For the cases where it's not: With the dummy-derivation approach, lib.meta.availableOn could be another candidate to just check <2nd-arg>._alias and fail if it's true.

Eh – we’re already relying on the scoping/splicing machinery to make __aliases real

With my approach of putting the aliases into by-name, the _aliases thing would be created from filtering the scope of _alias = true. I wasn't able to make that work, yet, so not sure whether I'd hit more infinite recursion...

So maybe we'd need a combination of _aliases = { } on the scope and ._alias = bool on each alias to handle all cases.