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

Michal Prívozník mprivozn at redhat.com
Fri Sep 4 13:30:14 UTC 2020


On 9/4/20 2:21 PM, Andrea Bolognani wrote:
> On Fri, 2020-09-04 at 11:50 +0200, Michal Prívozník wrote:
>> On 9/4/20 10:33 AM, Andrea Bolognani wrote:
>>> 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?
>>
>> I had this patched marked for review but did not get to it yesterday, sorry.
> 
> No need to apologise :)
> 
>> I understand your reasoning and agree that the current code looks bad.
>> But this is just the way it used to be when you wanted to built plugins
>> outside of wireshark. I had a long discussion with wireshark devels when
>> our plugin was being written. Part of them did not want other projects
>> to build plugins from outside at all (which didn't really work for us
>> because our RPC was changing a lot back then and we would have to wait
>> full release cycle of wireshark to dissect new procedures), and part was
>> at least willing to merge my patches that made our code less horrible.
>>
>> And truth to be told, I did not really watch the discussion on wireshark
>> end since and with your patch it looks they moved towards making it
>> easier for other to build plugins out of wireshark's source.
>>
>> End of history class.
>>
>> The patch looks good. ws_version.h was introduced with 2.9.0 release
>> which is 1.5 years old. Given that the dissector is aimed mostly on us,
>> developers to help us debug RPC issues, I think we can safely bump the
>> minimal wireshark version required (currently 2.4.0 which is 3 years old).
> 
> That sounds reasonable in theory, but if you look at
> 
>    https://gitlab.com/abologna/libvirt/-/pipelines/185421025
> 
> you'll see that even platforms that ship pretty recent Wireshark[1]
> don't include ws_version.h among the headers.
> 
> Not building the dissector on those non-obsolete platforms seems
> excessively harsh, so I think an approach similar to the one I
> described above is still necessary. And at that point, you might as
> well not bump the minimum required version and keep building the
> dissector on the current list of platforms...
> 
> 
> [1] Debian sid has 3.2.6 and Ubuntu 20.04 has 3.2.3

Any idea why they are not installing the file? Because while current 
solution is hacky, intentionally removing a header file that a package 
wants installed is way worse.

Michal




More information about the libvir-list mailing list