bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Rainer Orth]

I just tried to compile emacs 26.0.91 on Solaris 11.4 Beta.  This failed
building lcms.o:

  CC       lcms.o
/vol/gnu/src/emacs/emacs-26.0.91/src/lcms.c:23:19: fatal error: lcms2.h: No such file or directory

lcms.h lives in /usr/include/lcms/lcms.h and lcms2.pc correctly adds a
matching -I to Cflags.  However, configure.ac just does a link test for
cmsCreateTransform, which succeeds.  I can work around this by adding a
matching -I option to CPPFLAGS at configure time, but it seems silly
having to do so if the information can be derived automatically.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Noam Postavsky]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> I just tried to compile emacs 26.0.91 on Solaris 11.4 Beta.  This failed
> building lcms.o:
>
>   CC       lcms.o
> /vol/gnu/src/emacs/emacs-26.0.91/src/lcms.c:23:19: fatal error: lcms2.h: No such file or directory
>
> lcms.h lives in /usr/include/lcms/lcms.h and lcms2.pc correctly adds a
> matching -I to Cflags.  However, configure.ac just does a link test for
> cmsCreateTransform, which succeeds.  I can work around this by adding a
> matching -I option to CPPFLAGS at configure time, but it seems silly
> having to do so if the information can be derived automatically.

Does the below work?  (I notice that all of a sudden this bug's severity
has been set to "important", although it looks more like it should be
"minor" to me.  And I would intend the patch for master, not emacs-26.)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2618 bytes --]

From 30b2c9bcc26739cc6634f496b3dc415cf5310555 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 4 Feb 2018 20:43:26 -0500
Subject: [PATCH v1] Use pkg-config to find lcms2 CFLAGS and LIBS (Bug#30346)

* configure.ac: Use EMACS_CHECK_MODULES fors LCMS2 rather than
AC_SEARCH_LIBS.
* src/Makefile.in: Get LCMS2_LIBS and LCMS2_CFLAGS from configure,
instead of just LIBLCMS2.
---
 configure.ac    | 12 +++++-------
 src/Makefile.in |  7 ++++---
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index f9c7bb76e5..7ee1fae83b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3469,12 +3469,9 @@ AC_DEFUN
 HAVE_LCMS2=no
 LIBLCMS2=
 if test "${with_lcms2}" != "no"; then
-  OLIBS=$LIBS
-  AC_SEARCH_LIBS([cmsCreateTransform], [lcms2], [HAVE_LCMS2=yes])
-  LIBS=$OLIBS
-  case $ac_cv_search_cmsCreateTransform in
-    -*) LIBLCMS2=$ac_cv_search_cmsCreateTransform ;;
-  esac
+  EMACS_CHECK_MODULES([LCMS2], [lcms2])
+  AC_CHECK_HEADER([lcms2.h])
+  AC_CHECK_LIB([lcms2], [cmsCreateTransform])
 fi
 if test "${HAVE_LCMS2}" = "yes"; then
   AC_DEFINE([HAVE_LCMS2], 1, [Define to 1 if you have the lcms2 library (-llcms2).])
@@ -3483,7 +3480,8 @@ AC_DEFUN
      LIBLCMS2=
   fi
 fi
-AC_SUBST(LIBLCMS2)
+AC_SUBST(LCMS2_CFLAGS)
+AC_SUBST(LCMS2_LIBS)
 
 HAVE_ZLIB=no
 LIBZ=
diff --git a/src/Makefile.in b/src/Makefile.in
index 15ca1667d6..9fa1d7b6f2 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -234,7 +234,8 @@ LIBXML2_CFLAGS =
 
 GETADDRINFO_A_LIBS = @GETADDRINFO_A_LIBS@
 
-LIBLCMS2 = @LIBLCMS2@
+LCMS2_LIBS = @LCMS2_LIBS@
+LCMS2_CFLAGS = @LCMS2_CFLAGS@
 
 LIBZ = @LIBZ@
 
@@ -360,7 +361,7 @@ EMACS_CFLAGS=
   $(GNUSTEP_CFLAGS) $(CFLAGS_SOUND) $(RSVG_CFLAGS) $(IMAGEMAGICK_CFLAGS) \
   $(PNG_CFLAGS) $(LIBXML2_CFLAGS) $(DBUS_CFLAGS) \
   $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFLAGS) \
-  $(WEBKIT_CFLAGS) \
+  $(WEBKIT_CFLAGS) $(LCMS2_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
   $(LIBSYSTEMD_CFLAGS) \
@@ -492,7 +493,7 @@ LIBES =
    $(LIBXML2_LIBS) $(LIBGPM) $(LIBS_SYSTEM) $(CAIRO_LIBS) \
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
-   $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LIBLCMS2) \
+   $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LCMS2_LIBS) \
    $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS)
 
 ## FORCE it so that admin/unidata can decide whether these files
-- 
2.11.0

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sun, 04 Feb 2018 20:54:45 -0500
> Cc: 30346@debbugs.gnu.org
> 
> Does the below work?  (I notice that all of a sudden this bug's severity
> has been set to "important", although it looks more like it should be
> "minor" to me.  And I would intend the patch for master, not emacs-26.)

I agree.

>  HAVE_LCMS2=no
>  LIBLCMS2=
>  if test "${with_lcms2}" != "no"; then
> -  OLIBS=$LIBS
> -  AC_SEARCH_LIBS([cmsCreateTransform], [lcms2], [HAVE_LCMS2=yes])
> -  LIBS=$OLIBS
> -  case $ac_cv_search_cmsCreateTransform in
> -    -*) LIBLCMS2=$ac_cv_search_cmsCreateTransform ;;
> -  esac
> +  EMACS_CHECK_MODULES([LCMS2], [lcms2])
> +  AC_CHECK_HEADER([lcms2.h])
> +  AC_CHECK_LIB([lcms2], [cmsCreateTransform])
>  fi
>  if test "${HAVE_LCMS2}" = "yes"; then
>    AC_DEFINE([HAVE_LCMS2], 1, [Define to 1 if you have the lcms2 library (-llcms2).])
> @@ -3483,7 +3480,8 @@ AC_DEFUN
>       LIBLCMS2=
>    fi
>  fi
> -AC_SUBST(LIBLCMS2)
> +AC_SUBST(LCMS2_CFLAGS)
> +AC_SUBST(LCMS2_LIBS)

Maybe I'm missing something, but it looks like you effectively renamed
LIBLCMS2 to LCMS2_LIBS, so now LIBLCMS2 is set, but not used, and the
part of configure.ac that took care of the MS-Windows build, where
"-lcms2" should NOT be used at link time, now does nothing useful.
Right?

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Noam Postavsky]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

severity 30346 minor
quit

On Mon, Feb 5, 2018 at 12:21 PM, Eli Zaretskii <eliz@gnu.org> wrote:

>> Does the below work?  (I notice that all of a sudden this bug's severity
>> has been set to "important", although it looks more like it should be
>> "minor" to me.  And I would intend the patch for master, not emacs-26.)
>
> I agree.

> Maybe I'm missing something, but it looks like you effectively renamed
> LIBLCMS2 to LCMS2_LIBS, so now LIBLCMS2 is set, but not used, and the
> part of configure.ac that took care of the MS-Windows build, where
> "-lcms2" should NOT be used at link time, now does nothing useful.
> Right?

Ah, you're right. Also, I think the AC_CHECK_HEADERS/LIB calls I put
wouldn't work without some CFLAGS and LIBS juggling. But I guess
they're redundant anyway unless we start getting reports about
pkg-config of lcms2 being incorrect.

New patch attached (the patch is still against emacs-26 just for
convenience of OP).

[-- Attachment #2: v2-0001-Use-pkg-config-to-find-lcms2-CFLAGS-and-LIBS-Bug-.patch --]
[-- Type: application/octet-stream, Size: 2695 bytes --]

From 746237272c7857bc431f5dcf94a40a680921da3b Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 4 Feb 2018 20:43:26 -0500
Subject: [PATCH v2] Use pkg-config to find lcms2 CFLAGS and LIBS (Bug#30346)

* configure.ac: Use EMACS_CHECK_MODULES fors LCMS2 rather than
AC_SEARCH_LIBS.
* src/Makefile.in: Get LCMS2_LIBS and LCMS2_CFLAGS from configure,
instead of just LIBLCMS2.
---
 configure.ac    | 15 ++++++---------
 src/Makefile.in |  7 ++++---
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index f9c7bb7..32688d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3467,23 +3467,20 @@ AC_DEFUN
 AC_SUBST(LIBJPEG)
 
 HAVE_LCMS2=no
-LIBLCMS2=
+LCMS2_CFLAGS=
+LCMS2_LIBS=
 if test "${with_lcms2}" != "no"; then
-  OLIBS=$LIBS
-  AC_SEARCH_LIBS([cmsCreateTransform], [lcms2], [HAVE_LCMS2=yes])
-  LIBS=$OLIBS
-  case $ac_cv_search_cmsCreateTransform in
-    -*) LIBLCMS2=$ac_cv_search_cmsCreateTransform ;;
-  esac
+  EMACS_CHECK_MODULES([LCMS2], [lcms2])
 fi
 if test "${HAVE_LCMS2}" = "yes"; then
   AC_DEFINE([HAVE_LCMS2], 1, [Define to 1 if you have the lcms2 library (-llcms2).])
   ### mingw32 doesn't use -llcms2, since it loads the library dynamically.
   if test "${opsys}" = "mingw32"; then
-     LIBLCMS2=
+     LCMS2_LIBS=
   fi
 fi
-AC_SUBST(LIBLCMS2)
+AC_SUBST(LCMS2_CFLAGS)
+AC_SUBST(LCMS2_LIBS)
 
 HAVE_ZLIB=no
 LIBZ=
diff --git a/src/Makefile.in b/src/Makefile.in
index 15ca166..9fa1d7b 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -234,7 +234,8 @@ LIBXML2_CFLAGS =
 
 GETADDRINFO_A_LIBS = @GETADDRINFO_A_LIBS@
 
-LIBLCMS2 = @LIBLCMS2@
+LCMS2_LIBS = @LCMS2_LIBS@
+LCMS2_CFLAGS = @LCMS2_CFLAGS@
 
 LIBZ = @LIBZ@
 
@@ -360,7 +361,7 @@ EMACS_CFLAGS=
   $(GNUSTEP_CFLAGS) $(CFLAGS_SOUND) $(RSVG_CFLAGS) $(IMAGEMAGICK_CFLAGS) \
   $(PNG_CFLAGS) $(LIBXML2_CFLAGS) $(DBUS_CFLAGS) \
   $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFLAGS) \
-  $(WEBKIT_CFLAGS) \
+  $(WEBKIT_CFLAGS) $(LCMS2_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
   $(LIBSYSTEMD_CFLAGS) \
@@ -492,7 +493,7 @@ LIBES =
    $(LIBXML2_LIBS) $(LIBGPM) $(LIBS_SYSTEM) $(CAIRO_LIBS) \
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
-   $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LIBLCMS2) \
+   $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LCMS2_LIBS) \
    $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS)
 
 ## FORCE it so that admin/unidata can decide whether these files
-- 
2.6.2.windows.1

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Rainer Orth]

Hi Noam,

> New patch attached (the patch is still against emacs-26 just for
> convenience of OP).

just got around to trying it: worked like a charm.  Thanks.

It would be good if it could go into the emacs-26 branch, too, so that
the 26.1 release compiles out of the box.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Eli Zaretskii]

> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
> Cc: Eli Zaretskii <eliz@gnu.org>, 30346@debbugs.gnu.org
> Date: Tue, 06 Feb 2018 11:18:08 +0100
> 
> It would be good if it could go into the emacs-26 branch, too, so that
> the 26.1 release compiles out of the box.

Sorry, it's too late to make non-trivial changes in the build
machinery of emacs-26 (if we start using lcms2.pc, it might expose us
to issues we never saw since the lcms2 support was added to Emacs).

I think this problem is not too grave, since specifying CPPFLAGS at
configure time solves it.

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Rainer Orth]

Hi Eli,

> Sorry, it's too late to make non-trivial changes in the build
> machinery of emacs-26 (if we start using lcms2.pc, it might expose us
> to issues we never saw since the lcms2 support was added to Emacs).

Understood.

> I think this problem is not too grave, since specifying CPPFLAGS at
> configure time solves it.

Indeed: and there's even the patch in the bug :-)

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

bug#30346: lcms.c doesn't compile when lcms.h isn't in default search path

[Noam Postavsky]

tags 30346 fixed
close 30346 27.1
quit

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> Hi Eli,
>
>> Sorry, it's too late to make non-trivial changes in the build
>> machinery of emacs-26 (if we start using lcms2.pc, it might expose us
>> to issues we never saw since the lcms2 support was added to Emacs).
>
> Understood.
>
>> I think this problem is not too grave, since specifying CPPFLAGS at
>> configure time solves it.
>
> Indeed: and there's even the patch in the bug :-)

Pushed to master.

[1: cb3863370c]: 2018-02-17 08:49:18 -0500
  Use pkg-config to find lcms2 CFLAGS and LIBS (Bug#30346)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=cb3863370cbe574810f796726faa39ba0de0a429