-
-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
emacs: Build emacs packages on Hydra #188110
Conversation
In the past the motivation to not recurse into Emacs packages was that it added quite a lot of packages to the evalution and they were so fast to build locally that substituting them from a binary cache didn't make sense. With native compilation this equation has changed drastically, build times are much longer and build closures are larger so the utility of having cached packages has gone way up. Additionally, it looks to me like Emacs is the only ecosystem in nixpkgs to ever care about evaluation performance like this. Every other extensible editor ecosystem has recurseIntoAttrs set to true on their respective package sets.
This alias should live in aliases.nix but that would cause Hydra not to evaluate/build the packages. To be honest I don't particularly like this change, but it seems the most practical way forward to get Hydra to build Emacs packages.
Since emacs packages in nixpkgs are going to have binary cache[1], it is useful to split the whole overlay into two seperated ones so that users can use emacs packages in nixpkgs with emacsGit or emacsWithPackagesFromUsePackage in this overlay. [1]: NixOS/nixpkgs#188110
Since emacs packages in nixpkgs are going to have binary cache[1], it is useful to split the whole overlay into two seperated ones so that users can use emacs packages in nixpkgs with emacsWithPackagesFromUsePackage or emacsWithPackagesFromPackageRequires in this overlay. [1]: NixOS/nixpkgs#188110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if these changes are "correct", but they look good to me.
Edit the numbers. Could we also build Sure, this will double the number of emacs packages needing to be built, and the total will be about 11k. That number is about half of number of haskell packages in nixpkgs, which is 17k. I hear from a friend that on riscv haskell packages take about 2/3 of the totoal time when the whole nixpkgs is rebuilt. Assuming the build time of emacs packages is much shorter than that of haskell ones, adding Admittedly, current system of emacs packages in nixpkgs should be improved to prepare for new variants, e.g. What do you think? |
Since emacs packages in nixpkgs are going to have binary cache[1], it is useful to split the whole overlay into two seperated ones so that users can use emacs packages in nixpkgs with emacsWithPackagesFromUsePackage or emacsWithPackagesFromPackageRequires in this overlay. [1]: NixOS/nixpkgs#188110
Result of 7 packages marked as broken and skipped:
6 packages failed to build:
5530 packages built:
|
These types of comments are not helpful at all and only serve to add noise. |
I'm sorry I'm running automated builds on PRs on my rig but the portion of PRs where this appears to be annoying is higher than I thought so I'll quit doing it for now. |
It's OK, I understand you're just trying to help out :) In this particular case we've been running builds for years over at https://hydra.nix-community.org/project/emacs-overlay so we're well aware of roughly what's broken and why. This PR is mostly a policy change and less about figuring out which Emacs packages are broken. |
Uh, over 5k new jobs per platform. That's quite noticeable: https://hydra.nixos.org/eval/1777820#tabs-new |
It's not like any other package sets seem to care, I don't see why Emacs is any different other than historical reasons. |
I should make it clear that I didn't mean to imply that this PR is "wrong" (or correct even). Just that it's most likely a noticeable count of jobs for the farm, future mass rebuilds, etc. |
Right, that's something I was well aware of already, and historically has been the reason why we didn't eval them, but as I stated in the PR body, I think the reasoning has shifted with the much increased build times. And sorry for my somewhat grumpy tone, I should stop responding to Github for today... |
Emacs packages in nixpkgs have binary cache since [1]. It is useful to split the whole overlay into two seperated ones so that users can use emacs packages in nixpkgs with emacsWithPackagesFromUsePackage or emacsWithPackagesFromPackageRequires in this overlay. [1]: NixOS/nixpkgs#188110
Emacs packages in nixpkgs have binary cache since [1]. It is useful to split the whole overlay into two seperated ones so that users can use emacs packages in nixpkgs with emacsWithPackagesFromUsePackage or emacsWithPackagesFromPackageRequires in this overlay. [1]: NixOS/nixpkgs#188110
As of NixOS#188110, emacsPackages isn't an alias anymore and is automatically recursed into.
As of #188110, emacsPackages isn't an alias anymore and is automatically recursed into.
nodePackages is not recursed into because of performance reasons and all other editors do not have 5000+ packages otherwise the same would apply to them. |
Description of changes
In the past the motivation to not recurse into Emacs packages was that it added quite a lot of packages to the evalution and they were so fast to build locally that substituting them from a binary cache didn't make sense.
With native compilation this equation has changed drastically, build times are much longer and build closures are larger so the utility of having cached packages has gone way up.
Additionally, it looks to me like Emacs is the only ecosystem in nixpkgs to ever care about evaluation performance like this.
Every other extensible editor ecosystem has recurseIntoAttrs set to true on their respective package sets.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes