|Summary:||fonts.conf.in update for Windows cross-compiling|
|Component:||conf||Assignee:||Akira TAGOH <akira>|
|Status:||RESOLVED FIXED||QA Contact:||Behdad Esfahbod <freedesktop>|
|Priority:||medium||CC:||akira, drawoc, fontconfig-bugs|
|i915 platform:||i915 features:|
|Attachments:||Do not ignore missing default conf.d/|
Description Jehan 2013-09-26 07:12:08 UTC
Hi, I am a GIMP developer. We had a problem when cross-compiling GIMP for Windows (https://bugzilla.gnome.org/show_bug.cgi?id=708110). This was just a configuration issue, with the configuration files in conf.d/* being inaccessible because the path set by the autotools in fonts.conf during compilation were obviously different once the tree was moved over from a Unix to a Windows system. This simple patch would fix it for us: https://git.gnome.org/browse/gimp/tree/build/windows/jhbuild/patches/fontconfig-fix-config-dir.patch Since it is a configuration issue, it could be argued that it is not a bug. Still I feel that cross-compiling fontconfig would be quite common. And if our tree for GIMP has a patch for fontconfig, it may mean that it would be worth updating upstream too, no? :-) I would propose 2 changes: 1/ Replace @CONFIGDIR@ by "conf.d", and not a full path, only when cross-compiling for Windows like in our patch. 2/ and/or having a warning message when an <include>d directory set in fonts.conf does not exist, so that at least there is some feedback from the software to understand easily the issue (otherwise it was hard to diagnose what was going wrong). :-) Does it make sense?
Comment 1 Akira TAGOH 2013-09-26 10:48:02 UTC
Guess does this work? diff --git a/Makefile.am b/Makefile.am index d310b4b..2b949e4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -90,12 +90,14 @@ $(srcdir)/ChangeLog: .PHONY: ChangeLog +FC_CONFIGDIR = $(subst $(BASECONFIGDIR)/,,$(CONFIGDIR)) + fonts.conf: fonts.conf.in Makefile sed \ -e 's,@FC_CACHEDIR\@,$(FC_CACHEDIR),g' \ -e 's,@FC_DEFAULT_FONTS\@,$(FC_DEFAULT_FONTS),g' \ -e 's,@FC_FONTPATH\@,$(FC_FONTPATH),g' \ - -e 's,@CONFIGDIR\@,$(CONFIGDIR),g' \ + -e 's,@CONFIGDIR\@,$(FC_CONFIGDIR),g' \ -e 's,@PACKAGE\@,$(PACKAGE),g' \ -e 's,@VERSION\@,$(VERSION),g' \ $(srcdir)/$@.in > $@.tmp && \
Comment 2 Jehan 2013-09-29 03:52:10 UTC
Created attachment 86781 [details] [review] Do not ignore missing default conf.d/ Hi Akira, yes I just tested on master, your change looks good. Could I suggest an additional patch to this change? See the included patch. Basically I propose to just replace ignore_missing="yes" on the include by ignore_missing="no". For user-set include, one is free to ignore missing configuration (or to update the default ones). But for the default configuration, I think most users expects them to work out of the box. Consequently we should warn the user if for some reason the default conf.d/ was broken. I hope that makes sense. :-) So I'd propose this patch + your proposed Makefile.am patch would improve default configuration. Thanks!
Comment 3 Akira TAGOH 2013-09-30 02:31:39 UTC
Well, I don't have strong opinion on that but I suppose reading configurations from conf.d is totally optional. that's why it has ignore_missing="yes" there. Please file a separate bug for that and/or ask for that on the list. Anyway, I've committed that fix into master and closing this. Thanks for testing.