diff mbox series

util/parse: Fix build error on ubuntu

Message ID 20220315060426.140201-1-aneesh.kumar@linux.ibm.com (mailing list archive)
State New
Headers show
Series util/parse: Fix build error on ubuntu | expand

Commit Message

Aneesh Kumar K V March 15, 2022, 6:04 a.m. UTC
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(-)

Comments

Vaibhav Jain March 15, 2022, 8:55 a.m. UTC | #1
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
>
>
Aneesh Kumar K V March 15, 2022, 10:15 a.m. UTC | #2
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>
Vaibhav Jain March 15, 2022, 4:46 p.m. UTC | #3
"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''')
Verma, Vishal L March 15, 2022, 5:03 p.m. UTC | #4
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''')
> 
>
Dan Williams March 15, 2022, 5:19 p.m. UTC | #5
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.
Aneesh Kumar K V March 16, 2022, 3:50 a.m. UTC | #6
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
Vaibhav Jain March 16, 2022, 5:33 a.m. UTC | #7
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 mbox series

Patch

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>