-
-
Notifications
You must be signed in to change notification settings - Fork 77
treewide: general maintenance work #264
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
base: master
Are you sure you want to change the base?
Conversation
301a51d to
f296278
Compare
Kirottu
left a comment
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.
Seems to be a whole lot of useful stuff, the HM module should be addressed but aside from that it should be good to go.
Signed-off-by: NotAShelf <[email protected]> Change-Id: I6a6a6964e304a7ecf0a8b0516643e980a827224e ci: use cachix Nix installer Signed-off-by: NotAShelf <[email protected]> Change-Id: I6a6a6964db05f91668ef5c2d7f953f107084cf4b
Signed-off-by: NotAShelf <[email protected]> Change-Id: I6a6a6964e259ee1a05f31319fdfd90a1c163404f
Signed-off-by: NotAShelf <[email protected]> Change-Id: I6a6a6964a7d3d412c221bdf412bc241cad70610c
Signed-off-by: NotAShelf <[email protected]> Change-Id: I6a6a69643aa993276a3cf38558001100ccefda1b
Signed-off-by: NotAShelf <[email protected]> Change-Id: I6a6a69640bd938da0cb79a6e26310d121e72a53e
This is no doubt a hacky solution, but it aims to catch an edge case. It should be unlikely that the user is using the Anyrun module from the flake and the Anyrun package from Nixpkgs. Nevertheless, we correctly inform the user that what they're doing is not supported. If the user insists on using `pkgs.anyrun`, then they certainly have Home Manager available and thus they may avoid using the flake altogether. Signed-off-by: NotAShelf <[email protected]> Change-Id: Iefaf6915551aad04ea08e14bce16d0fa6a6a6964
f296278 to
f76807e
Compare
|
@Kirottu I believe this is now done. I would appreciate a second review just in case, because rebasing with Jujutsu is my worst nightmare. I've elected to expose the anyrun-provider under anyrun's passthru, and treat a lack of anyrun-provider as an edge case in the Home Manager module. If it is missing, then the user is attempting an unconventional setup that we are not responsible with handling--and the assertion clearly communicates what the user can do. Testing would be appreciated, as I no longer use Home Manager. |
Kirottu
left a comment
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.
I couldn't actually test the changes since this branch fails to compile now, reproduced both by CI and me (both through a Nix build and through a local build). Seems like you were right to regard a Jujutsu rebase a nightmare.
Regardless, there were a few other things I noticed from a cursory glance.
| defaultPackage = self.packages.${pkgs.stdenv.hostPlatform.system}.default; | ||
| defaultProvider = self.packages.${pkgs.stdenv.hostPlatform.system}.anyrun-provider; | ||
| defaultPackage = self.packages.${pkgs.stdenv.hostplatform.system}.default; | ||
| defaultProvider = defaultProvider.passthru.anyrun-provider or null; |
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.
This looks like a typo.
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.
Which part, passthru?
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.
defaultProvider = defaultProvider.<the rest>
| (assertNumeric cfg.config.y) | ||
|
|
||
| { | ||
| assertion = cfg.package.anyrun-provider != null; |
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.
Wouldn't this trigger every time a package does have a valid passthru instead of only when it doesn't? Also, since you can still configure the provider through the module (programs.anyrun.config.provider) shouldn't that case disable this assertion since it is a valid way of configuring the provider?
|
I'll finish this tomorrow hopefully. Been having network issues that made rebasing and rebuilding pain, shouldn't be too hard now that my network is back. |
Relatively small PR that introduces several improvements to Nix packaging, CI workflows, and configuration files. The main themes I've targeted today are to update the CI workflows for better reliability (i.e., remove a hostile workflow that insists on installing its own enterprise crap) in favor of a fully free and reliable one as well as reducing redundancies in the Nix packaging.
We now add anyrun-provider (re-exported in
packages) to Anyrun's PATH, and use a simple.overrideforanyrun-with-all-pluginspackage. This centralizes some packaging and installation steps.I've also went ahead and updated the formatter package for
nix fmt. The previously validnix fmt .is now discouraged, and is suggested to be replaced with wrappers like treefmt. I find this to be obsolte, however, as we can handle this much faster in-house without relying on a 3rd party wrapper.Lastly I've formatted TOML files with Taplo, which is a simple and reliable formatter. We can also set up rustfmt with a configuration file if desirable.
Signed-off-by: NotAShelf [email protected]
Change-Id: I6a6a6964e304a7ecf0a8b0516643e980a827224e