firefox: support Glide derivative#2240
Conversation
There was a problem hiding this comment.
CI is currently failing because lib.maintainers.zzzealed and glide-browser are undefined. If these are recent upstream additions, this PR requires #2180 to be merged first.
- Each commit in this PR is suitable for backport to the current stable branch
Are the necessary upstream patches available in our release-25.11 branch?
I hope I added everything correctly, as this is my first time making a contribution.
LGTM, except for one nitpick. I will also test this PR in a testbed once CI is working again.
Yeah so I removed myself as maintainer since I'm not in maintainer-list.nix. I hope this is fine.
Yes it look's to me like |
|
CI failures seem to be unrelated to this PR's changes, could you force push to trigger a re-run? also ideally a testbed would be added before merging. |
b8adbe5 to
bd2fac8
Compare
Sure I can do that. As I said Glide isn't in Nixpkgs but I could just add Glide's input in Does this approach make sense? |
trueNAHO
left a comment
There was a problem hiding this comment.
I removed myself as maintainer since I'm not in maintainer-list.nix. I hope this is fine.
It would be highly appreciated to add yourself as a maintainer so you are notified about potential Glide issues. The process is already documented:
Lines 225 to 251 in 8ed48a4
Otherwise, inspect commits touching /stylix/maintainers.nix for further inspiration.
I don't know if Stylix has any policy on non-nixpkgs (yet) projects, like Glide. Please let me know if that's the case.
[...]
Glide isn't in Nixpkgs but I could just add Glide's input in
flake.nix
Yes, this is how we handle external module integration, except that we do not publicly expose these test inputs by adding them to the internal /flake/dev/flake.nix. See commit aa0272b ("dank-material-shell: init (#2177)") as an example.
and copy the
modules/firefox/testbeds/firefox.nix. Then lastly import and modifymodules/firefox/testbeds/firefox-color.nixandmodules/firefox/testbeds/firefox-gnome-theme.nix.
Yes, adding a testbed for this external module would be highly appreciated. I suppose adding a single /modules/firefox/testbeds/glide.nix testbed suffices.
675e0f7 to
63acab1
Compare
63acab1 to
7c15c5b
Compare
7c15c5b to
ea4ac32
Compare
|
Hello :) I also tried cleaning up the commits a bit. Let me know if there are more changes needed. |
There was a problem hiding this comment.
Try resolving the CI failures and verify the theme working inside the nix run .#testbed:glide:{dark,light} testbeds.
There was a problem hiding this comment.
nix run .#testbed:glide-browser:light and nix run .#testbed:glide-browser:dark looks correct now.
There was a problem hiding this comment.
Try resolving the CI failures
Maybe the following resolves the aarch64-linux failures:
diff --git a/modules/firefox/each-config.nix b/modules/firefox/each-config.nix
index 846eb20d..11fb227a 100644
--- a/modules/firefox/each-config.nix
+++ b/modules/firefox/each-config.nix
@@ -9,6 +9,7 @@
lib,
pkgs,
config,
+ options,
...
}:
mkTarget {
@@ -26,7 +27,7 @@ mkTarget {
firefoxGnomeTheme.enable = lib.mkEnableOption "[Firefox GNOME theme](https://github.com/rafaelmardojai/firefox-gnome-theme) on ${humanName}";
};
- config = [
+ config = lib.optionals (name == "glide-browser" -> options.programs ? ${name}) [
(
{ cfg }:
{882d2e9 to
e53dbc5
Compare
Co-authored-by: Noah Biewesch <dev@noahbiewesch.com>
Hello and thanks for your work on Stylix :)
I added Glide to the Firefox (and derivatives)-module, since Glide now has a Home Manager-module made with
mkFirefoxModule. This means it should be compatible with the same options as the other derivatives.I tested this myself and can confirm that the options looks to be working.
I hope I added everything correctly, as this is my first time making a contribution. Please do not hesitate to leave feedback or request changes.