Re: [PATCH v1 0/4] Add 'version' to other exported types

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

 



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
> 

Attachment: signature.asc
Description: PGP signature


[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