Skip to content

ABI compatibility checker#269

Merged
schopin-pro merged 8 commits intomainfrom
slyon/abigail-FR-1799
Apr 26, 2022
Merged

ABI compatibility checker#269
schopin-pro merged 8 commits intomainfrom
slyon/abigail-FR-1799

Conversation

@slyon
Copy link
Contributor

@slyon slyon commented Mar 22, 2022

Description

Make use of abigail-tools to export the current ABI to an XML file and compare every build of our GitHub Actions CI to the committed version of the ABI, to check for breakage.

The committed version in abi-compat/focal_0.104.xml was generated on Focal (using abigail 2.0 from ppa:slyon/ci-netplan):

make && abidw libnetplan.so.0.0 --headers-dir include/ --header-file src/abi.h > abi-compat/focal_0.104.xml

This should be upgraded to make use of the meson build from #268 and be run on Ubuntu Jammy LTS runners, once available in GA. Meson will just slightly change the path to the library (build/src/libnetplan.so.0.0) but works equally otherwise.

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@slyon slyon force-pushed the slyon/abigail-FR-1799 branch 2 times, most recently from 1990785 to e830343 Compare March 22, 2022 16:40
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2022

Codecov Report

Merging #269 (bbff739) into main (a738597) will not change coverage.
The diff coverage is n/a.

❗ Current head bbff739 differs from pull request most recent head 81335f5. Consider uploading reports for the commit 81335f5 to get more accurate results

@@           Coverage Diff           @@
##             main     #269   +/-   ##
=======================================
  Coverage   99.07%   99.07%           
=======================================
  Files          61       61           
  Lines       10864    10864           
=======================================
  Hits        10764    10764           
  Misses        100      100           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a738597...81335f5. Read the comment docs.

@slyon slyon requested a review from schopin-pro March 22, 2022 16:59
@slyon slyon marked this pull request as ready for review March 22, 2022 17:00
Copy link
Contributor

@schopin-pro schopin-pro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the idea, but I'd like to be sure that we can break our internal stuff in peace while being yelled out loudly if we touch what's actually public.

@slyon slyon force-pushed the slyon/abigail-FR-1799 branch from e830343 to a27c609 Compare March 24, 2022 12:59
@schopin-pro
Copy link
Contributor

I took the liberty of adding a new commit that implements my "move the header around" approach, and also imports all subtypes that are transitively part of the ABI into the abi.h header. Feel free to move it aside ;-)

@slyon slyon force-pushed the slyon/abigail-FR-1799 branch from f2265b3 to 7f9e8c9 Compare April 19, 2022 09:18
slyon and others added 6 commits April 19, 2022 11:55
XML serialization generated on Focal via:
abidw libnetplan.so.0.0 --headers-dir include/ > abi-compat/focal_0.104.xml
This is to allow ABI compatibilty checks of NetplanNetDefinition using the
'abidiff' tool.
Also, only move them in include/ for the ABI check, so that they don't
get shipped as public headers.
@slyon slyon force-pushed the slyon/abigail-FR-1799 branch from 2c41b69 to 7c95220 Compare April 19, 2022 09:56
@slyon slyon force-pushed the slyon/abigail-FR-1799 branch from 3b6f0e7 to bbff739 Compare April 19, 2022 12:30
@slyon
Copy link
Contributor Author

slyon commented Apr 19, 2022

Thank you @schopin-pro Moving the abi.h header just for the duration of the CI check is a smart idea, thanks for your commit!

After some more testing, I have the feeling that abigail 1.6 (as found in Focal) is just not good enough for our needs, though. So I've prepared an abigail 2.0 backport/rebuild for Focal in ppa:slyon/ci-netplan and making use of the --header-file[2] parameter in the CI. This way we should have a clean path forward and all the current features available.

IMO this should be ready for merging.

@slyon slyon requested a review from schopin-pro April 19, 2022 12:55
@schopin-pro schopin-pro merged commit f71f14e into main Apr 26, 2022
@schopin-pro schopin-pro deleted the slyon/abigail-FR-1799 branch April 26, 2022 08:04
netdever added a commit to netdever/netplan that referenced this pull request Apr 27, 2022
* ABI compatibility checker (canonical#269)

* abigail: check-in 0.104 ABI

XML serialization generated on Focal via:
abidw libnetplan.so.0.0 --headers-dir include/ > abi-compat/focal_0.104.xml

* abi-compat: use abigail to check ABI compatibility via GA

* [WIP] src:types: move netplan_net_definition into a separate file

This is to allow ABI compatibilty checks of NetplanNetDefinition using the
'abidiff' tool.

* WIP: keep the ABI-specific structs self-contained

Also, only move them in include/ for the ABI check, so that they don't
get shipped as public headers.

* GA CI: update comments

* abi-compat: update XML according to latest state

* GA: upgrade to abigail-tools 2.0

Using PPA version from https://launchpad.net/~slyon/+archive/ubuntu/ci-netplan

* src: cleanup types.h ABI include

Co-authored-by: Simon Chopin <simon.chopin@canonical.com>

* networkd: rename GatewayOnlink= to GatewayOnLink= (canonical#273)

As of systemd v242 the "GatewayOnlink" spelling was deprecated and replaced by
"GatewayOnLink", while still being available in compat mode.

We've waited long enough and can switch to the proper spelling now.

systemd/systemd@9cb8c55

* cli:sriov: fix test coverage for quirk_devices

newer coverage tools seem to be more picky, make them happy

* Makefile: clean src/_features.h.gch file

* Adding VRF and VXLAN support for systemd-networkd. LP: #1764716, #1773522

* Add vxlan/vrf changes to new abi.h file

Co-authored-by: Lukas Märdian <slyon@ubuntu.com>
Co-authored-by: Simon Chopin <simon.chopin@canonical.com>
Co-authored-by: Anthony Timmins <atimmins@datto.com>
netdever added a commit to netdever/netplan that referenced this pull request Apr 27, 2022
* ABI compatibility checker (canonical#269)

* abigail: check-in 0.104 ABI

XML serialization generated on Focal via:
abidw libnetplan.so.0.0 --headers-dir include/ > abi-compat/focal_0.104.xml

* abi-compat: use abigail to check ABI compatibility via GA

* [WIP] src:types: move netplan_net_definition into a separate file

This is to allow ABI compatibilty checks of NetplanNetDefinition using the
'abidiff' tool.

* WIP: keep the ABI-specific structs self-contained

Also, only move them in include/ for the ABI check, so that they don't
get shipped as public headers.

* GA CI: update comments

* abi-compat: update XML according to latest state

* GA: upgrade to abigail-tools 2.0

Using PPA version from https://launchpad.net/~slyon/+archive/ubuntu/ci-netplan

* src: cleanup types.h ABI include

Co-authored-by: Simon Chopin <simon.chopin@canonical.com>

* networkd: rename GatewayOnlink= to GatewayOnLink= (canonical#273)

As of systemd v242 the "GatewayOnlink" spelling was deprecated and replaced by
"GatewayOnLink", while still being available in compat mode.

We've waited long enough and can switch to the proper spelling now.

systemd/systemd@9cb8c55

* cli:sriov: fix test coverage for quirk_devices

newer coverage tools seem to be more picky, make them happy

* Makefile: clean src/_features.h.gch file

Co-authored-by: Lukas Märdian <slyon@ubuntu.com>
Co-authored-by: Simon Chopin <simon.chopin@canonical.com>
Co-authored-by: Anthony Timmins <atimmins@datto.com>
@schopin-pro schopin-pro removed their request for review June 15, 2022 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants