Thanks for the feedback Vishal, Dan and Aneesh
I have sent out a patch based Dan's suggestion at https://lore.kernel.org/nvdimm/[email protected] Aneesh Kumar K V <[email protected]> writes: > On 3/15/22 10:16 PM, Vaibhav Jain wrote: >> "Aneesh Kumar K.V" <[email protected]> writes: >> >>> Vaibhav Jain <[email protected]> writes: >>> >>>> Second hunk of this diff seems to be a revert of [1] which might >>>> break the ndctl build on Arch Linux. >>>> >>>> AFAIS for Centos/Fedora/RHEL etc the iniparser.h file is present in the >>>> default include path('/usr/include') as a softlink to >>>> '/usr/include/iniparser/iniparser.h' . Ubuntu/Debian seems to an >>>> exception where path '/usr/include/iniparser.h' is not present. >>>> >>>> I guess thats primarily due to no 'make install' target available in >>>> iniparser makefiles [1] that fixes a single include patch. This may have >>>> led >>>> to differences across distros where to place these header files. >>>> >>>> I would suggest changing to this in meson.build to atleast catch if the >>>> iniparser.h is not present at the expected path during meson setup: >>>> >>>> iniparser = cc.find_library('iniparser', required : true, >>>> has_headers: 'iniparser.h') >>>> >>>> [1] addc5fd8511('Fix iniparser.h include') >>>> [2] https://github.com/ndevilla/iniparser/blob/master/Makefile >>> >>> >>> We can do this. >>> >>> diff --git a/config.h.meson b/config.h.meson >>> index 2852f1e9cd8b..b070df96cf4a 100644 >>> --- a/config.h.meson >>> +++ b/config.h.meson >>> @@ -82,6 +82,9 @@ >>> /* Define to 1 if you have the <unistd.h> header file. */ >>> #mesondefine HAVE_UNISTD_H >>> >>> +/* Define to 1 if you have the <iniparser/iniparser.h> header file. */ >>> +#mesondefine HAVE_INIPARSER_INCLUDE_H >>> + >>> /* Define to 1 if using libuuid */ >>> #mesondefine HAVE_UUID >>> >>> diff --git a/meson.build b/meson.build >>> index 42e11aa25cba..893f70c22270 100644 >>> --- a/meson.build >>> +++ b/meson.build >>> @@ -176,6 +176,7 @@ check_headers = [ >>> ['HAVE_SYS_STAT_H', 'sys/stat.h'], >>> ['HAVE_SYS_TYPES_H', 'sys/types.h'], >>> ['HAVE_UNISTD_H', 'unistd.h'], >>> + ['HAVE_INIPARSER_INCLUDE_H', 'iniparser/iniparser.h'], >>> ] >>> >>> foreach h : check_headers >>> diff --git a/util/parse-configs.c b/util/parse-configs.c >>> index c834a07011e5..8bdc9d18cbf3 100644 >>> --- a/util/parse-configs.c >>> +++ b/util/parse-configs.c >>> @@ -4,7 +4,11 @@ >>> #include <dirent.h> >>> #include <errno.h> >>> #include <fcntl.h> >>> +#ifdef HAVE_INIPARSER_INCLUDE_H >>> +#include <iniparser/iniparser.h> >>> +#else >>> #include <iniparser.h> >>> +#endif >>> #include <sys/stat.h> >>> #include <util/parse-configs.h> >>> #include <util/strbuf.h> >> >> Agree, above patch can fix this issue. Though I really wanted to avoid >> trickling changes to the parse-configs.c since the real problem is with >> non consistent location for iniparser.h header across distros. >> >> I gave it some thought and came up with this patch to meson.build that can >> pick up appropriate '/usr/include' or '/usr/include/iniparser' directory >> as include path to the compiler. >> >> Also since there seems to be no standard location for this header file >> I have included a meson build option named 'iniparser-includedir' that >> can point to the dir where 'iniparser.h' can be found. >> >> diff --git a/meson.build b/meson.build >> index 42e11aa25cba..8c347e64ca2d 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -158,9 +158,27 @@ endif >> >> cc = meson.get_compiler('c') >> >> -# keyutils and iniparser lack pkgconfig >> +# keyutils lack pkgconfig >> keyutils = cc.find_library('keyutils', required : get_option('keyutils')) >> -iniparser = cc.find_library('iniparser', required : true) >> + >> +# iniparser lacks pkgconfig and its header files are either at >> '/usr/include' or '/usr/include/iniparser' >> +# First use the path provided by user via meson configure >> -Diniparser-includedir=<somepath> >> +# if thats not provided than try searching for 'iniparser.h' in default >> system include path >> +# and if that not found than as a last resort try looking at >> '/usr/include/iniparser' >> + >> +if get_option('iniparser-includedir') == '' >> + iniparser = cc.find_library('iniparser', required : false, has_headers : >> 'iniparser.h') >> + # if not available at the default path try '/usr/include/iniparser' >> + if not iniparser.found() >> + iniparser = cc.find_library('iniparser', required : true, has_headers : >> 'iniparser/iniparser.h') >> + iniparser = >> declare_dependency(include_directories:'/usr/include/iniparser', >> dependencies:iniparser) >> + endif >> +else >> + iniparser_incdir = include_directories(get_option('iniparser-includedir')) >> + iniparser = cc.find_library('iniparser', required : true, has_headers : >> 'iniparser.h', >> + >> header_include_directories:iniparser_incdir) >> + iniparser = declare_dependency(include_directories:iniparser_incdir, >> dependencies:iniparser) >> +endif >> >> conf = configuration_data() >> check_headers = [ >> diff --git a/meson_options.txt b/meson_options.txt >> index aa4a6dc8e12a..d08151691553 100644 >> --- a/meson_options.txt >> +++ b/meson_options.txt >> @@ -23,3 +23,5 @@ option('pkgconfiglibdir', type : 'string', value : '', >> description : 'directory for standard pkg-config files') >> option('bashcompletiondir', type : 'string', >> description : '''${datadir}/bash-completion/completions''') >> +option('iniparser-includedir', type : 'string', >> + description : '''Path containing the iniparser header files''') >> >> > > > Looks good. Can you send this as a patch? > > -aneesh -- Cheers ~ Vaibhav
