Re: [virt-tools-list] [libosinfo v2 1/2] Add a private header

[Eric Blake]

On 10/30/2012 01:24 PM, Zeeshan Ali (Khattak) wrote:
> On Tue, Oct 30, 2012 at 4:18 PM, Michal Privoznik <address@hidden> wrote:
>> On 30.10.2012 14:52, Zeeshan Ali (Khattak) wrote:
>> diff --git a/cfg.mk b/cfg.mk
>> index 6aaa394..b90abed 100644
>> --- a/cfg.mk
>> +++ b/cfg.mk
>> @@ -134,8 +134,8 @@ exclude_file_name_regexp--sc_preprocessor_indentation = 
>> ^*/*.[ch]
>>
>>  exclude_file_name_regexp--sc_prohibit_strcmp = ^*/*.[ch]
>>
>> -exclude_file_name_regexp--sc_require_config_h = python/libvirt-glib.c
>> -exclude_file_name_regexp--sc_require_config_h_first = python/libvirt-glib.c
>> +exclude_file_name_regexp--sc_require_osinfo_priv_h = python/libvirt-glib.c
>> +exclude_file_name_regexp--sc_require_osinfo_priv_h_first = 
>> python/libvirt-glib.c
>>
>>  exclude_file_name_regexp--sc_trailing_blank = 
>> ^test/isodata/.*|docs/object-model.fig
>>
>> diff --git a/maint.mk b/maint.mk
>> index de7a303..6349d7e 100644
>> --- a/maint.mk
>> +++ b/maint.mk
>> @@ -432,29 +432,29 @@ sc_prohibit_have_config_h:
>>         halt='found use of HAVE''_CONFIG_H; remove'                     \
>>           $(_sc_search_regexp)
>>
>> -# Nearly all .c files must include <config.h>.  However, we also permit this
>> +# Nearly all .c files must include <osinfo_priv.h>.  However, we also 
>> permit this

NACK to this hunk - you want maint.mk to mirror upstream, for easiest
maintenance, and stick libosinfo-specific tweaks into cfg.mk.

>>  # via inclusion of a package-specific header, if cfg.mk specified one.
>> -# config_h_header must be suitable for grep -E.
> 
> The 'However' part of the original comment here makes me wonder if we
> need such an intrusive patch?

>> -config_h_header ?= <config\.h>

This is the kingpin line - add a definition of config_h_header in your
cfg.mk, and then maint.mk can be used unchanged but with your
alternative spelling of your common header (and also make sure your
osinfo/osinfo_priv.h includes config.h first).

>> -sc_require_config_h:
>> -       @require='^# *include $(config_h_header)'                       \
>> +# osinfo_priv_h_header must be suitable for grep -E.
>> +osinfo_priv_h_header ?= <osinfo/osinfo_priv\.h>
>> +sc_require_osinfo_priv_h:
>> +       @require='^# *include $(osinfo_priv_h_header)'                  \
>>         in_vc_files='\.c$$'                                             \
>> -       halt='the above files do not include <config.h>'                \
>> +       halt='the above files do not include <osinfo/osinfo_priv.h>'         
>>    \

Don't change these - they will get undone the next time we resync from
upstream gnulib.

Oh, that points out an upstream gnulib bug (cc'd) - the error message
here mentions <config.h> even when config_h_header has been customized.
 I wonder if it is worth fixing in gnulib.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature