nix-darwin icon indicating copy to clipboard operation
nix-darwin copied to clipboard

fonts: reimplement and rename to `fonts.packages`

Open emilazy opened this issue 1 year ago • 15 comments

This uses /Library/Fonts/Nix Fonts as suggested by @lilyball, and copies fonts according to their full store path to avoid having to do a slow checksum when you have large fonts (some CJK font packages can be in the hundreds of mebibytes). If adding a few seconds to every activation is considered acceptable, then a simpler flat directory structure could be used instead.

An error is thrown for existing users of fonts.fonts that explains the need to manually clean up existing font files.

I'm not completely confident about the dropping of fonts.fontDir.enable, but my reading of NixOS's fontconfig code and my old laptop configuration suggest that fonts.packages should work even without setting that.

cc @Enzime

emilazy avatar Aug 03 '23 03:08 emilazy

Another thing with this approach is that there will no longer be a defined order for identically-named font files to override each other. Not sure if that's a problem.

emilazy avatar Aug 03 '23 03:08 emilazy

Please keep fonts.fontDir.enable. I want the ability to insert fonts into fonts.packages from various modules without actually installing them on the system unless the user has agreed to that. Also keeping option parity with NixOS is nice as otherwise I need more conditional logic in my flake (I share most of my configuration between NixOS and nix-darwin).

I'm not too concerned about the identical fonts issue. If that is a problem, you could do something like build a single package that puts all fonts directly in $out (thus preserving the behavior of identically-named fonts overriding each other) and then copy that store path into /Library/Fonts/Nix Fonts (thus producing e.g. /Library/Fonts/Nix Fonts/deadbeefdeadbeefdeadbeefdeadbeef-fonts/*). This way you only need to do the copy if any fonts change, with the downside that a change to one font will have to copy all fonts again. That said, what you're doing now is conceptually simpler and comes with the upside that two distinct fonts that happen to share the same filename will actually work now. For example, a package that is structured like [compact/Regular.ttf compact/Bold.ttf normal/Regular.ttf normal/Bold.ttf] will actually work.

lilyball avatar Aug 03 '23 22:08 lilyball

FWIW on fonts.fontDir.enable I'm not actually particularly familiar with Linux font handling, but looking through the nixpkgs source it looks like fonts.packages is only actually used if either that's set or if fonts.fontconfig.enable is set.

lilyball avatar Aug 03 '23 22:08 lilyball

Please keep fonts.fontDir.enable. I want the ability to insert fonts into fonts.packages from various modules without actually installing them on the system unless the user has agreed to that. Also keeping option parity with NixOS is nice as otherwise I need more conditional logic in my flake (I share most of my configuration between NixOS and nix-darwin).

The thing is that, as far as I can tell and from past experience, fonts.packages = [ pkgs.inconsolata ]; does install Inconsolata on NixOS; the directory gets included in the fontconfig configuration and should therefore show up in GUI font pickers, etc., so requiring fonts.fontDir.enable for fonts to be installed is inconsistent with NixOS. What that option actually does on NixOS is expose them through an X11-specific global path, run a bunch of X11-specific commands over it, and add it to the Xorg configuration.

In other words, I believe our current behaviour is inconsistent with NixOS, and if we were to support it in a consistent way it would only relate to X server support; your modules that unconditionally inject font packages would already have the undesired behaviour on NixOS. I could be mistaken to some degree here, as it has been a while since I was actively using a NixOS desktop, but notably my old NixOS laptop configuration did not switch this option on and still installed many fonts.

emilazy avatar Aug 03 '23 22:08 emilazy

(I believe that you would need to turn that option on if you were installing e.g. X11 bitmap fonts that you want to use in applications that use the legacy X font APIs rather than fontconfig. We don't support those anyway, or have any X11 integration at all, so the configuration would be a no-op, and the nix-darwin convention is to not support NixOS options that aren't meaningful or don't do anything.)

emilazy avatar Aug 03 '23 22:08 emilazy

Ok, you've convinced me. I had not looked before into how Linux handles fonts. And now I'm wondering if I need to turn off fonts.fontconfig.enable on my headless NixOS installs since it apparently defaults to on rather than being enabled by xserver options, although I guess I haven't enabled any modules on those systems that would be adding fonts.

lilyball avatar Aug 03 '23 22:08 lilyball

Yeah, fontconfig is on by default; for headless NixOS machines you might want to set environment.noXlibs or even use profiles/minimal.nix. It could be reasonable for us to support something similar that would also turn font installation off, but I don't think there are too many headless nix-darwin machines out there and it probably wouldn't buy us much, given that the kludgy overlay noXlibs sets mostly disables GTK/X11 stuff.

(in my experience, the answer to "how Linux handles fonts" is "badly" ^^; )

emilazy avatar Aug 03 '23 22:08 emilazy

Suggestion: if fonts.packages == [] then we should just delete the /Library/Fonts/Nix Fonts directory entirely instead of leaving it empty.

lilyball avatar Aug 03 '23 22:08 lilyball

Ah, I did it this way because /Applications/Nix Apps and ~/Applications/Home Manager Apps exist unconditionally. I don't really love that either and I'd be happy to review a PR to make it work like that uniformly though, especially if it could be coordinated with Home Manager.

emilazy avatar Aug 03 '23 22:08 emilazy

I agree that those should probably be cleaned up if empty, but I don't think that should block doing that here. This PR introduces the brand new behavior of creating /Library/Fonts/Nix Fonts even if fonts.packages is empty, which does not seem great, especially as I'd wager most users nix-darwin don't configure fonts.

lilyball avatar Aug 03 '23 22:08 lilyball

Hmm, I guess I'm just not sure if it's clearly an issue, and this case seems less bad than for applications if anything, since it's not exposed to anywhere user-visible by default. The status quo is simpler in terms of both implementation and interface, and it's only visible to people poking at internal system paths that already have weird stuff out of the box (that presumably-ignored Arial Unicode.ttf symlink). So I'd mostly just prefer to think it over separately and decide on one behaviour for everything. An advantage to uniformly dropping these directories when they'd otherwise be empty is that the uninstaller could be simplified, so I would certainly lean that way, especially if we can get Home Manager to agree.

So I guess I've convinced myself that this should be done, but I'd rather do it for Nix Apps at the same time in another PR :)

emilazy avatar Aug 04 '23 04:08 emilazy

Okay never mind I talked myself into it with the uninstaller thing, and it turns out we can get rsync to do it for us. I'll open PRs to change the behaviour of Nix Apps and Home Manager Apps after this is merged.

emilazy avatar Aug 04 '23 04:08 emilazy

How come this PR stalled? Would be really nice to get parity with nixos again.

marcusramberg avatar Oct 19 '23 10:10 marcusramberg

@Enzime hey, is there an eta on when this will be merged?

the current implementation prevents me from having nix-managed fonts at all - some of the fonts in /Library/Fonts are managed by MDM, so removing them is not an option. and without fonts.fontDir.enable the thing doesn't seem to work at all

reyafyi avatar Mar 03 '24 18:03 reyafyi

Feel free to rebase this and create a new PR and I’ll take a look at it

Enzime avatar Mar 04 '24 05:03 Enzime

Rebased and tested; this should finally be good to go.

emilazy avatar Jun 13 '24 12:06 emilazy