Recent change to xfns.c breaks compilation

Recent change to xfns.c breaks compilation

[Markus Rost]

   This change:

   2003-07-12  Richard M. Stallman  <rms@bogus.example.com>

	   * xfns.c: Include libpng/png.h instead of png.h.

   causes problems on my GNU/Linux system (redhat 6.2) where png.h is in
   /usr/include.

   What is the rationale for that change?


   Since configure only checks for the presence of png.h (and not
   libpng/png.h), I think we need to check for both and condition the
   #include with HAVE_PNG_H and HAVE_LIBPNG_PNG_H.

It still breaks compilation for me, on gnu/linux and solaris.
Moreover autoconf does not work well after the recent change

2003-07-12  Richard M. Stallman  <rms@gnu.org>

	* configure.in [netbsd systems]: Define HAVE_CRTIN properly.

Here are some lines from the shell:

gcc -c -D_BSD_SOURCE   -Demacs -DHAVE_CONFIG_H -DUSE_LUCID  -I. -I/home/rost/cvs-gnu-linux1/emacs/src -D_BSD_SOURCE -I/usr/X11R6/include -g xfns.c
xfns.c:7568:24: libpng/png.h: No such file or directory
make[1]: *** [xfns.o] Error 1
make[1]: Leaving directory `/home/rost/cvs-gnu-linux1/emacs/src'
make: *** [bootstrap] Error 2

euler:~/cvs-gnu-linux1/emacs> autoconf
configure.in:50: error: possibly undefined macro: AC_DEFINE
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.

euler:~/cvs-gnu-linux1/emacs> autoconf --version
autoconf (GNU Autoconf) 2.57

euler:~/cvs-gnu-linux1/emacs> configure
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
./configure: line 1666: syntax error near unexpected token `AC_DEFINE(HAVE_CRTIN)'
./configure: line 1666: `      AC_DEFINE(HAVE_CRTIN)'

Re: Recent change to xfns.c breaks compilation

[Richard Stallman]

I don't understand this at all, because the call to AC_DEFINE
that I added is just like the others that do work.

Is there an Autoconf expert here?

Re: Recent change to xfns.c breaks compilation

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

|> I don't understand this at all, because the call to AC_DEFINE
|> that I added is just like the others that do work.

No, the other ones are not quoted.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Recent change to xfns.c breaks compilation

[Richard Stallman]

    |> I don't understand this at all, because the call to AC_DEFINE
    |> that I added is just like the others that do work.

    No, the other ones are not quoted.

What does "quoted" mean here?  I don't see anything that
looks like quoting to me.

However, someone introduced some strange square brackets whose
meaning I don't understand.  Emacs can't find what they match.
Was that a fix for this bug?

Re: Recent change to xfns.c breaks compilation

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     |> I don't understand this at all, because the call to AC_DEFINE
>     |> that I added is just like the others that do work.
> 
>     No, the other ones are not quoted.
> 
> What does "quoted" mean here?  I don't see anything that
> looks like quoting to me.
> 
> However, someone introduced some strange square brackets whose
> meaning I don't understand.  Emacs can't find what they match.
> Was that a fix for this bug?

m4 quotes inside of square brackets.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Recent change to xfns.c breaks compilation

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

|> However, someone introduced some strange square brackets whose meaning
|> I don't understand.

These are the quotes in autoconf.

|> Emacs can't find what they match.

That's because of the parens in the case patterns.  POSIX shells also
accept `(...)' for case patterns, but I don't think the Bourne Shell does.

|> Was that a fix for this bug?

Yes.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Recent change to xfns.c breaks compilation

[Markus Rost]

   What does "quoted" mean here?  I don't see anything that
   looks like quoting to me.

What about adding more explanations, like


===Buffer *diff*============================================
cd /home/rost/cvs/emacs/
diff -c /home/rost/cvs/emacs/configure.in /tmp/buffer-content-22086Ybz
*** /home/rost/cvs/emacs/configure.in	Tue Jul 15 11:46:51 2003
--- /tmp/buffer-content-22086Ybz	Thu Jul 17 10:36:19 2003
***************
*** 182,187 ****
--- 182,189 ----
  dnl per the autoconf manual, we can speed up autoconf somewhat by quoting
  dnl the great gob of text.  Thus it's not processed for possible expansion.
  dnl Just make sure the brackets remain balanced.
+ 
+ dnl evil changequote begins
  [
  
  ### If you add support for a new configuration, add code to this
***************
*** 1168,1173 ****
--- 1170,1176 ----
  fi
  
  ]
+ dnl evil changequote ends
  
  if test $unported = yes; then
    AC_MSG_ERROR([Emacs hasn't been ported to `${canonical}' systems.

Diff finished at Thu Jul 17 10:36:20
============================================================

Recent change to xfns.c breaks compilation

[Kim F. Storm]

This change:

2003-07-12  Richard M. Stallman  <rms@gnu.org>

	* xfns.c: Include libpng/png.h instead of png.h.

causes problems on my GNU/Linux system (redhat 6.2) where png.h is in
/usr/include.

What is the rationale for that change?


Since configure only checks for the presence of png.h (and not
libpng/png.h), I think we need to check for both and condition the
#include with HAVE_PNG_H and HAVE_LIBPNG_PNG_H.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Recent change to xfns.c breaks compilation

[Richard Stallman]

    I'm not an expert on autoconf, so I really cannot say.

    However, I think the simpler form to find the header (and define
    either HAVE_LIBPNG_PNG_H or HAVE_PNG_H) should work:

	AC_CHECK_HEADERS(libpng/png.h,,
			 [AC_CHECK_HEADERS(png.h)])

Could someone say if this is correct?

Re: Recent change to xfns.c breaks compilation

[Richard Stallman]

    However, I think the simpler form to find the header (and define
    either HAVE_LIBPNG_PNG_H or HAVE_PNG_H) should work:

	AC_CHECK_HEADERS(libpng/png.h,,
			 [AC_CHECK_HEADERS(png.h)])

Can someone tell me if that really works?
If it does, I'd like to install it.

Re: Recent change to xfns.c breaks compilation

[Kai Großjohann]

Richard Stallman <rms@gnu.org> writes:

>     However, I think the simpler form to find the header (and define
>     either HAVE_LIBPNG_PNG_H or HAVE_PNG_H) should work:
>
> 	AC_CHECK_HEADERS(libpng/png.h,,
> 			 [AC_CHECK_HEADERS(png.h)])
>
> Can someone tell me if that really works?
> If it does, I'd like to install it.

I'm using the following, which appears to compile for me (after
running autoconf and then configure and then make bootstrap).  I only
tested the png.h case and not the libpng/png.h case, though.

Warning: I don't know what I'm doing.

Changed configure.in:

--- configure.in.~1.345.~	Mon Jul 21 09:57:17 2003
+++ configure.in	Tue Jul 22 10:43:55 2003
@@ -2145,14 +2145,20 @@
 
 ### Use -lpng if available, unless `--with-png=no'.
 HAVE_PNG=no
+HAVE_LIBPNG_PNG=no
 if test "${HAVE_X11}" = "yes"; then
   if test "${with_png}" != "no"; then
     AC_CHECK_HEADER(png.h,
       AC_CHECK_LIB(png, png_get_channels, HAVE_PNG=yes, , -lz -lm))
+    AC_CHECK_HEADER(libpng/png.h,
+      AC_CHECK_LIB(png, png_get_channels, HAVE_LIBPNG_PNG=yes, , -lz -lm))
   fi
 
   if test "${HAVE_PNG}" = "yes"; then
     AC_DEFINE(HAVE_PNG, 1, [Define to 1 if you have the png library (-lpng).])
+  fi
+  if test "${HAVE_LIBPNG_PNG}" = "yes"; then
+    AC_DEFINE(HAVE_LIBPNG_PNG, 1, [Define to 1 if you have the png library (-lpng).])
   fi
 fi
 

Changed src/xnfs.c:

--- xfns.c.~1.585.~	Mon Jul 14 09:58:20 2003
+++ xfns.c	Tue Jul 22 10:45:20 2003
@@ -7564,8 +7564,13 @@
  ***********************************************************************/
 
 #if HAVE_PNG
-
+#include <png.h>
+#endif
+#if HAVE_LIBPNG_PNG
 #include <libpng/png.h>
+#endif
+
+#if (HAVE_PNG||HAVE_LIBPNG_PNG)
 
 /* Function prototypes.  */
 


-- 
~/.signature

Re: Recent change to xfns.c breaks compilation

[Richard Stallman]

    I'm using the following, which appears to compile for me (after
    running autoconf and then configure and then make bootstrap).  I only
    tested the png.h case and not the libpng/png.h case, though.

It looks like David Love solved this problem.
Thanks, Dave.