Re: [libvirt PATCH] wireshark: Don't include config.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2020-09-03 at 12:16 -0400, Laine Stump wrote:
> On 9/3/20 6:43 AM, Andrea Bolognani wrote:
> > While both Debian and Fedora include the header in their development
> > packages for Wireshark, that's not something that the upstream
> > developers intended and arguably quite wrong, as config.h is obviously
> > intended to only be used to drive the compilation of Wireshark itself.
> > The Arch Linux package behaves like the upstream Wireshark package, and
> > thus libvirt fails to build there.
> > 
> > It seems that there are multiple bugs to be addressed:
> > 
> >    * libvirt shouldn't include config.h;
> > 
> >    * Debian and Fedora shouldn't be shipping config.h in their Wireshark
> >      packages;
> > 
> >    * Wireshark should not use config.h defines such as HAVE_PLUGINS in
> >      its public headers, and define a public variant of them instead.
> > 
> > This patch takes care of the first one.
> > 
> > https://gitlab.com/libvirt/libvirt/-/issues/74
> > Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> 
> Funny, just this morning I was grepping for " HAVE_" and " WITH_" in 
> /usr/include, saw a couple of "config.h"s shoot past, and thought "Hmm. 
> That doesn't seem right! Surely those packages didn't *really* intend to 
> ship their config.h with their -dev package".
> 
> 
> Reviewed-by: Laine Stump <laine@xxxxxxxxxx>

I should have pushed the branch to GitLab first:

  https://gitlab.com/abologna/libvirt/-/pipelines/185421025

We might be able to adopt a more nuanced approach, where we use
ws_version.h where available and fall back to config.h where it's the
only option, but I'm not quite sure how to implement that in Meson
and can't spare the time to learn right now.

Are either you or Michal interested in giving it a try?

-- 
Andrea Bolognani / Red Hat / Virtualization




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux