[PATCH v3 27/30] scripts: apibuild: parse 'Since' for functions

Thu Apr 21 18:03:02 UTC 2022

On Thu, Apr 21, 2022 at 01:56:19PM +0200, Peter Krempa wrote:
> On Wed, Apr 20, 2022 at 21:08:16 +0200, Victor Toso wrote:
> > This patch adds 'version' parameter to generated XML API for functions
> > and functypes.
> > 
> > The 'version' metadata has been added with e0e0bf6628 by parsing .syms
> > files. This commit does not override that but it will warn if there is
> > not 'Since' metadata with new additions.
> > 
> > There is not clear benefit for keeping both. For now, I've added a
> > warning in case there is a mismatch between the version provided by
> > .syms and docstring.
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  scripts/apibuild.py | 126 +++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 113 insertions(+), 13 deletions(-)
> 
> Let's review the outliers and then the script which generated the rest:
> 
> > diff --git a/scripts/apibuild.py b/scripts/apibuild.py
> > index b77eea0624..ec10931151 100755
> > --- a/scripts/apibuild.py
> > +++ b/scripts/apibuild.py
> > @@ -111,6 +111,73 @@ ignored_functions = {
> >      "virErrorCopyNew": "private",
> >  }
> >  
> > +# The version in the .sym file might differnt from
> > +# the real version that the function was introduced.
> > +# This dict's value is the correct version, as it should
> > +# be in the docstrings.
> > +ignored_function_versions = {
> > +    'virDomainSetBlockThreshold': '3.2.0',
> 
> We've discussed this one.
> 
> > +    'virGetLastErrorMessage': '1.0.5.2',
> 
> Your script seems to have picked a stable release which we did at some
> point. The v1.0.6 where the symbol is exported is correct. This
> would be also the only outlier which has a 4 digit version
> string.
> 
> So the Since tag needs to be fixed and this removed.

Ack!

> > +    'virNodeDeviceCreate': '0.5.0',
> 
> This is an unfortunate side-effect of grepping in the
> repository rather than looking for actual code.
> 
> 'virNodeDeviceCreate' was indeed mentioned in a comment in the
> version you've added here, but was in fact implemented at the
> time it was actually exported.

Ack!

> This entry needs to be removed and the Since tag fixed.
> 
> 
> > +    'virAdmClientClose': '1.3.5',
> > +    'virAdmClientFree': '1.3.5',
> > +    'virAdmClientGetID': '1.3.5',
> > +    'virAdmClientGetInfo': '1.3.5',
> > +    'virAdmClientGetTimestamp': '1.3.5',
> > +    'virAdmClientGetTransport': '1.3.5',
> > +    'virAdmConnectClose': '1.2.17',
> > +    'virAdmConnectGetLibVersion': '1.3.1',
> > +    'virAdmConnectGetURI': '1.3.1',
> > +    'virAdmConnectIsAlive': '1.3.1',
> > +    'virAdmConnectListServers': '1.3.2',
> > +    'virAdmConnectLookupServer': '1.3.3',
> > +    'virAdmConnectOpen': '1.2.17',
> > +    'virAdmConnectRef': '1.2.17',
> > +    'virAdmConnectRegisterCloseCallback': '1.3.1',
> > +    'virAdmConnectUnregisterCloseCallback': '1.3.1',
> > +    'virAdmGetVersion': '1.3.0',
> > +    'virAdmServerFree': '1.3.2',
> > +    'virAdmServerGetClientLimits': '1.3.5',
> > +    'virAdmServerGetName': '1.3.2',
> > +    'virAdmServerGetThreadPoolParameters': '1.3.4',
> > +    'virAdmServerListClients': '1.3.5',
> > +    'virAdmServerLookupClient': '1.3.5',
> > +    'virAdmServerSetClientLimits': '1.3.5',
> > +    'virAdmServerSetThreadPoolParameters': '1.3.4',
> 
> All of the above should be removed and the Since tag should say v2.0.0
> to match the symbol as that's the first point at which we've exported
> these.

Ack!

> 
> > +    'virAdmServerUpdateTlsFiles': '6.2.0',
> 
> This one is needed as it's a mistake in the symbol export.

Ack!

> > +    'virConnectFindStoragePoolSources': '0.4.6',
> 
> This is a mistake in our repo. The 'v0.4.5' tag is missing but the
> function was added and exported in time of v0.4.5.
> 
> Remove this entry and fix the since tag.

Yes, this was more an issue in my side as I saw no v0.4.5 tag, I
probably adjusted it myself. I'll fix it.

> > +    'virConnectNumOfDefinedDomains': '0.1.6',
> > +    'virConnectOpenAuth': '0.4.1',
> 
> Same as above.
> 
> > +    'virDomainBlockPeek': '0.4.4',
> > +    'virDomainMemoryPeek': '0.4.4',
> 
> So in this case the code was added indeed post v0.4.2 release but the
> symbol was exported in the previous one, although it happened prior to
> v0.4.3 was released, but our repo is missing the tag.
> 
> So correctly both should be 0.4.3.

Ack.

> 
> > +    'virNetworkUpdate': '1.0.0',
> 
> Not sure what happened here, but v0.10.2 what the script
> detects in the symbol seems to be correct.

Likely a leftover from fixing the 1.0.0 versions. I'll fix it.

> > +    'virConnectClose': '0.0.1',
> > +    'virConnectGetType': '0.0.1',
> > +    'virConnectGetVersion': '0.0.1',
> > +    'virConnectListDomains': '0.0.1',
> > +    'virConnectNumOfDomains': '0.0.1',
> > +    'virConnectOpen': '0.0.1',
> > +    'virConnectOpenReadOnly': '0.0.1',
> > +    'virDomainCreateLinux': '0.0.1',
> > +    'virDomainDestroy': '0.0.1',
> > +    'virDomainFree': '0.0.1',
> > +    'virDomainGetID': '0.0.1',
> > +    'virDomainGetInfo': '0.0.1',
> > +    'virDomainGetMaxMemory': '0.0.1',
> > +    'virDomainGetName': '0.0.1',
> > +    'virDomainGetOSType': '0.0.1',
> > +    'virDomainGetXMLDesc': '0.0.1',
> > +    'virDomainLookupByID': '0.0.1',
> > +    'virDomainLookupByName': '0.0.1',
> > +    'virDomainRestore': '0.0.2',
> > +    'virDomainResume': '0.0.1',
> > +    'virDomainSave': '0.0.2',
> > +    'virDomainSetMaxMemory': '0.0.1',
> > +    'virDomainShutdown': '0.0.1',
> > +    'virDomainSuspend': '0.0.1',
> > +    'virGetVersion': '0.0.1',
> 
> Now for these you should follow again when the symbol was exported, many
> of these functions are mentioned in TODO first and implemented later.
> 
> As noted previously the Since tag should be the maximum version of
> following two:
>     1) when the symbol was exported
>     2) when it was implemented.
> 
> In the end I expect this list to be only contain:
> 
> 'virDomainSetBlockThreshold'
> 'virAdmServerUpdateTlsFiles'
> 'virDomainBlockPeek'
> 'virDomainMemoryPeek'

Many thanks for the careful review.
I'll have all those fixes by v4.

Cheers,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220421/c3119bda/attachment.sig>