Hi, On Tue, Apr 05, 2022 at 04:52:10PM +0200, Michal Prívozník wrote:
On 4/5/22 13:43, Victor Toso wrote:
Hi,
This series is an extension of what Daniel did in e0e0bf6628 "scripts: include function versions in API definition".
The main motivation behind this is to help code generators to consider if a given symbol is present in a given version of libvirt that they are either running/building against, more specifically:
https://gitlab.com/libvirt/libvirt-go-module/-/merge_requests/7
As headers are already a great source of documentation for developers, I'm suggesting to add a specific comment to each of this exported types:
/* ... Since <version> */
For the use case I mentioned above, I'm adding small parsing function in apibuild.py to fetch the above metadata and included it on the generated XML API.
To avoid adding too much noise in the githistory, I'm proposing the addition of symbols.versions.allowlist file, that apibuild.py can use to fetch the first git tag that a given symbol appeared in. I did a small script to generate it, but it sums up to calling the bellow command for any tag that starts with 'v' and has not '-rc'.
git grep $symbol $tag ./include
This gives you the latest tag that a symbol was touched in. You need to look at the very first commit that introduced the symbol.
Looking to each commit takes a few hours, that's why I took the approach at looking per tag instead.
For instance, in your allowlist file you have:
virConnectDomainEventDeregister v0.10.0
but src/libvirt_public.syms lists this symbol in 0.5.0 release:
LIBVIRT_0.5.0 { global: ... virConnectDomainEventDeregister;
Good catch! I think the problem here is that when I run: git tag --list 'v*' | grep -v 'rc' the order that I got was ... v0.1.8 v0.1.9 v0.10.0 v0.10.1 v0.10.2 v0.10.2.1 v0.10.2.2 v0.10.2.3 v0.10.2.4 v0.10.2.5 v0.10.2.6 v0.10.2.7 v0.10.2.8 v0.2.0 v0.2.1 v0.2.2 ... And I tested in that order, so v0.10.0 was tested prior to v0.5.0. I can see that git grep works with v0.5.0. but not with v0.4.6, as expected. I'll fix it and send a v2 later. Many thanks!
I'm trying the simples approach I could find but I'm happy to improve this further based on your suggestions.
Ah, the diff in the generated XMLs, per patch (sadly, pasteben drops it after a single day). The initial reference is current master 67c77744d7 'tests: Fixing compiler warning in cputest'
0001 (enums) ... https://paste.centos.org/view/e9137ef0 0002 (typedefs) https://paste.centos.org/view/76f0b397 0003 (macros) .. https://paste.centos.org/view/b68fb03a 0004 (variables) https://paste.centos.org/view/4a20d9bb
Cheers, Victor
Victor Toso (4): scripts: apibuild: parse 'Since' version for enums scripts: apibuild: parse 'Since' for typedefs scripts: apibuild: parse 'Since' for macros scripts: apibuild: add 'version' to variables
scripts/apibuild.py | 68 +- symbols.versions.allowlist | 2144 ++++++++++++++++++++++++++++++++++++ 2 files changed, 2199 insertions(+), 13 deletions(-) create mode 100644 symbols.versions.allowlist
I like this. It's not only for Golang bindings, but other bindings might benefit from this as well. And also developers reading the docs (they see immediately what version was the symbol they are looking at introduced in).
Having said that, maybe we should just add 'Since ...' to every symbol in include/**\.h instead of having it on a side then? If not, then I think scripts/ or docs/ is better location for the allowlist file.
Moreover, the allowlist file now contains
Michal