Re: [PATCH 15/26] meson: sort header tests

[Daniel P . Berrangé]

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 :|