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
fontconfig: 2.12.6 -> 2.13.92 #73795
Conversation
Is the reset-dirs element invalid ? |
The DTD looks wrong. The element should have |
bcef530
to
d4e5d2c
Compare
Un-drafted because it isn't broke anymore. Thanks @jtojnar for the quick patch ✨ |
d4e5d2c
to
4c06aab
Compare
This should probably be re-evaluated with 2.10 being insecure - I assume we might want to end up using 2.13 as a default, having Context: #92919 (comment) |
Can we incorporate the removal of 2.10 (#92919) and its config from the module system in here? |
We only use configs from 2.10 so that it should be safe. We cannot use non-versioned config location since it might break apps from the future and from the past. I suggest we drop non-versioned config on NixOS, altogether when we decide to stop supporting apps running fontconfig <2.11. |
But I would not do that in this pull request, it is already a big enough change and removing unversioned config dir would make it harder to determine what caused potential issue. |
Falling back to unversioned `/etc/fonts/conf.d` when versioned one does not exist is problematic since it only occurs on non-NixOS systems and those are likely to have a different version of fontconfig. When those versions use incompatible elements in the config, apps using fontconfig will crash. Instead, we are now falling back to the in-package `fonts.conf` file that loads both the versioned global `conf.d` directory and the in-package `conf.d` since using upstream settings on non-NixOS is preferable to not being able to use apps there. In fact, we would not even need to link `fonts.conf`, as the in-package `fonts.conf` will be always used unless someone creates the global one manually (the option is still retained if one wants to write a custom NixOS module and to avoid unnecessary stat call on NixOS). Additionally, since the `fonts.conf` will always load `conf.d` from the package, we no longer need to install them to sytem `/etc` in the module. This needed some mucking with `50-user.conf` which disables configs in user directories (a good thing IMO, NixOS module will turn it back on) but otherwise, it is cleaner. The files are still prioritized by their name, regardless of their location. See NixOS#73795 (comment) for more information.
With previous patch, we no longer load non-versioned fonts.conf file to avoid incompatibilities but this also means fontconfig will not load system-wide installed fonts on non-NixOS systems. As a compromise, let's hardcode the FHS font paths to the built-in config so that the system fonts work there. Unlike with the system config we do not need to worry about compatibility as incompatible font files will be simply ignored. Of course there will still be disparity if the system install fonts to some other location than these two but I am afraid this is the best we can do. See NixOS#73795 (comment) for discussion.
ITS rules are used for extracting translatable strings and they have been moved to external files in 2.13.92 so they are not needed in the config files themselves. Removing them also cuts down on errors/warnings produced when using older versions of fontconfig (< 2.12.92). Now it will only complain about the description element but that is fortunately just a warning, not errors like the ones caused by the its attributes. Thanks to this, we can change the config version in NixOS module back to 2.11 allowing us to re-use the 2.13/2.14 configs for apps built against 2.12 fontconfig.
e05961f
to
6f83450
Compare
Rebased. |
Okay. I assume such a change only is in the module system, so we could file the followup PR to master once this has cycled through staging. |
This test started failing as described in the code comment. May be related to Nixpkgs updating Fontconfig from version 2.12 to 2.13. See NixOS/nixpkgs#73795.
This test started failing as described in the code comment. May be related to Nixpkgs updating Fontconfig from version 2.12 to 2.13. See NixOS/nixpkgs#73795.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/thunderbird-crash-after-upgrade-to-20-09/9915/4 |
This test started failing as described in the code comment. May be related to Nixpkgs updating Fontconfig from version 2.12 to 2.13. See NixOS/nixpkgs#73795.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Motivation for this change
This currently fails to build atpostInstall
withThings done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @
Closes: #73725