Bug 69836 - fonts.conf.in update for Windows cross-compiling
Summary: fonts.conf.in update for Windows cross-compiling
Status: RESOLVED FIXED
Alias: None
Product: fontconfig
Classification: Unclassified
Component: conf (show other bugs)
Version: unspecified
Hardware: Other Windows (All)
: medium normal
Assignee: Akira TAGOH
QA Contact: Behdad Esfahbod
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-26 07:12 UTC by Jehan
Modified: 2013-09-30 02:57 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Do not ignore missing default conf.d/ (627 bytes, patch)
2013-09-29 03:52 UTC, Jehan
Details | Splinter Review

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.
Comment 4 Jehan 2013-09-30 02:57:30 UTC
Thanks for that. :-)
I have created bug 69946 to propose the configuration ignore-missing change.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.