On Thu, Apr 14, 2022 at 22:47:11 +0200, Victor Toso wrote:
Hi,
Couple of comments which I summarize on the top level rather than doing
specifics since they might need a reorg of the series:
The goal of this patch series is to provide 'since' version to all
exported types.
This is the non lazy version of the v1. In this series, we do
change the docstrings of all exported types, to add the version
metadata, in order to have scripts/apibuild.py to fetch it and add
it to the appropriated XML API. This patch series also enforces
that every new exported types requires a docstring with version
with the proper format.
v1:
https://listman.redhat.com/archives/libvir-list/2022-April/229881.html
I've created a script that helped me out and it covered a good
amound of cases. I hand fixes all the missing docstrings and
corner cases that my script missed.
As mentioned in v1, I've used:
git grep -rq $symbol $tag $includedir
This is used to find if a given $symbol exists in a given $tag or
not. $tag starts with v1.0.0 and I have ignored any $tag that does
not start with 'v' or has '-rc' in the name.
To help review this not so small changeset, the changes were split
in the following order:
* docs: generated: 98% work from the script. I've split it
further, by module + group type.
* docs: manual: -> 30% manual labor, 70% vim's macro. It was also
split where it seems reasonable.
* docs: (...) -> Some fixes needed in the docs.
* scripts: -> Improvements to apibuild script.
* syms: -> Some fixes found with the found mismatch between
docstring and sym files.
Other than that, I only caught two false positives, that is, a $symbol
was present in a $tag but it was exported only at a latter $tag.
Branch :
https://gitlab.com/victortoso/libvirt/-/commits/add-since-version
Green CI:
https://gitlab.com/victortoso/libvirt/-/pipelines/517262280
Cheers,
Victor
Victor Toso (34):
docs: Fix generated documentation of virConnectListAllNodeDeviceFlags
docs: variable: Move docstring from source to header file
docs: generated: enums: libvirt: append 'Since version' metadata
The tree fails to build after this commit. Per our guidelines the tree
must be buildable after every single commit:
https://www.libvirt.org/hacking.html#preparing-patches
This is to ensure bisectability. You'll need to reorganize the fixes
you've done after applying the changes automatically to happen before or
at the same time to ensure compliance.
docs: generated: enums: qemu: append 'Since version'
metadata
docs: generated: enums: admin: append 'Since version' metadata
docs: generated: macros: libvirt: append 'Since version' metadata
docs: generated: macros: admin: append 'Since version' metadata
docs: generated: typedefs: libvirt: append 'Since version' metadata
docs: generated: typedefs: qemu: append 'Since version' metadata
docs: generated: typedefs: admin: append 'Since version' metadata
docs: generated: functions: libvirt: append 'Since version' metadata
docs: generated: functions: qemu: append 'Since version' metadata
docs: generated: functions: lxc: append 'Since version' metadata
docs: generated: functions: admin: append 'Since version' metadata
docs: manual: typedef: add docstring and Since metadata
docs: manual: functions: add Since metadata
docs: manual: enums: add docstring and Since metadata
docs: manual: macros: add docstring and Since metadata
docs: manual: libvirt-common: add docstring and Since metadata
docs: Fix generated documentation of virStorageVolInfoFlags
docs: Fix and append Since to virConnectListAllStoragePoolsFlags
docs: Fix and append Since to virDomainDeviceModifyFlags
docs: Fix and append Since to virDomainMemoryModFlags
docs: Fix and append Since to virDomainVcpuFlags
None of these actually change anything in the 'docs' directory. I'd
suggest to pick an unambiguous prefix such as "symbols" or something
similar.
scripts: apibuild: parse 'Since' version for enums
scripts: apibuild: fix parsing block comments from typedef enum
scripts: apibuild: parse 'Since' for typedefs
scripts: apibuild: parse 'Since' for macros
After the fix mentioned below [1] the build of the tree breaks after
this commit:
FAILED: docs/libvirt-api.xml docs/libvirt-lxc-api.xml docs/libvirt-qemu-api.xml
docs/libvirt-admin-api.xml
/home/pipo/libvirt/scripts/meson-python.sh /bin/python3
/home/pipo/libvirt/scripts/apibuild.py /home/pipo/libvirt/docs
/home/pipo/build/libvirt/gcc/docs
Function virDomainSetBlockThreshold has symversion 3.1.0 but docstring says 3.2.0
Function virAdmClientClose has symversion 2.0.0 but docstring says 1.3.5
Function virAdmClientFree has symversion 2.0.0 but docstring says 1.3.5
Function virAdmClientGetID has symversion 2.0.0 but docstring says 1.3.5
...
scripts: apibuild: parse 'Since' for functions
scripts: apibuild: factor out comment cleaning
scripts: apibuild: add parsing variable's comments
syms: admin: Add sections to match when API was introduced
syms: libvirt: move virDomainSetBlockThreshold to 3.2.0
These two commits are not acceptable as they break existing builds of
software that references the symbols. At some point in the past the
mistake was noticed but we must not ever change it due to linking
intricacies:
Example:
$ cat threshold.c
#include <stddef.h>
#include <libvirt/libvirt.h>
int main()
{
virInitialize();
virDomainSetBlockThreshold(NULL, 0, 0, 0);
return 0;
}
$ gcc threshold.c -o threshold -lvirt -pedantic -Wall
$ ./threshold
libvirt: Domain error : invalid domain pointer in virDomainSetBlockThreshold
$ LD_LIBRARY_PATH=/home/pipo/build/libvirt/gcc/src/ ./threshold
./threshold: symbol lookup error: ./threshold: undefined symbol:
virDomainSetBlockThreshold, version LIBVIRT_3.1.0
Now my system libvirt without your patches causes the example program to
link against a different version of the symbol and your change then
breaks everything linked against libvirt.
The two changes must be dropped and the rest of the series modified to
build cleanly.
syntax-check: sc_prohibit_nonreentrant: skip comments
Ah, so moving this commit to the beginning fixes the build issue I've
seen after 3/34.