Re: [libvirt] [PATCH] Add support for VirtualBox 3.2
2010/5/27 Eric Blake <eblake(a)redhat.com>:
On 05/27/2010 02:21 PM, Matthias Bolte wrote:
>> Is this generator something that should be run as part of building
>> libvirt, or are we okay with checking in just the generated file?
>
> He probably didn't generate the file. Those headers are part of the
> VirtualBox SDK. As the comment indicates they can also be generated
> from the VirtualBox.xidl file using a XSLT transformation.
How likely is this file to change? And are those changes likely to be
manual edits or re-running a generator?
The vbox_CAPI_*.h files we already have in GIT will never change, as
they reflect the released COM API for different VirtualBox version.
The VirtualBox COM API will probably change in future versions, that's
why we have the different headers and detect at runtime what version
of the VirtualBOX driver to use. But we won't change the existing
headers, instead we add new headers for future VirtualBox versions.
>> Lines like this fail the cppi syntax-check. Either we need
to modify
>> cfg.mk to exempt this file from cppi checks, or you need to fix the
>> generation process to filter the file through cppi before committing it
>> into libvirt.
>>
>
> I just tried to make the vbox_CAPI_v3_2.h conform to cppi's checks,
> but the patch touches ~500 lines. Therefore, the less invasive fix
> would be to just ignore the vbox_CAPI_*.h headers for cppi checks.
You mean you tried this by hand? Yeah, that would be painful. Instead,
you can use cppi to touch those 500 lines in a completely automated way,
in less than a second (see commit 36d8e7d):
I basically did a s/#/# / and fixed the ~10 remaining wrong lines by
hand, so not that painful.
for f in vbox_CAPI_*.h; do cppi $f > $f.t && mv $f.t $f;
done
At which point, if the header comment about being generated is
copy-and-paste and not indicative of how we will be maintaining the
file, I'd almost feel better reformatting the file than adding the
exemption (and maybe even adding a comment clarifying the 'this file is
generated' comment).
We do not actually generate those headers, but just included them from
the VirtualBox SDK.
Yes, I think that's a good idea. We should extend the 'This is a
generated file.' comment saying that we did edit them and you should
not regenerate them because you don't need to.
Matthias