On Tue, Mar 15, 2022 at 9:47 AM Vaibhav Jain <[email protected]> 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') == ''

Approach looks good, but I think I would change this to be relative to
includedir as in:

iniparserdir = get_option(iniparserdir)
iniparser = cc.find_library('iniparser', required : false, has_headers
: 'iniparser.h', dirs : includedir / iniparserdir)
if not iniparser.found()
    initparserdir = 'iniparser'
    iniparser = cc.find_library('iniparser', required : true,
has_headers : 'iniparser.h', dirs : includedir / iniparserdir)
endif
iniparser = declare_dependency(include_directories : includedir /
iniparserdir, dependencies:iniparser)

...just in case someone has already overridden @prefix and / or @includedir.

Reply via email to