-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
nyxt: 2.2.4 -> 3.1.0 #235095
nyxt: 2.2.4 -> 3.1.0 #235095
Conversation
Result of 46 packages failed to build:
374 packages built:
|
I also got it to work by registering systems as immutable, I picked this up from upstream: --- a/pkgs/development/lisp-modules/packages.nix
+++ b/pkgs/development/lisp-modules/packages.nix
@@ -430,13 +430,10 @@ let
pkgs.gnome.adwaita-icon-theme
];
- postConfigure = ''
- export CL_SOURCE_REGISTRY=$CL_SOURCE_REGISTRY:$(pwd)//
- '';
-
buildScript = pkgs.writeText "build-nyxt.lisp" ''
(load "${super.nyxt.asdfFasl}/asdf.${super.nyxt.faslExt}")
(asdf:load-system :nyxt/gi-gtk-application)
+ (map () 'asdf:register-immutable-system (asdf:already-loaded-systems))
(sb-ext:save-lisp-and-die "nyxt" :executable t
#+sb-core-compression :compression
#+sb-core-compression t
I don't know which approach is better though, appending to CL_SOURCE_REGISTRY or registering systems as immutable. Probably we should use whichever and we'll see if issues come up. |
Hmm right, so with the flag globally the dependencies would be "shared" between all lisp packages and nyxt. I would vote for increasing the heap globally, in |
Nice catch! I think using
That's how I understood it working, so upping the heap globally would make sense to stop build duplication. |
Does this globally pin quri version in sbcl.pkgs even after we update from quicklisp? |
Yes |
Maybe we should give an attribute name with version included then? |
Since Nyxt uses git submodules to manage their dependencies I went and updated all dependencies that were outdated in quicklisp to the versions pinned by Nyxt, some dependencies may still work if we use their quicklisp version, but to be safe I updated all of the ones that were outdated.
I'm not sure, since I'm assuming that using newer version of the dependencies should be fine, (but I'm a bit worried about using older versions) so as long as the quicklisp version is at or above what Nyxt uses a pin shouldn't be necessary, so a pin would only be useful as long as it's outdated on quicklisp. |
To how much should it be increased? |
Could we use submodules instead of the dependencies from quicklisp or nix packaged ones? Or, we keep two version of nyxt. One is from quicklisp, and the other is manual packaged. |
Really good work, @dariof4, it is truly appreciated. I apologise for being absent on both maintenance front and this PR, but life has been occupying. Either ways, I've not been using Nyxt for a while now, and don't consider myself qualified enough to maintain nor review. If possible could you please also remove me from the package maintainers? (old name payas, as is present in maintainers, user id is same), TIA! I will make sure to give the new Nyxt a spin once it reached unstable, quite excited for changes, but my current machine had quite a few troubles in the past so had to stop. |
Silly question: how much RAM to load both Nyxt and magicl? (I know the compiler is hungry and so the question is whether it is «max» or «sum» in practice). |
Using submodules instead of quicklisp dependecies is possible, although that would cause some extra building due to not sharing Nyxt's dependecies with other lisp packages.
Okay, I'll remove you from the Nyxt maintainers. Thanks for your previous work maintaining it!
I'm not exactly sure how to test that since trying to build Nyxt and magicl simultaneously ( |
I was just playing around with this PR, and everything runs great, but I realised that I get a lot of warnings in the console about portals:
And noticed that the portal location is pointing to Is this an issue on my side that I don't have the right desktop portal installed, or should this point to a store path? Thanks for all the great work! |
I unfortunately can't reproduce :(, are you using wayland or x11? And what gpu drivers are you using? (I'm using X11 with the proprietary nvidia drivers on a laptop)
I updated the branch with the fix for copy/pasting, and I set the |
This comment was marked as off-topic.
This comment was marked as off-topic.
I think maintainer GitHub id needs to be in the same representation as the other ids in the file |
Set the `--dynamic-spzce-size` flag for sbcl to add more heap size to make certain packages be able to compile. ref: NixOS#235095 (comment)
Oops, that was a dumb mistake 😅, I fixed it. |
I still think that if we have quicklisp mass-import, then version overrides are better with versions in the attribute names. (I am not 100% sure what is the proper way to handle Nyxt so that «you can't load Nyxt and magicl at once at all» goes away, but that question is out of scope…) |
Adds various updated lisp packages that are needed to update nyxt to 3.1.0.
Promter was an internal part of nyxt that was split off as it's own external library in nyxt 3.0.0.
Package :nyxt/gi-gtk-application instead of :nyxt/gtk-application. ref: NixOS#217888 (comment)
`WEBKIT_FORCE_SANDBOX=0` is no longer needed since web extension support has been diabled by default. ref: atlas-engineer/nyxt#2897 Release notes: https://nyxt.atlas.engineer/article/release-3.0.0.org https://github.com/atlas-engineer/nyxt/releases/tag/3.1.0
I went ahead and switched the pr to naming the attributes rather than shadowing them, this required explicitly listing all the |
Description of changes
Update Nyxt from version 2.2.4 to version 3.1.0.
Release notes since 2.2.4:
https://nyxt.atlas.engineer/article/release-3.0.0.org
https://nyxt.atlas.engineer/article/release-3.1.0.org
Fixes: #217888
This also updates the lisp dependencies needed to build Nyxt 3.1.0 and adds
prompter
which was transferred from Nyxt to an external library not yet on quicklisp, it also changes the build fromnyxt/gtk-application
tonyxt/gi-gtk-application
since that seems to be the one to package according to Nyxt devs (the makefile and Guix packagegi-gtk
).I have a few concerns though:
Each time Nyxt is launched it for some reason tries to recompile
nasdf
(which it fails since it can't create/homeless-shelter
) and crashes with ansystem nyxt out of date
error, I found several ways to "fix" this, either settingCL_SOURCE_REGISTRY=$CL_SOURCE_REGISTRY:$(pwd)//
inpostConfigure
which makes it stop trying to recompile itself, using(asdf:disable-output-translations)
likestumpwm
does it, which makes it still try to recompilenasdf
and fails, but makes Nyxt run fine, or settingHOME=/tmp
which allows Nyxt to successfully recompilenasdf
in/tmp/.cache
(Guix setsHOME=/tmp
for Nyxt as well, may be related?).I'm a little bit out of my depth here and have no clue why Nyxt would be trying to recompile
nasdf
, it seems to be something to do withCL_SOURCE_REGISTRY
, but I don't have enough experience with CL to understand this at all 😅I would appreciate if someone more experienced could shed some light as to what is the best solution.
Should this PR be split up into multiple? and if so in how many should it be split? I tried to keep the commits be logically separate so this should be easy.
I think using
wrapLisp
within Nyxt'sdefault.nix
(needed to compile Nyxt with the--dynamic-memory-size
flag to increase the heap size) causes quite a few extra builds, since the way I understand it can't use the cache for all the lisp dendencies of Nyxt, since it useswrapLisp
, it might be better to apply--dynamic-memory-size
globally tosbcl
?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/
)