| Message ID | 20220315060426.140201-1-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | util/parse: Fix build error on ubuntu | expand |
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
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Fix the below build error on ubuntu:
> ../util/parse-configs.c:7:10: fatal error: iniparser.h: No such file or directory
> 7 | #include <iniparser.h>
> | ^~~~~~~~~~~~~
>
> The same error is not observed on other OS because they do create symlinks as
> below
>
> lrwxrwxrwx. 1 root root 21 Jul 22 2021 /usr/include/iniparser.h -> iniparser/iniparser.h
>
> the error can be avoided by using the correct include path. Also, catch the error
> during setup instead of the build by adding the check for meson.build
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> meson.build | 2 +-
> util/parse-configs.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 42e11aa25cba..a4c4c1cd3df3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -160,7 +160,7 @@ cc = meson.get_compiler('c')
>
> # keyutils and iniparser lack pkgconfig
> keyutils = cc.find_library('keyutils', required : get_option('keyutils'))
> -iniparser = cc.find_library('iniparser', required : true)
> +iniparser = cc.find_library('iniparser', required : true, has_headers: 'iniparser/iniparser.h')
>
> conf = configuration_data()
> check_headers = [
> diff --git a/util/parse-configs.c b/util/parse-configs.c
> index c834a07011e5..1b7ffa69f05f 100644
> --- a/util/parse-configs.c
> +++ b/util/parse-configs.c
> @@ -4,7 +4,7 @@
> #include <dirent.h>
> #include <errno.h>
> #include <fcntl.h>
> -#include <iniparser.h>
> +#include <iniparser/iniparser.h>
> #include <sys/stat.h>
> #include <util/parse-configs.h>
> #include <util/strbuf.h>
> --
> 2.35.1
>
>
Vaibhav Jain <vaibhav@linux.ibm.com> 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>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > Vaibhav Jain <vaibhav@linux.ibm.com> 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''')
On Tue, 2022-03-15 at 22:16 +0530, Vaibhav Jain wrote: > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > > > Vaibhav Jain <vaibhav@linux.ibm.com> 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. > > Sigh, yeah, that's an unfortunate situation. > > > > 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. This approach sounds fine, do you want to send it as a proper patch. > > 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 s/lack/lacks/ > 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''') > >
On Tue, Mar 15, 2022 at 9:47 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote: > > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > > > Vaibhav Jain <vaibhav@linux.ibm.com> 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.
On 3/15/22 10:16 PM, Vaibhav Jain wrote: > "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > >> Vaibhav Jain <vaibhav@linux.ibm.com> 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
Thanks for the feedback Vishal, Dan and Aneesh I have sent out a patch based Dan's suggestion at https://lore.kernel.org/nvdimm/20220316053030.2954642-1-vaibhav@linux.ibm.com Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> writes: > On 3/15/22 10:16 PM, Vaibhav Jain wrote: >> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: >> >>> Vaibhav Jain <vaibhav@linux.ibm.com> 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
diff --git a/meson.build b/meson.build index 42e11aa25cba..a4c4c1cd3df3 100644 --- a/meson.build +++ b/meson.build @@ -160,7 +160,7 @@ cc = meson.get_compiler('c') # keyutils and iniparser lack pkgconfig keyutils = cc.find_library('keyutils', required : get_option('keyutils')) -iniparser = cc.find_library('iniparser', required : true) +iniparser = cc.find_library('iniparser', required : true, has_headers: 'iniparser/iniparser.h') conf = configuration_data() check_headers = [ diff --git a/util/parse-configs.c b/util/parse-configs.c index c834a07011e5..1b7ffa69f05f 100644 --- a/util/parse-configs.c +++ b/util/parse-configs.c @@ -4,7 +4,7 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> -#include <iniparser.h> +#include <iniparser/iniparser.h> #include <sys/stat.h> #include <util/parse-configs.h> #include <util/strbuf.h>
Fix the below build error on ubuntu: ../util/parse-configs.c:7:10: fatal error: iniparser.h: No such file or directory 7 | #include <iniparser.h> | ^~~~~~~~~~~~~ The same error is not observed on other OS because they do create symlinks as below lrwxrwxrwx. 1 root root 21 Jul 22 2021 /usr/include/iniparser.h -> iniparser/iniparser.h the error can be avoided by using the correct include path. Also, catch the error during setup instead of the build by adding the check for meson.build Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- meson.build | 2 +- util/parse-configs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)