Re: [PATCH] build: Use pkg-config to find Freetype

[Daniel Kiper]

On Thu, Jan 25, 2018 at 12:23:58PM +0000, Colin Watson wrote:
> pkg-config is apparently preferred over freetype-config these days (see
> the BUGS section of freetype-config(1)).  pkg-config support was added
> to Freetype in version 2.1.5, which was released in 2003, so it should

OK but please add somewhere into INSTALL that starting from this patch
Freetype 2.1.5 and newer are only supported versions.

> comfortably be available everywhere by now.
>
> Fixes Debian bug #887721.
>
> Reported-by: Hugh McMaster <address@hidden>
> Signed-off-by: Colin Watson <address@hidden>
> ---
>  INSTALL           |  9 ++++---
>  Makefile.am       |  6 ++---
>  Makefile.util.def |  4 +--
>  configure.ac      | 74 
> +++++++++++++++++++++++--------------------------------
>  4 files changed, 41 insertions(+), 52 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index f3c20edc8..899b9cac5 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -38,6 +38,7 @@ configuring the GRUB.
>  * GNU binutils 2.9.1.0.23 or later
>  * Flex 2.5.35 or later
>  * Other standard GNU/Unix tools
> +* pkg-config

Please put this after Flex.

>  * a libc with large file support (e.g. glibc 2.1 or later)
>
>  On GNU/Linux, you also need:
> @@ -158,8 +159,8 @@ For this example the configure line might look like (more 
> details below)
>  (some options are optional and included here for completeness but some rarely
>  used options are omitted):
>
> -./configure BUILD_CC=gcc BUILD_FREETYPE=freetype-config 
> --host=amd64-linux-gnu
> -CC=amd64-linux-gnu-gcc CFLAGS="-g -O2" 
> FREETYPE=amd64-linux-gnu-freetype-config
> +./configure BUILD_CC=gcc BUILD_PKG_CONFIG=pkg-config --host=amd64-linux-gnu
> +CC=amd64-linux-gnu-gcc CFLAGS="-g -O2" PKG_CONFIG=amd64-linux-gnu-pkg-config
>  --target=arm --with-platform=uboot TARGET_CC=arm-elf-gcc
>  TARGET_CFLAGS="-Os -march=armv6" TARGET_CCASFLAGS="-march=armv6"
>  TARGET_OBJCOPY="arm-elf-objcopy" TARGET_STRIP="arm-elf-strip"
> @@ -176,7 +177,7 @@ corresponding platform are not needed for the platform in 
> question.
>      2. BUILD_CFLAGS= for C options for build.
>      3. BUILD_CPPFLAGS= for C preprocessor options for build.
>      4. BUILD_LDFLAGS= for linker options for build.
> -    5. BUILD_FREETYPE= for freetype-config for build (optional).
> +    5. BUILD_PKG_CONFIG= for pkg-config for build (optional).
>
>    - For host
>      1. --host= to autoconf name of host.
> @@ -184,7 +185,7 @@ corresponding platform are not needed for the platform in 
> question.
>      3. HOST_CFLAGS= for C options for host.
>      4. HOST_CPPFLAGS= for C preprocessor options for host.
>      5. HOST_LDFLAGS= for linker options for host.
> -    6. FREETYPE= for freetype-config for host (optional).
> +    6. PKG_CONFIG= for pkg-config for host (optional).
>      7. Libdevmapper if any must be in standard linker folders (-ldevmapper) 
> (optional).
>      8. Libfuse if any must be in standard linker folders (-lfuse) (optional).
>      9. Libzfs if any must be in standard linker folders (-lzfs) (optional).
> diff --git a/Makefile.am b/Makefile.am
> index 7795baeb6..da4e65bcc 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -71,7 +71,7 @@ endif
>  starfield_theme_files = $(srcdir)/themes/starfield/blob_w.png 
> $(srcdir)/themes/starfield/boot_menu_c.png 
> $(srcdir)/themes/starfield/boot_menu_e.png 
> $(srcdir)/themes/starfield/boot_menu_ne.png 
> $(srcdir)/themes/starfield/boot_menu_n.png 
> $(srcdir)/themes/starfield/boot_menu_nw.png 
> $(srcdir)/themes/starfield/boot_menu_se.png 
> $(srcdir)/themes/starfield/boot_menu_s.png 
> $(srcdir)/themes/starfield/boot_menu_sw.png 
> $(srcdir)/themes/starfield/boot_menu_w.png 
> $(srcdir)/themes/starfield/slider_c.png 
> $(srcdir)/themes/starfield/slider_n.png 
> $(srcdir)/themes/starfield/slider_s.png 
> $(srcdir)/themes/starfield/starfield.png 
> $(srcdir)/themes/starfield/terminal_box_c.png 
> $(srcdir)/themes/starfield/terminal_box_e.png 
> $(srcdir)/themes/starfield/terminal_box_ne.png 
> $(srcdir)/themes/starfield/terminal_box_n.png 
> $(srcdir)/themes/starfield/terminal_box_nw.png 
> $(srcdir)/themes/starfield/terminal_box_se.png 
> $(srcdir)/themes/starfield/terminal_box_s.png 
> $(srcdir)/themes/starfield/terminal_box_sw.png 
> $(srcdir)/themes/starfield/terminal_box_w.png 
> $(srcdir)/themes/starfield/theme.txt $(srcdir)/themes/starfield/README 
> $(srcdir)/themes/starfield/COPYING.CC-BY-SA-3.0
>
>  build-grub-mkfont$(BUILD_EXEEXT): util/grub-mkfont.c grub-core/unidata.c 
> grub-core/kern/emu/misc.c util/misc.c
> -     $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 -DGRUB_BUILD_PROGRAM_NAME=\"build-grub-mkfont\" $^ 
> $(build_freetype_cflags) $(build_freetype_libs)
> +     $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 -DGRUB_BUILD_PROGRAM_NAME=\"build-grub-mkfont\" $^ 
> $(build_freetype_CFLAGS) $(build_freetype_LIBS)
>  CLEANFILES += build-grub-mkfont$(BUILD_EXEEXT)
>
>  garbage-gen$(BUILD_EXEEXT): util/garbage-gen.c
> @@ -80,11 +80,11 @@ CLEANFILES += garbage-gen$(BUILD_EXEEXT)
>  EXTRA_DIST += util/garbage-gen.c
>
>  build-grub-gen-asciih$(BUILD_EXEEXT): util/grub-gen-asciih.c
> -     $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 $^ $(build_freetype_cflags) $(build_freetype_libs) -Wall -Werror
> +     $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 $^ $(build_freetype_CFLAGS) $(build_freetype_LIBS) -Wall -Werror
>  CLEANFILES += build-grub-gen-asciih$(BUILD_EXEEXT)
>
>  build-grub-gen-widthspec$(BUILD_EXEEXT): util/grub-gen-widthspec.c
> -     $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 $^ $(build_freetype_cflags) $(build_freetype_libs) -Wall -Werror
> +     $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 $^ $(build_freetype_CFLAGS) $(build_freetype_LIBS) -Wall -Werror
>  CLEANFILES += build-grub-gen-widthspec$(BUILD_EXEEXT)
>
>  if COND_STARFIELD
> diff --git a/Makefile.util.def b/Makefile.util.def
> index f9caccb97..471b226c9 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -302,14 +302,14 @@ program = {
>    common = grub-core/kern/emu/argp_common.c;
>    common = grub-core/osdep/init.c;
>
> -  cflags = '$(freetype_cflags)';
> +  cflags = '$(freetype_CFLAGS)';

May I ask you to this change in separate patch?
And please use FREETYPE_CFLAGS instead of freetype_CFLAGS.

>    cppflags = '-DGRUB_MKFONT=1';
>
>    ldadd = libgrubmods.a;
>    ldadd = libgrubgcry.a;
>    ldadd = libgrubkern.a;
>    ldadd = grub-core/gnulib/libgnu.a;
> -  ldadd = '$(freetype_libs)';
> +  ldadd = '$(freetype_LIBS)';

Same as above... And of course s/freetype_LIBS/FREETYPE_LIBS/

>    ldadd = '$(LIBINTL) $(LIBDEVMAPPER) $(LIBZFS) $(LIBNVPAIR) $(LIBGEOM)';
>    condition = COND_GRUB_MKFONT;
>  };
> diff --git a/configure.ac b/configure.ac
> index c7888e40f..4797c23df 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -50,6 +50,10 @@ AC_PREREQ(2.60)
>  AC_CONFIG_SRCDIR([include/grub/dl.h])
>  AC_CONFIG_HEADER([config-util.h])
>
> +# Explicitly check for pkg-config early on, since otherwise conditional
> +# calls are problematic.
> +PKG_PROG_PKG_CONFIG
> +
>  # Program name transformations
>  AC_ARG_PROGRAM
>  grub_TRANSFORM([grub-bios-setup])
> @@ -1508,29 +1512,22 @@ if test x"$enable_grub_mkfont" = xno ; then
>    grub_mkfont_excuse="explicitly disabled"
>  fi
>
> -if test x"$grub_mkfont_excuse" = x ; then
> -  # Check for freetype libraries.
> -  AC_CHECK_TOOLS([FREETYPE], [freetype-config])
> -  if test "x$FREETYPE" = x ; then
> -    grub_mkfont_excuse=["need freetype2 library"]
> -  fi
> -fi
> -
>  unset ac_cv_header_ft2build_h
>
>  if test x"$grub_mkfont_excuse" = x ; then
>    # Check for freetype libraries.
> -  freetype_cflags=`$FREETYPE --cflags`
> -  freetype_libs=`$FREETYPE --libs`
> -  SAVED_CPPFLAGS="$CPPFLAGS"
> -  SAVED_LIBS="$LIBS"
> -  CPPFLAGS="$CPPFLAGS $freetype_cflags"
> -  LIBS="$LIBS $freetype_libs"
> -  AC_CHECK_HEADERS([ft2build.h], [],
> -     [grub_mkfont_excuse=["need freetype2 headers"]])
> -  AC_LINK_IFELSE([AC_LANG_CALL([], [FT_Load_Glyph])], [], 
> [grub_mkfont_excuse=["freetype2 library unusable"]])
> -  CPPFLAGS="$SAVED_CPPFLAGS"
> -  LIBS="$SAVED_LIBS"
> +  PKG_CHECK_MODULES([freetype], [freetype2], [
> +    SAVED_CPPFLAGS="$CPPFLAGS"
> +    SAVED_LIBS="$LIBS"
> +    CPPFLAGS="$CPPFLAGS $freetype_CFLAGS"
> +    LIBS="$LIBS $freetype_LIBS"
> +    AC_CHECK_HEADERS([ft2build.h], [],
> +      [grub_mkfont_excuse=["need freetype2 headers"]])
> +    AC_LINK_IFELSE([AC_LANG_CALL([], [FT_Load_Glyph])], [],
> +      [grub_mkfont_excuse=["freetype2 library unusable"]])
> +    CPPFLAGS="$SAVED_CPPFLAGS"
> +    LIBS="$SAVED_LIBS"
> +  ], [grub_mkfont_excuse=["need freetype2 library"]])
>  fi
>
>  if test x"$enable_grub_mkfont" = xyes && test x"$grub_mkfont_excuse" != x ; 
> then
> @@ -1542,8 +1539,6 @@ else
>  enable_grub_mkfont=no
>  fi
>  AC_SUBST([enable_grub_mkfont])
> -AC_SUBST([freetype_cflags])
> -AC_SUBST([freetype_libs])

Could you explain why do you drop this...

>  SAVED_CC="$CC"
>  SAVED_CPP="$CPP"
> @@ -1573,25 +1568,21 @@ AC_SUBST([BUILD_WORDS_BIGENDIAN])
>
>  if test x"$grub_build_mkfont_excuse" = x ; then
>    # Check for freetype libraries.
> -  AC_CHECK_PROGS([BUILD_FREETYPE], [freetype-config])
> -  if test "x$BUILD_FREETYPE" = x ; then
> -    grub_build_mkfont_excuse=["need freetype2 library"]
> -  fi
> -fi
> -
> -if test x"$grub_build_mkfont_excuse" = x ; then
> -  # Check for freetype libraries.
> -  build_freetype_cflags=`$BUILD_FREETYPE --cflags`
> -  build_freetype_libs=`$BUILD_FREETYPE --libs`
> -  SAVED_CPPFLAGS_2="$CPPFLAGS"
> -  SAVED_LIBS="$LIBS"
> -  CPPFLAGS="$CPPFLAGS $build_freetype_cflags"
> -  LIBS="$LIBS $build_freetype_libs"
> -  AC_CHECK_HEADERS([ft2build.h], [],
> -     [grub_build_mkfont_excuse=["need freetype2 headers"]])
> -  AC_LINK_IFELSE([AC_LANG_CALL([], [FT_Load_Glyph])], [], 
> [grub_build_mkfont_excuse=["freetype2 library unusable"]])
> -  LIBS="$SAVED_LIBS"
> -  CPPFLAGS="$SAVED_CPPFLAGS_2"
> +  SAVED_PKG_CONFIG="$PKG_CONFIG"
> +  test -z "$BUILD_PKG_CONFIG" || PKG_CONFIG="$BUILD_PKG_CONFIG"
> +  PKG_CHECK_MODULES([build_freetype], [freetype2], [
> +    SAVED_CPPFLAGS_2="$CPPFLAGS"
> +    SAVED_LIBS="$LIBS"
> +    CPPFLAGS="$CPPFLAGS $build_freetype_CFLAGS"
> +    LIBS="$LIBS $build_freetype_LIBS"
> +    AC_CHECK_HEADERS([ft2build.h], [],
> +      [grub_build_mkfont_excuse=["need freetype2 headers"]])
> +    AC_LINK_IFELSE([AC_LANG_CALL([], [FT_Load_Glyph])], [],
> +      [grub_build_mkfont_excuse=["freetype2 library unusable"]])
> +    LIBS="$SAVED_LIBS"
> +    CPPFLAGS="$SAVED_CPPFLAGS_2"
> +  ], [grub_build_mkfont_excuse=["need freetype2 library"]])
> +  PKG_CONFIG="$SAVED_PKG_CONFIG"
>  fi
>
>  if test x"$enable_build_grub_mkfont" = xyes && test 
> x"$grub_build_mkfont_excuse" != x ; then
> @@ -1610,9 +1601,6 @@ if test x"$enable_build_grub_mkfont" = xno && ( test 
> "x$platform" = xqemu || tes
>    fi
>  fi
>
> -AC_SUBST([build_freetype_cflags])
> -AC_SUBST([build_freetype_libs])

...and this?

Daniel