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 |
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 |
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 |
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 |
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 |
Free forum by Nabble | Edit this page |