GNU bug report logs - #39953
27.0.50; [PATCH] explicitly fail if Emacs was configured with but cannot compile native JSON support

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39953 in the body.
You can then email your comments to 39953 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stéphane Maniaci <stephane.maniaci <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; [PATCH] explicitly fail if Emacs was configured with but
 cannot compile native JSON support
Date: Fri, 6 Mar 2020 12:13:17 +0000

[Message part 1 (text/plain, inline)]

Hello,

I was trying to give the Emacs 27 pre-test a spin, notably for the
native JSON support, and it took me a long time to figure out it
wasn't actually being configured for it as I was missing the Jansson
library headers.

This patch explicitly fails ./configure if `--with-json' is passed but
cannot be fulfilled. Which means users without the Jansson headers
won't be able to compile Emacs unless explicitly disabling it with
`--without-json'; I don't know if that is too extreme, maybe
AC_MSG_WARN would be more appropriate, but as long as it's notified in
some way to the user I'm happy.

It's also my very first patch so please excuse any mistakes in the
protocol/patch.

Cheers,
Stéphane.

[0001-configure.ac-warn-users-if-they-can-t-compile-native.patch (application/octet-stream, attachment)]

Message #8 received at 39953 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Maniaci <stephane.maniaci <at> gmail.com>
Cc: 39953 <at> debbugs.gnu.org
Subject: Re: bug#39953: 27.0.50;
 [PATCH] explicitly fail if Emacs was configured with but cannot
 compile native JSON support
Date: Fri, 06 Mar 2020 17:49:10 +0200

> From: Stéphane Maniaci <stephane.maniaci <at> gmail.com>
> Date: Fri, 6 Mar 2020 12:13:17 +0000
> 
> This patch explicitly fails ./configure if `--with-json' is passed but
> cannot be fulfilled. Which means users without the Jansson headers
> won't be able to compile Emacs unless explicitly disabling it with
> `--without-json'; I don't know if that is too extreme, maybe
> AC_MSG_WARN would be more appropriate, but as long as it's notified in
> some way to the user I'm happy.

I think such handling of optional libraries is generally considered as
annoyance, so we only do that for some specific libraries.

Let's see what others think about this proposal.

Message #11 received at 39953 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stéphane Maniaci <stephane.maniaci <at> gmail.com>,
 39953 <at> debbugs.gnu.org
Subject: Re: bug#39953: 27.0.50;
 [PATCH] explicitly fail if Emacs was configured with but cannot
 compile native JSON support
Date: Mon, 09 Mar 2020 22:14:03 -0400

>> This patch explicitly fails ./configure if `--with-json' is passed but
>> cannot be fulfilled. Which means users without the Jansson headers
>> won't be able to compile Emacs unless explicitly disabling it with
>> `--without-json'; I don't know if that is too extreme,

This seems a bit too extreme to me, but I think giving an error on
missing Jansson headers only if --with-json is explicitly passed would
be okay.

Message #14 received at 39953 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stéphane Maniaci <stephane.maniaci <at> gmail.com>,
 39953 <at> debbugs.gnu.org
Subject: Re: bug#39953: 27.0.50; [PATCH] explicitly fail if Emacs was
 configured with but cannot compile native JSON support
Date: Sat, 11 Apr 2020 09:47:34 -0400

[Message part 1 (text/plain, inline)]

Noam Postavsky <npostavs <at> gmail.com> writes:

>>> This patch explicitly fails ./configure if `--with-json' is passed but
>>> cannot be fulfilled. Which means users without the Jansson headers
>>> won't be able to compile Emacs unless explicitly disabling it with
>>> `--without-json'; I don't know if that is too extreme,
>
> This seems a bit too extreme to me, but I think giving an error on
> missing Jansson headers only if --with-json is explicitly passed would
> be okay.

I was reminded about this from some reddit posts:

https://old.reddit.com/r/emacs/comments/fwgpkd/weekly_tipstricketc_thread/fmo9d5v/

Here's a proposed patch implementing my suggestion above.

[0001-Make-with-json-imply-than-jansson-support-is-mandato.patch (text/x-diff, inline)]

From 7161e9ae2e55d2624ec60d33a467af6d9960d6bb Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sat, 11 Apr 2020 09:33:20 -0400
Subject: [PATCH] Make --with-json imply than jansson support is mandatory
 (Bug#39953)

Currently, it is easy to miss that jansson support was not included,
since configure succeeds if the jansson development files (headers,
etc) are missing.  Failing when jansson is missing by default would be
a bit extreme, so this is a compromise; we only fail if --with-json
was explicitly passed.
* configure.ac (OPTION_DEFAULT_IFAVAILABLE): New macro.  Use it to
define the --with-json option.  Add with_json and HAVE_JSON to the
'MISSING' checks.
---
 configure.ac | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 73243001ba..f4b01e833b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -219,6 +219,21 @@ AC_DEFUN
     m4_bpatsubst([with_$1], [[^0-9a-z]], [_])=no])dnl
 ])dnl
 
+dnl OPTION_DEFAULT_IFAVAILABLE(NAME, HELP-STRING)
+dnl Create a new --with option that defaults to 'ifavailable'.
+dnl NAME is the base name of the option.  The shell variable with_NAME
+dnl   will be set to either the user's value (if the option is
+dnl   specified; 'yes' for a plain --with-NAME) or to 'ifavailable' (if the
+dnl   option is not specified).  Note that the shell variable name is
+dnl   constructed as autoconf does, by replacing non-alphanumeric
+dnl   characters with "_".
+dnl HELP-STRING is the help text for the option.
+AC_DEFUN([OPTION_DEFAULT_IFAVAILABLE], [dnl
+  AC_ARG_WITH([$1],[AS_HELP_STRING([--with-$1],[$2])],[],[dnl
+    m4_bpatsubst([with_$1], [[^0-9a-z]], [_])=ifavailable])dnl
+])dnl
+
+
 dnl OPTION_DEFAULT_ON(NAME, HELP-STRING)
 dnl Create a new --with option that defaults to $with_features.
 dnl NAME is the base name of the option.  The shell variable with_NAME
@@ -433,7 +448,7 @@ AC_DEFUN
 OPTION_DEFAULT_OFF([cairo],[compile with Cairo drawing])
 OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
 OPTION_DEFAULT_OFF([imagemagick],[compile with ImageMagick image support])
-OPTION_DEFAULT_ON([json], [don't compile with native JSON support])
+OPTION_DEFAULT_IFAVAILABLE([json], [don't compile with native JSON support])
 
 OPTION_DEFAULT_ON([xft],[don't use XFT for anti aliased fonts])
 OPTION_DEFAULT_ON([harfbuzz],[don't use HarfBuzz for text shaping])
@@ -2950,7 +2965,7 @@ AC_DEFUN
 HAVE_JSON=no
 JSON_OBJ=
 
-if test "${with_json}" = yes; then
+if test "${with_json}" != no; then
   EMACS_CHECK_MODULES([JSON], [jansson >= 2.7],
     [HAVE_JSON=yes], [HAVE_JSON=no])
   if test "${HAVE_JSON}" = yes; then
@@ -3893,6 +3908,11 @@ AC_DEFUN
   *) MISSING="$MISSING gnutls"
      WITH_IFAVAILABLE="$WITH_IFAVAILABLE --with-gnutls=ifavailable";;
 esac
+case $with_json,$HAVE_JSON in
+  no,* | ifavailable,* | *,yes) ;;
+  *) MISSING="$MISSING json"
+     WITH_IFAVAILABLE="$WITH_IFAVAILABLE --with-json=ifavailable";;
+esac
 if test "X${MISSING}" != X; then
   AC_MSG_ERROR([The following required libraries were not found:
     $MISSING
-- 
2.11.0

Message #17 received at 39953 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 Stéphane Maniaci <stephane.maniaci <at> gmail.com>,
 39953 <at> debbugs.gnu.org
Subject: Re: bug#39953: 27.0.50;
 [PATCH] explicitly fail if Emacs was configured with but cannot
 compile native JSON support
Date: Sat, 25 Apr 2020 16:07:45 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

>> This seems a bit too extreme to me, but I think giving an error on
>> missing Jansson headers only if --with-json is explicitly passed would
>> be okay.
>
> I was reminded about this from some reddit posts:
>
> https://old.reddit.com/r/emacs/comments/fwgpkd/weekly_tipstricketc_thread/fmo9d5v/
>
> Here's a proposed patch implementing my suggestion above.

My knowledge about the build system is rudimentary at best, so I can't
comment on your implementation.  The behaviour you suggest sounds
reasonable though.

Best regards,
Stefan Kangas

Message #20 received at 39953 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 Stéphane Maniaci <stephane.maniaci <at> gmail.com>,
 39953 <at> debbugs.gnu.org
Subject: Re: bug#39953: 27.0.50; [PATCH] explicitly fail if Emacs was
 configured with but cannot compile native JSON support
Date: Sat, 08 Aug 2020 15:54:28 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

> I was reminded about this from some reddit posts:
>
> https://old.reddit.com/r/emacs/comments/fwgpkd/weekly_tipstricketc_thread/fmo9d5v/
>
> Here's a proposed patch implementing my suggestion above.
>
>>From 7161e9ae2e55d2624ec60d33a467af6d9960d6bb Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sat, 11 Apr 2020 09:33:20 -0400
> Subject: [PATCH] Make --with-json imply than jansson support is mandatory
>  (Bug#39953)

This sounds like a good idea to me, but this patch from April was never
applied?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #23 received at 39953 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 Stéphane Maniaci <stephane.maniaci <at> gmail.com>,
 39953 <at> debbugs.gnu.org
Subject: Re: bug#39953: 27.0.50; [PATCH] explicitly fail if Emacs was
 configured with but cannot compile native JSON support
Date: Fri, 14 Aug 2020 19:31:52 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>>>>From 7161e9ae2e55d2624ec60d33a467af6d9960d6bb Mon Sep 17 00:00:00 2001
>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Sat, 11 Apr 2020 09:33:20 -0400
>> Subject: [PATCH] Make --with-json imply than jansson support is mandatory
>>  (Bug#39953)
>
> This sounds like a good idea to me, but this patch from April was never
> applied?

I applied the patch and did some testing, and it works for me, so I've
pushed the change to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.