-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
ghostty: allow darwin users to manager their config #6300
base: master
Are you sure you want to change the base?
Conversation
Seems like a duplicate of #6298? |
This is more complete than my PR in #6298. But i'd prefer to make things conditionally if the hostPlaform is darwin. |
I think |
60ccfa9
to
f61c272
Compare
Currently nixpkgs does not contain a package defintion for ghostty compatible with Darwin. Darwin users may still want to use this module to manage their config or share config between systems. This carries over behaviour from the beta period where this same technique was used. see: https://github.com/clo4/ghostty-hm-module/blob/887e13a6e7acf5ffaab0119d96e476d84db90904/module.nix#L167-L173 Also improves validation to cover theme files.
f61c272
to
cbb6ba5
Compare
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.
Here is how I think this should be handled:
modules/programs/ghostty.nix
Outdated
package = lib.mkPackageOption pkgs "ghostty" { | ||
nullable = true; | ||
extraDescription = | ||
"On darwin set this to null to manage ghostty config while ghostty is not present in nixpkgs."; | ||
}; |
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.
Just to capitalize ghostty in the description
package = lib.mkPackageOption pkgs "ghostty" { | |
nullable = true; | |
extraDescription = | |
"On darwin set this to null to manage ghostty config while ghostty is not present in nixpkgs."; | |
}; | |
package = lib.mkPackageOption pkgs "Ghostty" { default = [ "ghostty" ]; }; |
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.
That doesn't appear to work when nixpkgs is marking the package as broken. Setting the package to null in config isn't allowed (not nullable) and nixpkgs evaluates the default since it is present. This does work when the package isn't present at all.
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.
It won't be present look at: #6300 (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.
Testing.
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.
Yup, my mistake on where the actual eval was coming from. "Ghostty" isn't needed if we're guarding all the package sites. mkPackageOption
currently only checks for presence not whether or not the package is broken.
I considered checking if the package is broken instead and looked at ~hostPlatform. I think Linux is the only supported output from hostPlatform
so that doesn't seem to buy much generality.
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.
Turns out this trying to avoid installing only on linux brings in different complications. When we just gate on isLinux
we add a synchronization point with nixpkgs. If nixpkgs gains a darwin version of ghostty (as some are working on) this module will have to change at the ~same as it's introduced into nixpkgs. Addtionally, to a lesser degree, it also interferes with users ability to set their own package if they figure out some work around for themselves. If, say, a *bsd or darwin user gets a package working for themselves before it lands in nixpkgs they would not be able to use the module as is.
There's a bit of weirdness around a transition from broken -> working for macos users too. If a nixpkgs copy exists some users will want that copy and some will want the macos managed version instead. (technically this could also be the case for nix on Arch)
To me this suggests setting nullable: true
and handling the null case. Trying to default to the 'right' value for darwin users runs into that point above around choosing darwin or nix hosted packages. My user facing suggestion for that is adding an assertion: While it's possible ghostty darwin becomes a package there isn't an easy option to avoid setting the package to null, as far as I can see.(!lib.meta.availableOn stdenv.hostPlatform cfg.package) -> (cfg.package == null)
with the message "programs.ghostty.package is marked as broken on your platform, set the value to null to manage your configuration without installing the package"
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.
Handling null for package
is exactly the path I have chosen for my private Ghostty module. I support that route.
ghostty is currently marked as broken in nixpkgs. Darwin users still want to manage ghostty config via home-manager. Avoiding installing the package and any extra files that depend on the package outside Linux allows this.
891c29d
to
901423c
Compare
901423c
to
6be244f
Compare
Perhaps allow setting |
Should close #6295 (maybe edit the description with this so it auto closes my issue when this is merged 😃) Thanks for this! |
Description
Currently nixpkgs does not contain a package definition for ghostty compatible with Darwin. Darwin users may still want to use this module to manage their config or share config between systems. This carries over behaviour from the beta period where this same technique was used.
see: https://github.com/clo4/ghostty-hm-module/blob/887e13a6e7acf5ffaab0119d96e476d84db90904/module.nix#L167-L173
Also improves validation to cover theme files.
Requested via ghostty discord. Testing included building against a local copy of nixpkgs with the ghostty package deleted and marked as broken.
Checklist
Change is backwards compatible.
Code formatted with
./format
.Code tested through
nix-shell --pure tests -A run.all
ornix develop --ignore-environment .#all
using Flakes.Test cases updated/added. See example.
Commit messages are formatted correctly
Maintainer CC
cc @HeitorAugustoLN
closes #6295