Skip to content
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: update 2.10.2 -> 2.11.1 #4410

Closed
wants to merge 1 commit into from

Conversation

lucabrunox
Copy link
Contributor

Reference: #2050

  • Retained fontconfig_210
  • Search for configs under /etc/fonts/2.11
  • Use $fontconfig/etc/fonts.conf instead of /etc/fonts/fonts.conf
  • Move nix profile share/fonts and lib/X11/fonts from nixos to makeFontsConf. This allows applications using makeFontsConf, nixos and non-nixos to find fonts in the nix profile in all cases.

Nixos

  • /etc/fonts/2.11/conf.d/00-nixos.conf is being created with options plus <dir> for each font.
  • For backward-compatibility /etc/fonts/fonts.conf is created for fontconfig 2.10.

Non-nixos

  • Applications with fontconfig 2.10 keeps working as they read the system /etc/fonts/fonts.conf.
  • Applications with fontconfig 2.11 will have no fonts. Either install some font with nix-env or set FONTCONFIG_FILE=/etc/fonts/fonts.conf

Tested

  • NixOS vm with fontconfig 2.11 by default. Was able to run an app with fontconfig 2.10 without any font problem.
  • Non-nixos, just needed to install a random font with nix-env. Also works by setting FONTCONFIG_FILE=/etc/fonts/fonts.conf

Untested

Whether apps with custom makeFontsConf work properly. I see no reason for which they shouldn't.

@lucabrunox lucabrunox added the 1.severity: mass-rebuild This PR causes a large number of packages to rebuild label Oct 7, 2014
@lucabrunox
Copy link
Contributor Author

cc @vcunat

@vcunat vcunat self-assigned this Oct 7, 2014
@edolstra
Copy link
Member

edolstra commented Oct 9, 2014

I don't think we should change the default path, because it breaks compatibility with every other system. It requires setting FONTCONFIG_FILE on non-NixOS systems forever going forward, and there is no correct default value for it. That's too high a price for supporting some old binaries (which can always be run by setting FONTCONFIG_FILE).

BTW, if fontconfig 2.11 supports the old config files, can't we generate a config file valid to both 2.10 and 2.11?

@vcunat
Copy link
Member

vcunat commented Oct 9, 2014

Well, yes, currently the 2.11 fontconfig seems to work with our 2.10 /etc/. It might be the least invasive way to go, until further incompatibilities appear. But I don't like the property that the binaries people have would suddenly break now and just eat CPU without displaying anything.

As a compromise, my WIP is designed in a way that the default config is in ${fontconfig}/etc and only eventual changes to that can be put on some special path (like /etc/fonts-nix or even a versioned one; high-level description in #2050). I could integrate it with this and polish within the start of the next week, I believe.

@lucabrunox
Copy link
Contributor Author

@vcunat it's what this PR does as well, except a little more useful for non-nixos and versioned for later incompatibilities.

@edolstra
Copy link
Member

Isn't this something upstream should address? Ideally they would come up with a versioned configuration path (like dbus and polkit do).

@lucabrunox
Copy link
Contributor Author

@vcunat do you remember exactly the bug you encountered when using an old fontconfig with a 2.11 fonts.conf? The dtd changed, but it seems to me it's for specific corner cases.

@vcunat
Copy link
Member

vcunat commented Oct 13, 2014

The diff of changes between 2.10.2 and 2.11.1 DTD:

--- a/fonts.dtd
+++ b/fonts.dtd
@@ -140,7 +140,7 @@
     if 'target' is 'font', execute the match on the result of a font
     selection.
 -->
-<!ELEMENT match (test*, edit*)>
+<!ELEMENT match (test|edit)+>
 <!ATTLIST match
          target (pattern|font|scan) "pattern">

@@ -189,7 +189,7 @@
 <!ELEMENT edit (%expr;)*>
 <!ATTLIST edit
          name CDATA        #REQUIRED
-         mode (assign|assign_replace|prepend|append|prepend_first|append_last) "assign"
+         mode (assign|assign_replace|prepend|append|prepend_first|append_last|delete|delete_all) "assign"
          binding (weak|strong|same) "weak">

 <!--
@@ -201,13 +201,14 @@
 <!ATTLIST double xml:space (default|preserve) 'preserve'>
 <!ELEMENT string (#PCDATA)>
 <!ATTLIST string xml:space (default|preserve) 'preserve'>
-<!ELEMENT matrix (double,double,double,double)>
+<!ELEMENT matrix ((%expr;), (%expr;), (%expr;), (%expr;))>
 <!ELEMENT bool (#PCDATA)>
 <!ELEMENT charset (int|range)*>
 <!ELEMENT range (int,int)>
 <!ELEMENT langset (string)*>
 <!ELEMENT name (#PCDATA)>
-<!ATTLIST name xml:space (default|preserve) 'preserve'>
+<!ATTLIST name xml:space (default|preserve) 'preserve'
+         target (default|font|pattern) 'default'>
 <!ELEMENT const (#PCDATA)>
 <!ATTLIST const xml:space (default|preserve) 'preserve'>
 <!ELEMENT or (%expr;)*>

I'm not good in reading DTDs, so e.g. I don't know why the xml:space started to create problems with 2.11 (no longer, as we clean the xml:space attributes in generated fonts.conf). @lethalman: IIRC some of the matrix elements caused a problem for the 2.10 lib.

@lucabrunox
Copy link
Contributor Author

@edolstra the above diff seems clearly incompatible with version 2.10, how could one e.g. replace an expression in matrix of config 2.11 to be a constant compatible with fontconfig 2.10? Or mode "delete".
Evidently upstream doesn't care because they assume a global state of the system. Distributions upgrade fontconfig and with it the configuration used by all applications in the system.

@vcunat
Copy link
Member

vcunat commented Oct 18, 2014

In principle this looks good to me, and very similar to what I had in mind anyway. I got deeper into fontconfig issues three weeks ago (time flies too fast), so I would like to integrate my observations into this.

It would be nice to stabilize this before the release, so e.g. it does not get locked out of gnome 3.14. I plan to integrate and re-test the ideas tomorrow.

Cache

  • Upstream versions cache and they did bump .3 suffix to .4 this time, so it should be fine to share it.
  • <cachedir>~/.fontconfig</cachedir> is obsolete and should be just replaced by <cachedir prefix="xdg">fontconfig</cachedir>.
  • Note: /var/cache/fontconfig doesn't have much meaning for us (although we may well keep it). I think on other distros the way is that some complete cache-regeneration is run by the package manager, so all system-wide stuff is cached in there, shared by all users, so each user only has to cache his/her specific changes. We do no such generation (yet; we could put some cache in nix store), so the directory is only written when root uses some fontconfig stuff.

Font dirs

  • I would add /run/current-system/sw/* to font paths. It seems natural to just add fonts to systemPackages, so why not pick them up. (We need to keep the special fonts.fonts attribute, though, at least for compatibility.)
  • Maybe remove the obsoleted <dir>~/.fonts</dir>?
  • Perhaps put a font or two into the configure option --with-default-fonts. It is the list of fonts used when loading config fails for some reason. I hope we would then have at least something readable instead of those rectangles. (However, these would get a build+runtime dependency of fontconfig.)

Config inclusion

  • We should add <include prefix="xdg" ignore_missing="yes">fontconfig/conf.d</include> (for example), so that people can have per-user config modifications. I would probably not make this path depend on fontconfig version, as it's meant for minor config changes, which should be compatible.
  • I would reorder the config inclusion from the upstream conf.d to the most user-specific. The best order of adding the config directories is slightly uncertain to me; see Cannot override fontconfig #1827 (comment).
  • Future: consider bohoomil's fontconfig changes, but most likely just optionally adding at nixos-level to /etc/fonts (he is maintaining what originally was infinality).

@lucabrunox
Copy link
Contributor Author

I'm ok with most of the things, except using systemPackages to install fonts. It becomes dirty in my opinion, don't please.

@lucabrunox
Copy link
Contributor Author

Updated:

  • Added default fontbhttf, because it's less than 1mb. It does not cover all unicode though, e.g. the square root in gnome-calculator is not shown. Not sure if we should pick a bigger font instead, like dejavu-fonts which is 6mb.
  • Order of conf.d is now first $fontconfig, then /etc/fonts/2.11, then user xdg fontconfig/conf.d
  • First cachedir is now user xdg fontconfig, removed ~/.fontconfig. Retained /var/cache/fontconfig for non-nixos
  • Added user xdg fonts for <dir> before $fontDirectories, removed ~/.fonts

Question: instead of using 2.11 as version, should we instead use the fontconfig version used for the cache? For 2.11 is 4, while for 2.10 is 3. I'm not sure the cache versioning follows the config versioning.

@vcunat
Copy link
Member

vcunat commented Oct 18, 2014

That occurred to me, too. They call it "font configureation file version number" in docs. I had tried to find it in the source tree for some minutes to confirm that, but I failed. IMHO it doesn't really matter what the identifier is, as long as we bump it in correct moment. ("2.11" may be more understandable for users, at least until "2.12" is released without config change.)

Good point about /var/cache/fontconfig.

BTW, you added the font in a wrong place (to get the effect I meant), but I'll fix that easily. The problem is that all standard config is dropped if it finds a mistake on any place, and you only have that configure-time list of font dirs (according to configure help).

@lucabrunox
Copy link
Contributor Author

On Sat, Oct 18, 2014 at 5:30 PM, Vladimír Čunát [email protected]
wrote:

BTW, you added the font in a wrong place (to get the effect I meant), but
I'll fix that easily. The problem is that all standard config is dropped if
it finds a mistake on any place, and you only have that configure-time list
of font dirs (according to configure help).

The --default-fonts in configure just adds

to the auto generated
conf, which gets replaced by the make-conf xsl.


Reply to this email directly or view it on GitHub
#4410 (comment).

www.debian.org - The Universal Operating System

@vcunat
Copy link
Member

vcunat commented Oct 18, 2014

Not only. The configure scripts takes --with-default-fonts and defines its $default_fonts variable, which is injected into the build enviroment as $FC_DEFAULT_FONTS. That one is picked up by makefiles and defined as a preprocessor symbol FC_DEFAULT_FONTS that is also used in ./src/fcinit.c, function FcInitFallbackConfig, to try after loading config fails.

@lucabrunox
Copy link
Contributor Author

Ah that's nice, I overlooked that. Will update the branch :)

On Sat, Oct 18, 2014 at 6:10 PM, Vladimír Čunát [email protected]
wrote:

Not only. The configure scripts takes --with-default-fonts and defines
its $default_fonts variable, which is injected into the build enviroment
as $FC_DEFAULT_FONTS. That one is picked up by makefiles and defined as a
preprocessor symbol FC_DEFAULT_FONTS that is also used in ./src/fcinit.c,
function FcInitFallbackConfig, to try after loading config fails.


Reply to this email directly or view it on GitHub
#4410 (comment).

www.debian.org - The Universal Operating System

@lucabrunox
Copy link
Contributor Author

Added --with-default-fonts=${fontbhttf}. @vcunat without it fontconfig scans the whole tree, with it correctly shows fonts. You probably run the app with a different fontconfig.

@vcunat
Copy link
Member

vcunat commented Oct 19, 2014

Thanks for noting! I deleted that misguided post of mine.

@edolstra
Copy link
Member

Hm, making fontconfig depend on specific fonts seems very strange to me. Is that really necessary? Surely fonts are runtime configuration, not build time dependencies.

@lucabrunox
Copy link
Contributor Author

It's a default one font, just to make sure on any case (especially
non-nixos) one does not see squares. Also the fact that fontconfig accepts
--with-default-fonts seems a sign that depending on a specific font is
not something strange.

Are you suggesting using propagatedUserEnvPkg?

On Sun, Oct 19, 2014 at 2:58 PM, Eelco Dolstra [email protected]
wrote:

Hm, making fontconfig depend on specific fonts seems very strange to me.
Is that really necessary? Surely fonts are runtime configuration, not build
time dependencies.


Reply to this email directly or view it on GitHub
#4410 (comment).

www.debian.org - The Universal Operating System

@vcunat
Copy link
Member

vcunat commented Oct 19, 2014

@edolstra: when loading font config fails for some reason, you get that font. If you don't specify it, you get no font at all...

@lucabrunox
Copy link
Contributor Author

@vcunat so what about merging in staging?

@vcunat
Copy link
Member

vcunat commented Oct 25, 2014

Merging staging into master? Not all rebuilds finished yet, and there are a few regressions. http://hydra.nixos.org/eval/1156552?compare=1156478&full=1#tabs-now-fail The pypy stuff is probably just random, but a few problems are reproducible. Especially #4419, but we can revert that one if need be and it's not a mass-rebuild on Linux.

@lucabrunox
Copy link
Contributor Author

@vcunat no merging the fontconfig upgrade into staging, iirc you have it only in your branch. EDIT: sorry I missed the merge into staging of the change.

@vcunat
Copy link
Member

vcunat commented Oct 25, 2014

Yes, well, github does not show the change, presumably because it was in exactly the same commit hash, only pushed into the official repo.

@edolstra
Copy link
Member

Sorry, but I'm pretty unenthusiastic about this change (using a different /etc/fonts directory and generating 2.10 fonts.conf). It breaks compatibility with non-NixOS systems and is going to produce headaches into the indefinite future. (For instance, how long are we going to keep generating fonts.conf for older versions?) And I don't think fontconfig should provide fonts.

So I think we should just bump fontconfig from 2.10 to 2.11 (assuming we actually need 2.11) and leave it at that (and maybe for the time being generate a 2.10 compatible /etc/fonts.conf).

Going forward we could introduce environment variables FONTCONFIG_FILE_<version> (with fallback to FONTCONFIG_FILE and /etc/fonts). That won't help for existing 2.10 binaries, but it will give a way to handle the problem if it occurs in the future.

(BTW, we've had incompatible fontconfig changes in the past - at some point old fontconfig binaries crashed due to incompatible changes in the format of the fontconfig cache.)

@vcunat
Copy link
Member

vcunat commented Oct 25, 2014

I don't understand the "incompatibility with non-NixOS" part. Why do we need get font config from the other distro?

@edolstra
Copy link
Member

When you run a Nixpkgs application on another distribution, it needs to respect the /etc/fonts of that distribution.

@lucabrunox
Copy link
Contributor Author

Another possibility is to set FONTCONFIG_FILE=/etc/fonts/fonts.conf in nix.sh.

On a side note, I don't see how the fontconfig cache could be a problem since it's versioned.

I'm open to reverting the change, still thinking more about it.

@edolstra
Copy link
Member

What we could do:

  • Patch fontconfig to use $FONTCONFIG_FILE_2_11, $FONTCONFIG_FILE and /etc/fonts/fonts.conf, in that order.
  • On NixOS, set $FONTCONFIG_FILE_2_11 to point to a 2.11 config and $FONTCONFIG_FILE to point to a 2.10 config. We would get rid of the latter at some point in the future.

This way, 2.10 binaries would still work on NixOS, and 2.11 binaries would use /etc/fonts/fonts.conf on non-NixOS systems.

Such a patch might even be acceptable upstream.

@lucabrunox
Copy link
Contributor Author

That seems sensible to me.

@vcunat
Copy link
Member

vcunat commented Oct 25, 2014

Yes, now that it kida works even with wrong config, that approach sounds good... except that I don't much like to pollute the global NixOS env with another variable, so from that point of view I'd prefer @lethalman's suggestion to set FONTCONFIG_FILE=/etc/fonts/fonts.conf in nix.sh.

@vcunat
Copy link
Member

vcunat commented Nov 4, 2014

Hmm, I do have a trivial fontconfig patch for that @edolstra's suggestion, but I think this order will be better: $FONTCONFIG_FILE, /etc/fonts/2.11/fonts.conf, /etc/fonts/fonts.conf. That way we will not need any variable defined, on nixos or non-nixos. Moreover we preserve the ability to override the config in wrappers by the same variable as specified upstream.

Against @edolstra's suggestion we lose the ability to have one environment for mixed 2.10 + 2.11 that need non-standard fonts.conf locations, but I don't think we have any use case for such a combination. (I can implement this if you agree.)

@edolstra
Copy link
Member

edolstra commented Nov 4, 2014

Sounds good to me.

@lucabrunox
Copy link
Contributor Author

👍

@vcunat
Copy link
Member

vcunat commented Nov 5, 2014

Done and tested c0e2ace.

I just wonder why a "command not found" error in postInstall would not make the build fail (I had a typo in there). I couldn't reproduce it with other packages, not even with fontconfig-2.10.

@edolstra
Copy link
Member

edolstra commented Nov 5, 2014

Maybe the generation of the legacy fonts.conf should be conditional on some option and disabled by default. After all it's only needed if you have old packages installed via nix-env.

@lucabrunox
Copy link
Contributor Author

@edolstra I don't think keeping it there even if there's no user is a problem either. It can still be peeked by some statically built program. In this regard, I think /etc/fonts/fonts.conf must always be there. In the future we may replace it with the 2.11 version.

@edolstra
Copy link
Member

edolstra commented Nov 5, 2014

Well, it pulls in fontconfig_210, doesn't it?

@lucabrunox
Copy link
Contributor Author

@edolstra yes, it's not that it's such a big binary either

@vcunat
Copy link
Member

vcunat commented Nov 5, 2014

I thought about extracting only the config part, which is small enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.severity: mass-rebuild This PR causes a large number of packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants