bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Randy Taylor]

[-- Attachment #1.1: Type: text/plain, Size: 102 bytes --]

--without-gif was only checked against X11; with this patch it is now checked against all the options.

[-- Attachment #1.2: Type: text/html, Size: 526 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-configure.ac-Fix-without-gif-for-W32-NS-Haiku-and-PG.patch --]
[-- Type: text/x-patch; name=0001-configure.ac-Fix-without-gif-for-W32-NS-Haiku-and-PG.patch, Size: 2575 bytes --]

From 3fa7d851d2e2925132114e4d33edd37f28143812 Mon Sep 17 00:00:00 2001
From: Randy Taylor <dev@rjt.dev>
Date: Tue, 29 Mar 2022 20:59:18 -0400
Subject: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and
 PGTK.

Copyright-paperwork-exempt: yes
---
 configure.ac | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/configure.ac b/configure.ac
index 10358c2b64..2f998ca4ff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4288,25 +4288,27 @@ AC_DEFUN
   if test "${HAVE_GIF}" = "yes"; then
     AC_DEFINE(HAVE_GIF, 1, [Define to 1 if you have a gif (or ungif) library.])
   fi
-elif test "${HAVE_X11}" = "yes" && test "${with_gif}" != "no" \
-        || test "${HAVE_W32}" = "yes" || test "${HAVE_NS}" = "yes" \
-	|| test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk"; then
-  AC_CHECK_HEADER(gif_lib.h,
-# EGifPutExtensionLast only exists from version libungif-4.1.0b1.
-# Earlier versions can crash Emacs, but version 5.0 removes EGifPutExtensionLast.
-    [AC_CHECK_LIB(gif, GifMakeMapObject, HAVE_GIF=yes,
-        [AC_CHECK_LIB(gif, EGifPutExtensionLast, HAVE_GIF=yes, HAVE_GIF=maybe)])])
-
-  if test "$HAVE_GIF" = yes; then
-    LIBGIF=-lgif
-  elif test "$HAVE_GIF" = maybe; then
-# If gif_lib.h but no libgif, try libungif.
-    AC_CHECK_LIB(ungif, EGifPutExtensionLast, HAVE_GIF=yes, HAVE_GIF=no)
-    test "$HAVE_GIF" = yes && LIBGIF=-lungif
-  fi
+elif test "${HAVE_X11}" = "yes" || test "${HAVE_W32}" = "yes" \
+     || test "${HAVE_NS}" = "yes" || test "${HAVE_BE_APP}" = "yes" \
+     || test "$window_system" = "pgtk"; then
+  if test "${with_gif}" != "no"; then
+    AC_CHECK_HEADER(gif_lib.h,
+      # EGifPutExtensionLast only exists from version libungif-4.1.0b1.
+      # Earlier versions can crash Emacs, but version 5.0 removes EGifPutExtensionLast.
+      [AC_CHECK_LIB(gif, GifMakeMapObject, HAVE_GIF=yes,
+          [AC_CHECK_LIB(gif, EGifPutExtensionLast, HAVE_GIF=yes, HAVE_GIF=maybe)])])
+
+    if test "$HAVE_GIF" = yes; then
+      LIBGIF=-lgif
+    elif test "$HAVE_GIF" = maybe; then
+      # If gif_lib.h but no libgif, try libungif.
+      AC_CHECK_LIB(ungif, EGifPutExtensionLast, HAVE_GIF=yes, HAVE_GIF=no)
+      test "$HAVE_GIF" = yes && LIBGIF=-lungif
+    fi
 
-  if test "${HAVE_GIF}" = "yes"; then
-    AC_DEFINE(HAVE_GIF, 1, [Define to 1 if you have a gif (or ungif) library.])
+    if test "${HAVE_GIF}" = "yes"; then
+      AC_DEFINE(HAVE_GIF, 1, [Define to 1 if you have a gif (or ungif) library.])
+    fi
   fi
 fi
 AC_SUBST(LIBGIF)
-- 
2.35.1

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Randy Taylor <dev@rjt.dev> writes:

> --without-gif was only checked against X11; with this patch it is now
> checked against all the options.

That might've been intentional, I think there was a discussion about
optional image libraries and w32 some time ago.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Eli Zaretskii]

> Date: Wed, 30 Mar 2022 01:45:16 +0000
> From: Randy Taylor <dev@rjt.dev>
> 
> --without-gif was only checked against X11; with this patch it is now checked against all the options.

Please describe the problem you had with the current code.  Because I
don't think I see the problem you are trying to fix.

Thanks.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Randy Taylor]

When I build Emacs I specify "--without-gif".

When using PGTK:
  - With the original code and specifying "--without-gif", here is what configure gives me: Does Emacs use a gif library?                           yes -lgif
  - With the patch and specifying "--without-gif", I get this (as expected): Does Emacs use a gif library?                           no
  - With the patch, if I remove "--without-gif" or specify "--with-gif=yes", I get this (as expected): Does Emacs use a gif library?                           yes -lgif

When using X11:
  - With the original code and specifying "--without-gif", here is what configure gives me (as expected): Does Emacs use a gif library?                           no
  - With the original code and removing "--without-gif" or specifying "--with-gif="yes", here is what configure gives me (as expected): Does Emacs use a gif library?                           yes -lgif

The check in the code for "--without-gif" only applies to X11, but not the other systems.
This patch fixes it to apply for all the systems in that check, in the same fashion as the SVG check (see line 2660 in configure.ac).

------- Original Message -------

On Wednesday, March 30th, 2022 at 08:13, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Wed, 30 Mar 2022 01:45:16 +0000
>
> > From: Randy Taylor dev@rjt.dev
> >
> > --without-gif was only checked against X11; with this patch it is now checked against all the options.
>
> Please describe the problem you had with the current code. Because I
>
> don't think I see the problem you are trying to fix.
>
> Thanks.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Eli Zaretskii]

> Date: Wed, 30 Mar 2022 12:42:54 +0000
> From: Randy Taylor <dev@rjt.dev>
> Cc: 54633@debbugs.gnu.org
> 
> When I build Emacs I specify "--without-gif".
> 
> When using PGTK:
>   - With the original code and specifying "--without-gif", here is what configure gives me: Does Emacs use a gif library?                           yes -lgif
>   - With the patch and specifying "--without-gif", I get this (as expected): Does Emacs use a gif library?                           no
>   - With the patch, if I remove "--without-gif" or specify "--with-gif=yes", I get this (as expected): Does Emacs use a gif library?                           yes -lgif
> 
> When using X11:
>   - With the original code and specifying "--without-gif", here is what configure gives me (as expected): Does Emacs use a gif library?                           no
>   - With the original code and removing "--without-gif" or specifying "--with-gif="yes", here is what configure gives me (as expected): Does Emacs use a gif library?                           yes -lgif
> 
> The check in the code for "--without-gif" only applies to X11, but not the other systems.
> This patch fixes it to apply for all the systems in that check, in the same fashion as the SVG check (see line 2660 in configure.ac).

Thanks.  But your patch touched more than just the PGTK build.  For
w32, for example, I see no reason to make any changes; do you?
Likewise for NS.

Po Lu, do you agree that only PGTK needs a fix?  If co, can you
suggest a fix for PGTK only?

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Randy Taylor]

------- Original Message -------

On Wednesday, March 30th, 2022 at 09:27, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Wed, 30 Mar 2022 12:42:54 +0000
>
> > From: Randy Taylor dev@rjt.dev
> >
> > Cc: 54633@debbugs.gnu.org
> >
> > When I build Emacs I specify "--without-gif".
> >
> > When using PGTK:
> >
> > - With the original code and specifying "--without-gif", here is what configure gives me: Does Emacs use a gif library? yes -lgif
> >
> > - With the patch and specifying "--without-gif", I get this (as expected): Does Emacs use a gif library? no
> >
> > - With the patch, if I remove "--without-gif" or specify "--with-gif=yes", I get this (as expected): Does Emacs use a gif library? yes -lgif
> >
> > When using X11:
> >
> > - With the original code and specifying "--without-gif", here is what configure gives me (as expected): Does Emacs use a gif library? no
> >
> > - With the original code and removing "--without-gif" or specifying "--with-gif="yes", here is what configure gives me (as expected): Does Emacs use a gif library? yes -lgif
> >
> > The check in the code for "--without-gif" only applies to X11, but not the other systems.
> >
> > This patch fixes it to apply for all the systems in that check, in the same fashion as the SVG check (see line 2660 in configure.ac).
>
> Thanks. But your patch touched more than just the PGTK build. For
>
> w32, for example, I see no reason to make any changes; do you?
>
> Likewise for NS.
>
> Po Lu, do you agree that only PGTK needs a fix? If co, can you
>
> suggest a fix for PGTK only?

If someone specifies "--without-gif", then Emacs should not build with gif support. That is why I made the check to apply for all systems. The same behaviour already applies to all the other formats, like SVG which I mentioned.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  But your patch touched more than just the PGTK build.  For
> w32, for example, I see no reason to make any changes; do you?
> Likewise for NS.
>
> Po Lu, do you agree that only PGTK needs a fix?  If co, can you
> suggest a fix for PGTK only?

I didn't yet test this, but it should work correctly.  Randy, please
test.  Thanks.

Native image APIs are used on Haiku and NS for GIF image support, so
this probably doesn't have to change.

diff --git a/configure.ac b/configure.ac
index 93c821eda0..99b2ecc669 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4292,7 +4292,8 @@ AC_DEFUN
   fi
 elif test "${HAVE_X11}" = "yes" && test "${with_gif}" != "no" \
         || test "${HAVE_W32}" = "yes" || test "${HAVE_NS}" = "yes" \
-	|| test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk"; then
+	|| test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk" \
+	&& test "${with_gif}" != "no"; then
   AC_CHECK_HEADER(gif_lib.h,
 # EGifPutExtensionLast only exists from version libungif-4.1.0b1.
 # Earlier versions can crash Emacs, but version 5.0 removes EGifPutExtensionLast.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Randy Taylor <dev@rjt.dev> writes:

> If someone specifies "--without-gif", then Emacs should not build with
> gif support. That is why I made the check to apply for all
> systems. The same behaviour already applies to all the other formats,
> like SVG which I mentioned.

Emacs built without libgif on NS or Haiku will still result in a build
that can display GIF files, since native image APIs are enabled by
default on both those platforms.  (On Haiku the relevant translator also
has to be installed, but it comes in a default installation.)

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Eli Zaretskii]

> Date: Wed, 30 Mar 2022 13:34:22 +0000
> From: Randy Taylor <dev@rjt.dev>
> Cc: Po Lu <luangruo@yahoo.com>, 54633@debbugs.gnu.org
> 
> If someone specifies "--without-gif", then Emacs should not build
> with gif support.

Only on systems where --without-gif indeed removes the GIF support.
For example, if GIF support is built-in with native image display,
then using --without-gif is meaningless.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Randy Taylor]

------- Original Message -------

On Wednesday, March 30th, 2022 at 09:34, Po Lu <luangruo@yahoo.com> wrote:

> Eli Zaretskii eliz@gnu.org writes:
>
> > Thanks. But your patch touched more than just the PGTK build. For
> >
> > w32, for example, I see no reason to make any changes; do you?
> >
> > Likewise for NS.
> >
> > Po Lu, do you agree that only PGTK needs a fix? If co, can you
> >
> > suggest a fix for PGTK only?
>
> I didn't yet test this, but it should work correctly. Randy, please
>
> test. Thanks.
>
> Native image APIs are used on Haiku and NS for GIF image support, so
>
> this probably doesn't have to change.
>
> diff --git a/configure.ac b/configure.ac
>
> index 93c821eda0..99b2ecc669 100644
>
> --- a/configure.ac
>
> +++ b/configure.ac
>
> @@ -4292,7 +4292,8 @@ AC_DEFUN
>
> fi
>
> elif test "${HAVE_X11}" = "yes" && test "${with_gif}" != "no" \
>
> || test "${HAVE_W32}" = "yes" || test "${HAVE_NS}" = "yes" \
>
> - || test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk"; then
>
> + || test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk" \
>
> + && test "${with_gif}" != "no"; then
>
> AC_CHECK_HEADER(gif_lib.h,
>
> # EGifPutExtensionLast only exists from version libungif-4.1.0b1.
>
> # Earlier versions can crash Emacs, but version 5.0 removes EGifPutExtensionLast.

Works for me. Makes sense to only apply it to PGTK.

Thanks.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Randy Taylor]

Anything stopping this from going in?


------- Original Message -------
On Wednesday, March 30th, 2022 at 10:55, Randy Taylor <dev@rjt.dev> wrote:


>
>
>
>
> ------- Original Message -------
>
> On Wednesday, March 30th, 2022 at 09:34, Po Lu luangruo@yahoo.com wrote:
>
> > Eli Zaretskii eliz@gnu.org writes:
> >
> > > Thanks. But your patch touched more than just the PGTK build. For
> > >
> > > w32, for example, I see no reason to make any changes; do you?
> > >
> > > Likewise for NS.
> > >
> > > Po Lu, do you agree that only PGTK needs a fix? If co, can you
> > >
> > > suggest a fix for PGTK only?
> >
> > I didn't yet test this, but it should work correctly. Randy, please
> >
> > test. Thanks.
> >
> > Native image APIs are used on Haiku and NS for GIF image support, so
> >
> > this probably doesn't have to change.
> >
> > diff --git a/configure.ac b/configure.ac
> >
> > index 93c821eda0..99b2ecc669 100644
> >
> > --- a/configure.ac
> >
> > +++ b/configure.ac
> >
> > @@ -4292,7 +4292,8 @@ AC_DEFUN
> >
> > fi
> >
> > elif test "${HAVE_X11}" = "yes" && test "${with_gif}" != "no" \
> >
> > || test "${HAVE_W32}" = "yes" || test "${HAVE_NS}" = "yes" \
> >
> > - || test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk"; then
> >
> > + || test "${HAVE_BE_APP}" = "yes" || test "$window_system" = "pgtk" \
> >
> > + && test "${with_gif}" != "no"; then
> >
> > AC_CHECK_HEADER(gif_lib.h,
> >
> > # EGifPutExtensionLast only exists from version libungif-4.1.0b1.
> >
> > # Earlier versions can crash Emacs, but version 5.0 removes EGifPutExtensionLast.
>
>
> Works for me. Makes sense to only apply it to PGTK.
>
> Thanks.

bug#54633: [PATCH] * configure.ac: Fix --without-gif for W32, NS, Haiku, and PGTK.

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Randy Taylor <dev@rjt.dev> writes:

> Anything stopping this from going in?

No, sorry.  I installed it and am closing this bug.