Re: [PATCH v2] capstone: use <capstone/capstone.h> instead of <capstone.h>

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Nov 14, 2022 at 11:59:31AM +0000, Peter Maydell wrote:
> On Sun, 13 Nov 2022 at 20:10, Michael Tokarev <mjt@tls.msk.ru> wrote:
> >
> > The upcoming capstone 5.0 drops support for the old way
> > of including its header, due to this change:
> > https://github.com/capstone-engine/capstone/commit/6656bcb63ab4e87dc6079bd6b6b12cc8dd9b2ad8
> > The official way is to use <capstone/capstone.h>
> >
> > This change has already been proposed before, see
> > https://patchwork.kernel.org/project/qemu-devel/patch/20180215173539.11033-1-f4bug@amsat.org/
> > but it didn't find its way into qemu at that time.
> >
> > On current systems, using <capstone/capstone.h> works
> > now (despite the pkg-config-supplied -I/usr/include/capstone) -
> > since on all systems capstone headers are put into capstone/
> > subdirectory of a system include dir. So this change is
> > compatible with both the obsolete way of including it
> > and the only future way.
> 
> That's only true if capstone happened to be installed
> into a system include directory subdirectory. That
> is probably true for most distros, but it isn't
> necessarily true when an end user has built and
> installed capstone locally themselves.
> 
> In other words, this is a breaking non-back-compatible
> change by capstone upstream, which we now need to work
> around somehow :-(
> 
> 
> > diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> > index e29068dd97..d8fdc5d537 100644
> > --- a/include/disas/capstone.h
> > +++ b/include/disas/capstone.h
> > @@ -3,7 +3,7 @@
> >
> >  #ifdef CONFIG_CAPSTONE
> >
> > -#include <capstone.h>
> > +#include <capstone/capstone.h>
> >
> >  #else
> >
> > diff --git a/meson.build b/meson.build
> > index cf3e517e56..6f34c963f7 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2680,12 +2680,7 @@ if not get_option('capstone').auto() or have_system or have_user
> >    capstone = dependency('capstone', version: '>=3.0.5',
> >                          kwargs: static_kwargs, method: 'pkg-config',
> >                          required: get_option('capstone'))
> > -
> > -  # Some versions of capstone have broken pkg-config file
> > -  # that reports a wrong -I path, causing the #include to
> > -  # fail later. If the system has such a broken version
> > -  # do not use it.
> > -  if capstone.found() and not cc.compiles('#include <capstone.h>',
> > +  if capstone.found() and not cc.compiles('#include <capstone/capstone.h>',
> >                                            dependencies: [capstone])
> >      capstone = not_found
> >      if get_option('capstone').enabled()
> 
> We can do something like
> 
> config_host_data.set('HAVE_CAPSTONE_CAPSTONE_H',
> cc.has_header('capstone/capstone.h', depedencies: [capstone])
> 
> to check that this capstone really does have capstone/capstone.h,
> for instance.
> 
> Dan: is there a reason why in commit 8f4aea712ffc4 you wrote
> the "check that capstone.h really exists" check with cc.compiles
> rather than cc.has_header ?

I was probably just unaware of 'has_header' existing

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