Various eval fixes with `allowAliases = false` by infinisil · Pull Request #276112 · NixOS/nixpkgs

infinisil · 2023-12-22T19:14:59Z

Description of changes

This fixes evaluation of various attributes with allowAliases = false, sometimes by removing them if they shouldn't exist at all with allowAliases = false:

chia
chia-dev-tools
chia-plotter
dotnet-sdk_2
dotnet-sdk_3
dotnet-sdk_5
gofumpt
gruvbox-plus-icons
mod_wsgi
nextcloud25
nextcloud25Packages
nix-repl
teams
wtk

Things done

Tested evaluation of these attributes with nix-instantiate --eval --arg config '{ allowAliases = false; }' -A <attr>. They either don't exist or succeed

Add a 👍 reaction to pull requests you find important.

Unsupported platforms are handled with `meta.platforms`, there's no need to have an extra `throw` for this. This makes the top-level attribute `teams` attribute evaluate

No idea how this passes ofborg eval..

This makes top-level attribute evaluate

…ases disabled

eureka-cpu

It seems there are some changes here that can be viewed as unrelated, ie the changes to the icon package. If it's not too much trouble, can you group the commits separately and add closing keywords for corresponding issues?

I'm fairly new to the Nix language btw, so feel free to ignore my review if you don't find it helpful 🙂

eureka-cpu · 2023-12-22T21:20:35Z

pkgs/applications/networking/instant-messengers/teams/default.nix

-in
-if stdenv.isDarwin
-then darwin
-else throw "Teams app for Linux has been removed as it is unmaintained by upstream. (2023-09-29)"


Is it unreasonable to still include this error message for Linux users?

There's no good infrastructure to have that easily, and I think the default error for unsupported platforms is good enough:

error: Package ‘teams-1.6.00.4464’ in /home/tweagysil/src/nixpkgs/pr/pkgs/applications/networking/instant-messengers/teams/default.nix:18 is not available on the requested hostPlatform: hostPlatform.config = "x86_64-unknown-linux-gnu" package.meta.platforms = [ "x86_64-darwin" "aarch64-darwin" ] package.meta.badPlatforms = [ ] , refusing to evaluate.

eureka-cpu · 2023-12-22T21:22:55Z

pkgs/servers/nextcloud/default.nix

-  nextcloud25 = throw ''
-    Nextcloud v25 has been removed from `nixpkgs` as the support for is dropped
-    by upstream in 2023-10. Please upgrade to at least Nextcloud v26 by declaring
-
-        services.nextcloud.package = pkgs.nextcloud26;
-
-    in your NixOS config.
-
-    WARNING: if you were on Nextcloud 24 you have to upgrade to Nextcloud 25
-    first on 23.05 because Nextcloud doesn't support upgrades across multiple major versions!
-  '';
-


This is still fairly recent. Can we not remove this?

Not sure what you mean. I just moved this part around while keeping the same error message, since I know close to nothing about all of these packages 😆

Ah just saw #276112 (comment) haha

eureka-cpu · 2023-12-22T21:23:31Z

pkgs/by-name/gr/gruvbox-plus-icons/package.nix

  nativeBuildInputs = [ gtk3 ];

-  propagatedBuildInputs = [ breeze-icons gnome-icon-theme hicolor-icon-theme ];
+  propagatedBuildInputs = [ plasma5Packages.breeze-icons gnome-icon-theme hicolor-icon-theme ];


Are these changes necessary? They seem to bringing more than just the icons into scope for an icon package.

Yeah this is necessary because on master, this fails:

$ nix-instantiate -A gruvbox-plus-icons --arg config '{ allowAliases = false; }' error: … while calling the 'throw' builtin at /home/tweagysil/src/nixpkgs/pr/lib/customisation.nix:206:13: 205| then makeOverridable f allArgs 206| else throw "lib.customisation.callPackageWith: ${error}"; | ^ 207| error: lib.customisation.callPackageWith: Function called without required argument "breeze-icons" at /home/tweagysil/src/nixpkgs/pr/pkgs/by-name/gr/gruvbox-plus-icons/package.nix:6

This is because breeze-icons is an alias. We can look into aliases.nix to see where it comes from:

nixpkgs/pkgs/top-level/aliases.nix

Lines 1087 to 1089 in 989df07

inherit (plasma5Packages)

akonadi akregator arianna ark bluedevil bomber bovo breeze-grub breeze-gtk

breeze-icons breeze-plymouth breeze-qt5 colord-kde discover dolphin dragon elisa falkon

So the change here just inlines the alias, therefore not using the alias anymore

eureka-cpu · 2023-12-22T21:24:11Z

pkgs/top-level/aliases.nix

+  nextcloud25 = throw ''
+    Nextcloud v25 has been removed from `nixpkgs` as the support for is dropped
+    by upstream in 2023-10. Please upgrade to at least Nextcloud v26 by declaring
+
+        services.nextcloud.package = pkgs.nextcloud26;
+
+    in your NixOS config.
+
+    WARNING: if you were on Nextcloud 24 you have to upgrade to Nextcloud 25
+    first on 23.05 because Nextcloud doesn't support upgrades across multiple major versions!
+  ''; # Added 2023-10-13


Disregard my previous comment about the message being removed.

katexochen

Changes lgtm, thanks. :)
I guess it's to expensive to check this on every PR?

infinisil · 2023-12-22T21:57:22Z

I guess it's to expensive to check this on every PR?

It's actually very cheap, under 10 seconds on my machine! Here's how to see all failing attributes on x86_64-linux:

❯ nix repl lib
warning: future versions of Nix will require using `--file` to load a file
Welcome to Nix 2.18.1. Type :? for help.

Loading installable ''...
Added 422 variables.
nix-repl> attrNames (filterAttrs (name: value: ! (builtins.tryEval value).success) (import ./. { config.allowAliases = false; system = "x86_64-linux"; }))
[ "AAAAAASomeThingsFailToEvaluate" "androidsdk_9_0" "gams" "gccWithoutTargetLibc" "libcCross" "mathematica10" "mathematica11" "mathematica9" "neoload" "pkgsx86_64Darwin" "sc2-headless" "yabai" ]

There's not a lot left, and a bunch will also be fixed with #276120.

However, having taken a look at the rest of these, there are some kind of tricky cases which I'm not sure yet how to handle. Furthermore, the list is much longer if you use e.g. x86_64-darwin instead, or even some obscure platform. Then the question is whether this should be checked for all platforms somehow. And then there's also cross compilation, which probably has a ton more failures.

So overall, this PR is just a small step in the bigger picture :)

katexochen · 2023-12-22T22:33:20Z

Thanks for the nice explanations (also on the review comments) @infinisil. It's great to get such background, I learned a lot reviewing this PR! 😊

RGBCube · 2023-12-24T09:21:15Z

LGTM as well

jtojnar · 2024-01-04T18:33:28Z

pkgs/top-level/all-packages.nix

  mkchromecast = libsForQt5.callPackage ../applications/networking/mkchromecast { };

-  # Backwards compatibility.
-  mod_dnssd = apacheHttpdPackages.mod_dnssd;


This broke building GNOME with aliases disabled, fixed in #278744

Ofborg didn't catch it when calculating outpaths because of #271123 (comment) as well, by the way (I've been investigating this)

I'm working on a fix

infinisil added 7 commits December 22, 2023 20:04

teams: Don't use root-level throw for unsupported platforms

7d926d1

Unsupported platforms are handled with `meta.platforms`, there's no need to have an extra `throw` for this. This makes the top-level attribute `teams` attribute evaluate

gruvbox-plus-icons: Fix evaluation when allowAliases disabled

7f6b897

No idea how this passes ofborg eval..

dotnet-sdk: Don't add attributes when allowAliases disabled

12e32d6

wtk: Use meta.platforms to check for supported platform

8814211

This makes top-level attribute evaluate

gofumpt: Fix evaluation when allowAliases disabled

38ad4fe

nextcloud25: Disable attributes when allowAliases disabled

1470c52

chia: Disable attributes if allowAliases disabled

8bbbb78

infinisil requested a review from IvarWithoutBones as a code owner December 22, 2023 19:14

infinisil added 2 commits December 22, 2023 20:47

apacheHttpdPackages: Disable renamed/removed attributes when allowAli…

feb114f

…ases disabled

nix-repl: Disable attribute when allowAliases disabled

14b1188

infinisil force-pushed the some-eval-fixes branch from ec71200 to 14b1188 Compare December 22, 2023 19:47

ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package labels Dec 22, 2023

ofborg bot requested review from RGBCube, eureka-cpu, katexochen, liff, rvolosatovs and tricktron December 22, 2023 20:24

ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 22, 2023

eureka-cpu reviewed Dec 22, 2023

View reviewed changes

katexochen approved these changes Dec 22, 2023

View reviewed changes

eureka-cpu approved these changes Dec 22, 2023

View reviewed changes

delroth added 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Dec 22, 2023

liff approved these changes Dec 28, 2023

View reviewed changes

delroth added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 2 This PR was reviewed and approved by two persons. labels Dec 28, 2023

infinisil merged commit c98d1bd into NixOS:master Dec 31, 2023

infinisil deleted the some-eval-fixes branch December 31, 2023 21:07

jtojnar reviewed Jan 4, 2024

View reviewed changes

This was referenced Jan 4, 2024

gnome.gnome-user-share: Fix eval with allowAliases = false #278744

Merged

lib.callPackageWith: Use abort again instead of throw and fix evaluation errors caused by it #278777

Merged

	inherit (plasma5Packages)
	akonadi akregator arianna ark bluedevil bomber bovo breeze-grub breeze-gtk
	breeze-icons breeze-plymouth breeze-qt5 colord-kde discover dolphin dragon elisa falkon

Uh oh!

Conversation

infinisil commented Dec 22, 2023

Description of changes

Things done

Uh oh!

eureka-cpu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infinisil Dec 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

katexochen left a comment

Choose a reason for hiding this comment

Uh oh!

infinisil commented Dec 22, 2023

Uh oh!

katexochen commented Dec 22, 2023

Uh oh!

RGBCube commented Dec 24, 2023

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

eureka-cpu left a comment •

edited

Loading

infinisil Dec 22, 2023 •

edited

Loading