Extending configure.ac

[Abdallah Elshamy]

Greetings,

I was trying to add RapidJSON [1] to Octave. At first, I thought about using AC_CHECK_LIB (after some reading in configure.ac, I noticed that Octave uses the custom macro OCTAVE_CHECK_LIB which I think is the same thing with some extra parameters.) But, since RapidJSON [1] is a header-only library, I thought about using AC_CHECK_HEADER. Which option is more preferable? 

Also, should I stop the configure script (with AC_MSG_ERROR for example) If the library is not found or should I disable the functions ? (I think the latter is better. If it is, any source on how to do this will be helpful.)

Thanks in advance,

Abdallah

[1] https://rapidjson.org/

[siko1056]

On 5/21/20 10:51 AM, Abdallah Elshamy wrote:

> Greetings,
>
> I was trying to add RapidJSON [1] to Octave. At first, I thought about
> using AC_CHECK_LIB (after some reading in configure.ac
> I noticed that Octave uses the custom macro
> OCTAVE_CHECK_LIB which I think is the same thing with some extra
> parameters.) But, since RapidJSON [1] is a header-only library, I
> thought about using AC_CHECK_HEADER. Which option is more preferable? 
>
> Also, should I stop the configure script (with AC_MSG_ERROR for example)
> If the library is not found or should I disable the functions ? (I think
> the latter is better. If it is, any source on how to do this will be
> helpful.)
>
> Thanks in advance,
> Abdallah
>

Dear Abdallah,
Dear maintainers,  (please read below)

Good question.  RapidJSON should be an optional dependency, thus a
warning shall be issued, if not available.  Later in your implementation
you wrap your code with

   #if defined (HAVE_RAPIDJSON)

     // RapidJSON dependent code

   #else

     err_disabled_feature ("RapidJSON", "JSON library");

   #endif

to avoid unexpected usage.

For the rest, I think OCTAVE_CHECK_LIB is the right way
to go.  This macro is documented here [2] and does everything needed,
but a little too much :(

I tested a small code (AC_LANG_PUSH thanks to StackOverflow [3]):


  ### Check for RAPIDJSON (C++ header only) library.

  AC_LANG_PUSH([C++])
  OCTAVE_CHECK_LIB(rapidjson, RAPIDJSON,
    [RAPIDJSON library not found.  Octave will not be able to read and
     write JSON files.],
    [rapidjson/rapidjson.h])
  AC_LANG_POP([C++])


but it is still not working, as the default macro wants to check for a
linkable library, i.e. "-lrapidjson".  This of course fails.

For now you can assume, that `#include "rapidjson/rapidjson.h"` will
work and ignore the "configure.ac" part.  Later when merging your
contribution, this becomes important.



Dear maintainers,

Stated the problem above: How do we properly check for a C++ header only
library?

1. General solution: Copy OCTAVE_CHECK_LIB to OCTAVE_CHECK_HEADER_LIB
and to remove the unnecessary linking check parts.

2. RapidJSON solution: Copy necessary parts from OCTAVE_CHECK_LIB.

3. Did I miss a better option? ;-)

Thanks for your help,
Kai


[1] https://rapidjson.org/
[2]
https://hg.savannah.gnu.org/hgweb/octave/file/70908e5d8865/m4/acinclude.m4#l414
[3] https://stackoverflow.com/a/6129834/3778706

[mmuetzel]

Am 22. Mai 2020 um 09:40 Uhr schrieb "Kai Torben Ohlhus":

> On 5/21/20 10:51 AM, Abdallah Elshamy wrote:
> > Greetings,
> >
> > I was trying to add RapidJSON [1] to Octave. At first, I thought about
> > using AC_CHECK_LIB (after some reading in configure.ac
> > I noticed that Octave uses the custom macro
> > OCTAVE_CHECK_LIB which I think is the same thing with some extra
> > parameters.) But, since RapidJSON [1] is a header-only library, I
> > thought about using AC_CHECK_HEADER. Which option is more preferable? 
> >
> > Also, should I stop the configure script (with AC_MSG_ERROR for example)
> > If the library is not found or should I disable the functions ? (I think
> > the latter is better. If it is, any source on how to do this will be
> > helpful.)
> >
> > Thanks in advance,
> > Abdallah
> >
>
> Dear Abdallah,
> Dear maintainers,  (please read below)
>
> Good question.  RapidJSON should be an optional dependency, thus a
> warning shall be issued, if not available.  Later in your implementation
> you wrap your code with
>
>    #if defined (HAVE_RAPIDJSON)
>
>      // RapidJSON dependent code
>
>    #else
>
>      err_disabled_feature ("RapidJSON", "JSON library");
>
>    #endif
>
> to avoid unexpected usage.
>
> For the rest, I think OCTAVE_CHECK_LIB is the right way
> to go.  This macro is documented here [2] and does everything needed,
> but a little too much :(
>
> I tested a small code (AC_LANG_PUSH thanks to StackOverflow [3]):
>
>
>   ### Check for RAPIDJSON (C++ header only) library.
>
>   AC_LANG_PUSH([C++])
>   OCTAVE_CHECK_LIB(rapidjson, RAPIDJSON,
>     [RAPIDJSON library not found.  Octave will not be able to read and
>      write JSON files.],
>     [rapidjson/rapidjson.h])
>   AC_LANG_POP([C++])
>
>
> but it is still not working, as the default macro wants to check for a
> linkable library, i.e. "-lrapidjson".  This of course fails.
>
> For now you can assume, that `#include "rapidjson/rapidjson.h"` will
> work and ignore the "configure.ac" part.  Later when merging your
> contribution, this becomes important.
>
>
>
> Dear maintainers,
>
> Stated the problem above: How do we properly check for a C++ header only
> library?
>
> 1. General solution: Copy OCTAVE_CHECK_LIB to OCTAVE_CHECK_HEADER_LIB
> and to remove the unnecessary linking check parts.
>
> 2. RapidJSON solution: Copy necessary parts from OCTAVE_CHECK_LIB.
>
> 3. Did I miss a better option? ;-)

I think that for header only libraries, you should use AC_CHECK_HEADERS - like Abdallah suggested.
See e.g. the checks for (alternative) termio headers:
## Find a termio header to include.

AC_CHECK_HEADERS([termios.h], have_termios_h=yes, have_termios_h=no)
AC_CHECK_HEADERS([termio.h], have_termio_h=yes, have_termio_h=no)
AC_CHECK_HEADERS([sgtty.h], have_sgtty_h=yes, have_sgtty_h=no)
AC_CHECK_HEADERS([conio.h], have_conio_h=yes, have_conio_h=no)

Is there a reason this won't work for RapidJSON?

Markus

[siko1056]

On 5/22/20 5:25 PM, "Markus Mützel" wrote:

> Am 22. Mai 2020 um 09:40 Uhr schrieb "Kai Torben Ohlhus":
>> On 5/21/20 10:51 AM, Abdallah Elshamy wrote:
>>> Greetings,
>>>
>>> I was trying to add RapidJSON [1] to Octave. At first, I thought about
>>> using AC_CHECK_LIB (after some reading in configure.ac
>>> I noticed that Octave uses the custom macro
>>> OCTAVE_CHECK_LIB which I think is the same thing with some extra
>>> parameters.) But, since RapidJSON [1] is a header-only library, I
>>> thought about using AC_CHECK_HEADER. Which option is more preferable? 
>>>
>>> Also, should I stop the configure script (with AC_MSG_ERROR for example)
>>> If the library is not found or should I disable the functions ? (I think
>>> the latter is better. If it is, any source on how to do this will be
>>> helpful.)
>>>
>>> Thanks in advance,
>>> Abdallah
>>>
>>
>> Dear Abdallah,
>> Dear maintainers,  (please read below)
>>
>> Good question.  RapidJSON should be an optional dependency, thus a
>> warning shall be issued, if not available.  Later in your implementation
>> you wrap your code with
>>
>>    #if defined (HAVE_RAPIDJSON)
>>
>>      // RapidJSON dependent code
>>
>>    #else
>>
>>      err_disabled_feature ("RapidJSON", "JSON library");
>>
>>    #endif
>>
>> to avoid unexpected usage.
>>
>> For the rest, I think OCTAVE_CHECK_LIB is the right way
>> to go.  This macro is documented here [2] and does everything needed,
>> but a little too much :(
>>
>> I tested a small code (AC_LANG_PUSH thanks to StackOverflow [3]):
>>
>>
>>   ### Check for RAPIDJSON (C++ header only) library.
>>
>>   AC_LANG_PUSH([C++])
>>   OCTAVE_CHECK_LIB(rapidjson, RAPIDJSON,
>>     [RAPIDJSON library not found.  Octave will not be able to read and
>>      write JSON files.],
>>     [rapidjson/rapidjson.h])
>>   AC_LANG_POP([C++])
>>
>>
>> but it is still not working, as the default macro wants to check for a
>> linkable library, i.e. "-lrapidjson".  This of course fails.
>>
>> For now you can assume, that `#include "rapidjson/rapidjson.h"` will
>> work and ignore the "configure.ac" part.  Later when merging your
>> contribution, this becomes important.
>>
>>
>>
>> Dear maintainers,
>>
>> Stated the problem above: How do we properly check for a C++ header only
>> library?
>>
>> 1. General solution: Copy OCTAVE_CHECK_LIB to OCTAVE_CHECK_HEADER_LIB
>> and to remove the unnecessary linking check parts.
>>
>> 2. RapidJSON solution: Copy necessary parts from OCTAVE_CHECK_LIB.
>>
>> 3. Did I miss a better option? ;-)
>
> I think that for header only libraries, you should use AC_CHECK_HEADERS - like Abdallah suggested.
> See e.g. the checks for (alternative) termio headers:
> ## Find a termio header to include.
>
> AC_CHECK_HEADERS([termios.h], have_termios_h=yes, have_termios_h=no)
> AC_CHECK_HEADERS([termio.h], have_termio_h=yes, have_termio_h=no)
> AC_CHECK_HEADERS([sgtty.h], have_sgtty_h=yes, have_sgtty_h=no)
> AC_CHECK_HEADERS([conio.h], have_conio_h=yes, have_conio_h=no)
>
> Is there a reason this won't work for RapidJSON?
>
> Markus
>

Thank you for looking into it Markus.  By using AC_CHECK_HEADERS only,
we do not get the configure options like

   ./configure --with-rapidjson-includedir=

automatically, right?  Thus, copying the AC_CHECK_HEADERS macro and
other necessary parts from OCTAVE_CHECK_LIB seems like my second
suggestion seems more appropriate.

Kai

[mmuetzel]

Am 22. Mai 2020 um 13:53 Uhr schrieb "Kai Torben Ohlhus"
> Thank you for looking into it Markus.  By using AC_CHECK_HEADERS only,
> we do not get the configure options like
>
>    ./configure --with-rapidjson-includedir=
>
> automatically, right?  Thus, copying the AC_CHECK_HEADERS macro and
> other necessary parts from OCTAVE_CHECK_LIB seems like my second
> suggestion seems more appropriate.

Do we need to have that option for a header-only library?
Wouldn't it be good enough to use something like this if the headers are in a "non-standard" path:
./configure CPPFLAGS=-I/some/path/to/the/headers --with-rapidjson

If you think that the ...-includir syntax is absolutely necessary, you'd probably need to go the way you outlined

Markus