1. Patchew
  2. QEMU
  3. [PATCH 00/22] Convert more checks to Meson
  4. View patch

[PATCH 15/26] meson: sort header tests

Paolo Bonzini posted 22 patches 4 years, 8 months ago
[PATCH 15/26] meson: sort header tests
Posted by Paolo Bonzini 4 years, 8 months ago 
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 28825dec18..5fdb757751 100644
--- a/meson.build
+++ b/meson.build
@@ -1258,16 +1258,17 @@ config_host_data.set('QEMU_VERSION_MAJOR', meson.project_version().split('.')[0]
 config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1])
 config_host_data.set('QEMU_VERSION_MICRO', meson.project_version().split('.')[2])
 
+config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
+
 config_host_data.set('HAVE_BTRFS_H', cc.has_header('linux/btrfs.h'))
 config_host_data.set('HAVE_DRM_H', cc.has_header('libdrm/drm.h'))
 config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
+config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
 config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
 config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
-config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
-config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
-config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
 
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
+config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
 
 ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 'CONFIG_BDRV_RO_WHITELIST']
-- 
2.31.1
Re: [PATCH 15/26] meson: sort header tests
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago 
On 6/8/21 1:22 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Re: [PATCH 15/26] meson: sort header tests
Posted by Daniel P. Berrangé 4 years, 7 months ago 
On Tue, Jun 08, 2021 at 01:22:50PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 28825dec18..5fdb757751 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1258,16 +1258,17 @@ config_host_data.set('QEMU_VERSION_MAJOR', meson.project_version().split('.')[0]
>  config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1])
>  config_host_data.set('QEMU_VERSION_MICRO', meson.project_version().split('.')[2])
>  
> +config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
> +
>  config_host_data.set('HAVE_BTRFS_H', cc.has_header('linux/btrfs.h'))
>  config_host_data.set('HAVE_DRM_H', cc.has_header('libdrm/drm.h'))
>  config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
> +config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
>  config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
>  config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
> -config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
> -config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
> -config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
>  
>  config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
> +config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))

Was tis supposde to be sorting based on header name or cpp macro name ?

Either way, IIUC, this is in the wrong place because "HAVE_SYSTEM"
would be before CONFIG_PREADV, and stdlib.h is before sys/uio.h


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 15/26] meson: sort header tests
Posted by Paolo Bonzini 4 years, 7 months ago 
On 15/06/21 16:47, Daniel P. Berrangé wrote:
> On Tue, Jun 08, 2021 at 01:22:50PM +0200, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   meson.build | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 28825dec18..5fdb757751 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1258,16 +1258,17 @@ config_host_data.set('QEMU_VERSION_MAJOR', meson.project_version().split('.')[0]
>>   config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1])
>>   config_host_data.set('QEMU_VERSION_MICRO', meson.project_version().split('.')[2])
>>   
>> +config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
>> +
>>   config_host_data.set('HAVE_BTRFS_H', cc.has_header('linux/btrfs.h'))
>>   config_host_data.set('HAVE_DRM_H', cc.has_header('libdrm/drm.h'))
>>   config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
>> +config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
>>   config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
>>   config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
>> -config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
>> -config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
>> -config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
>>   
>>   config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
>> +config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
> 
> Was tis supposde to be sorting based on header name or cpp macro name ?
> 
> Either way, IIUC, this is in the wrong place because "HAVE_SYSTEM"
> would be before CONFIG_PREADV, and stdlib.h is before sys/uio.h

Based on macro name.  HAVE_SYSTEM_FUNCTION is sorted after CONFIG_PREADV 
among the users of has_function.  I'll explain this better in the commit 
message.

Paolo
Re: [PATCH 15/26] meson: sort header tests
Posted by Daniel P. Berrangé 4 years, 7 months ago 
On Tue, Jun 15, 2021 at 05:16:48PM +0200, Paolo Bonzini wrote:
> On 15/06/21 16:47, Daniel P. Berrangé wrote:
> > On Tue, Jun 08, 2021 at 01:22:50PM +0200, Paolo Bonzini wrote:
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > >   meson.build | 7 ++++---
> > >   1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 28825dec18..5fdb757751 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -1258,16 +1258,17 @@ config_host_data.set('QEMU_VERSION_MAJOR', meson.project_version().split('.')[0]
> > >   config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1])
> > >   config_host_data.set('QEMU_VERSION_MICRO', meson.project_version().split('.')[2])
> > > +config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
> > > +
> > >   config_host_data.set('HAVE_BTRFS_H', cc.has_header('linux/btrfs.h'))
> > >   config_host_data.set('HAVE_DRM_H', cc.has_header('libdrm/drm.h'))
> > >   config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
> > > +config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
> > >   config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
> > >   config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
> > > -config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
> > > -config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
> > > -config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
> > >   config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
> > > +config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
> > 
> > Was tis supposde to be sorting based on header name or cpp macro name ?
> > 
> > Either way, IIUC, this is in the wrong place because "HAVE_SYSTEM"
> > would be before CONFIG_PREADV, and stdlib.h is before sys/uio.h
> 
> Based on macro name.  HAVE_SYSTEM_FUNCTION is sorted after CONFIG_PREADV
> among the users of has_function.  I'll explain this better in the commit
> message.

Oh, so you're attempting to group the checks first by 'has_header'
and 'has_function', and then alpha based on macro name within the
group.


Probably worth putting comments inline in the meson.build explicitly
marking the start of each group of checks. Future people modifying
the file won't look at the commit message and are going to find it
hard to figure out the sorting used without inline comment hints

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.