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@redhat.com>
---
scripts/apibuild.py | 126 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 113 insertions(+), 13 deletions(-)
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',
+ 'virGetLastErrorMessage': '1.0.5.2',
+ 'virNodeDeviceCreate': '0.5.0',
+ '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',
+ 'virAdmServerUpdateTlsFiles': '6.2.0',
+ 'virConnectFindStoragePoolSources': '0.4.6',
+ 'virConnectNumOfDefinedDomains': '0.1.6',
+ 'virConnectOpenAuth': '0.4.1',
+ 'virDomainBlockPeek': '0.4.4',
+ 'virDomainMemoryPeek': '0.4.4',
+ 'virNetworkUpdate': '1.0.0',
+ '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',
+}
+
ignored_macros = {
"_virSchedParameter": "backward compatibility macro for virTypedParameter",
"_virBlkioParameter": "backward compatibility macro for virTypedParameter",
@@ -2185,9 +2252,10 @@ class docBuilder:
self.scanVersions()
# Fetch tags from the comment. Only 'Since' supported at the moment.
- # Return the tags and the original comment without the tags.
- def retrieve_comment_tags(self, name: str, comment: str
- ) -> (str, str):
+ # For functions, since tags are on Return comments.
+ # Return the tags and the original comments, but without the tags.
+ def retrieve_comment_tags(self, name: str, comment: str, return_comment=""
+ ) -> (str, str, str):
since = ""
if comment is not None:
comment_match = re.search(r"\(?Since: v?(\d+\.\d+\.\d+\.?\d?)\)?",
@@ -2200,9 +2268,20 @@ class docBuilder:
# Only the version
since = comment_match.group(1)
+ if since == "" and return_comment is not None:
+ return_match = re.search(r"\(?Since: v?(\d+\.\d+\.\d+\.?\d?)\)?",
+ return_comment)
+ if return_match:
+ # Remove Since tag from the comment
+ (start, end) = return_match.span()
+ return_comment = return_comment[:start] + return_comment[end:]
+ return_comment = return_comment.strip()
+ # Only the version
+ since = return_match.group(1)
+
if since == "":
self.warning("Missing 'Since' tag for: " + name)
- return (since, comment)
+ return (since, comment, return_comment)
def modulename_file(self, file):
module = os.path.basename(file)
@@ -2238,7 +2317,7 @@ class docBuilder:
output.write(" type='%s'" % info[2])
if info[1] is not None and info[1] != '':
# Search for 'Since' version tag
- (since, comment) = self.retrieve_comment_tags(name, info[1])
+ (since, comment, _) = self.retrieve_comment_tags(name, info[1])
if len(since) > 0:
output.write(" version='%s'" % escape(since))
if len(comment) > 0:
@@ -2268,7 +2347,7 @@ class docBuilder:
else:
output.write(" raw='%s'" % escape(rawValue))
- (since, comment) = self.retrieve_comment_tags(name, desc)
+ (since, comment, _) = self.retrieve_comment_tags(name, desc)
if len(since) > 0:
output.write(" version='%s'" % escape(since))
output.write(">\n")
@@ -2302,7 +2381,7 @@ class docBuilder:
def serialize_typedef(self, output, name):
id = self.idx.typedefs[name]
- (since, comment) = self.retrieve_comment_tags(name, id.extra)
+ (since, comment, _) = self.retrieve_comment_tags(name, id.extra)
version_tag = len(since) > 0 and f" version='{since}'" or ""
if id.info[0:7] == 'struct ':
output.write(" <struct name='%s' file='%s' type='%s'%s" % (
@@ -2354,6 +2433,12 @@ class docBuilder:
if name == debugsym and not quiet:
print("=>", id)
+ (ret, params, desc) = id.info
+ return_comment = (ret is not None and ret[1] is not None) and ret[1] or ""
+ (since, comment, return_comment) = self.retrieve_comment_tags(name, desc, return_comment)
+ # Simple way to avoid setting empty version
+ version_tag = len(since) > 0 and f" version='{since}'" or ""
+
# NB: this is consumed by a regex in 'getAPIFilenames' in hvsupport.pl
if id.type == "function":
ver = self.versions[name]
@@ -2363,9 +2448,10 @@ class docBuilder:
name, self.modulename_file(id.header),
self.modulename_file(id.module), self.versions[name]))
else:
- output.write(" <functype name='%s' file='%s' module='%s'>\n" % (
+ output.write(" <functype name='%s' file='%s' module='%s'%s>\n" % (
name, self.modulename_file(id.header),
- self.modulename_file(id.module)))
+ self.modulename_file(id.module),
+ version_tag))
#
# Processing of conditionals modified by Bill 1/1/05
#
@@ -2376,19 +2462,33 @@ class docBuilder:
apstr = apstr + " && "
apstr = apstr + cond
output.write(" <cond>%s</cond>\n" % (apstr))
+
try:
- (ret, params, desc) = id.info
- output.write(" <info><![CDATA[%s]]></info>\n" % (desc))
+ # For functions, we get the since version from .syms files.
+ # This is an extra check to see that docstrings are correct
+ # and to avoid wrong versions in the .sym files too.
+ ver = name in self.versions and self.versions[name] or None
+ if len(since) > 0 and ver is not None and since != ver:
+ if name in ignored_function_versions:
+ allowedver = ignored_function_versions[name]
+ if allowedver != since:
+ self.warning(f"Function {name} has allowed version {allowedver} but docstring says {since}")
+ else:
+ self.warning(f"Function {name} has symversion {ver} but docstring says {since}")
+
+ output.write(" <info><![CDATA[%s]]></info>\n" % (comment))
self.indexString(name, desc)
+
if ret[0] is not None:
if ret[0] == "void":
output.write(" <return type='void'/>\n")
- elif (ret[1] is None or ret[1] == '') and name not in ignored_functions:
+ elif (return_comment == '') and name not in ignored_functions:
self.error("Missing documentation for return of function `%s'" % name)
else:
output.write(" <return type='%s' info='%s'/>\n" % (
- ret[0], escape(ret[1])))
+ ret[0], escape(return_comment)))
self.indexString(name, ret[1])
+
for param in params:
if param[0] == 'void':
continue
--
2.35.1On 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@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.
> + '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.
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.
> + 'virAdmServerUpdateTlsFiles': '6.2.0',
This one is needed as it's a mistake in the symbol export.
> + '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.
> + '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.
> + 'virNetworkUpdate': '1.0.0',
Not sure what happened here, but v0.10.2 what the script detects in the
symbol seems to be correct.
> + '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'
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@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
© 2016 - 2026 Red Hat, Inc.