Defer machine font directory creation#4
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prevents Linux and macOS font manager initialization from eagerly creating machine-wide font directories, and instead creates the chosen target directory lazily during installation—fixing user-scope installs when system directories are missing or not writable.
Changes:
- Remove unconditional
ensureDirfor machine font directories duringNewFontManager()on Linux/macOS. - Ensure the selected
targetDirexists insideInstallFont()before copying the font.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/platform/linux.go | Stops eager creation of /usr/local/share/fonts; ensures the chosen install directory exists during install. |
| internal/platform/darwin.go | Stops eager creation of /Library/Fonts; ensures the chosen install directory exists during install. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if err := ensureDir(targetDir); err != nil { | ||
| return fmt.Errorf("failed to ensure font directory exists: %w", err) |
Comment on lines
+73
to
+75
| if err := ensureDir(targetDir); err != nil { | ||
| return fmt.Errorf("failed to ensure font directory exists: %w", err) | ||
| } |
| } | ||
|
|
||
| if err := ensureDir(targetDir); err != nil { | ||
| return fmt.Errorf("failed to ensure font directory exists: %w", err) |
Comment on lines
+72
to
+74
| if err := ensureDir(targetDir); err != nil { | ||
| return fmt.Errorf("failed to ensure font directory exists: %w", err) | ||
| } |
Graphixa
approved these changes
Apr 24, 2026
Owner
Graphixa
left a comment
Graphixa
left a comment
There was a problem hiding this comment.
Thanks @Magniquick
This fixes user-scope installs failing when system font dirs are missing/unwritable by deferring directory creation until InstallFont() (and only for the selected target dir).
Merging.
Graphixa
added a commit
that referenced
this pull request
Apr 24, 2026
Builds on Magniquick's PR #4 by keeping Linux/macOS FontManager initialization side-effect free (no eager user-dir creation) and including scope + target path in ensureDir failures for easier troubleshooting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/usr/local/share/fontsor/Library/Fontsis not writable or does not existVerification
go test ./internal/platformgo run . install google.google-sans --scope user --accept-agreements --accept-defaults --debug/home/magni/.local/share/fonts/usr/local/share/fonts