[python] Scikit-build-core driven wheel creation #1454

DownerCase · 2024-03-12T14:14:27Z

Description

Python wheels are now built via scikit-build-core! 🎉
This work is based off of Kerstin's attempt a few months ago.

You should have write permissions to this branch if you wish to make modifications 😄

Changes of this PR:

Removed

BREAKING CHANGE: Removed all protobuf bindings. Shipping protoc generated python code is difficult if not impossible to do whilst being maximally compatible.
- Eg: Compatibility of generated Python code with protoc >= 3.19.0 protocolbuffers/protobuf#11123
Old wheel mechanism removed and replaced with scikit-build-core
The CMake option BUILD_STANDALONE_PY_WHEEL as it's not longer needed
ecal_python_functions.cmake as no functions from it are used now

Updated

Runtime dependencies (ecal_core, hdf5) copied at install time (not build time) via CMake's install command
Wheels are built and uploaded by cibuildwheel
CMake 3.18 or newer is required if building the Python modules.
- Access to the Development.Module component is strictly required for this workflow.
- This is only enforced if BUILD_PY_BINDING is on.
The "wheel" CMake preset has been renamed to "python" and updated to reflect how wheels are now built (i.e: static where possible)
The eCAL version may be provided by scikit-build-core via the SKBUILD_PROJECT_VERSION CMake variable, this is the only change to the CMake scripts that interacts with scikit-build-core.
protobuf >=3.8 but < 4.X is listed as dependency.
- By not shipping generated code, greater compatibility is possible. Should be able to extend to the python protobuf packages v4 and v5?

Added

The wheels now contain an autogenerated _version.py file containing the package version information
CMake has a new python component which has to be explicitly listed to CPack to be installed.

Identified issues:

setuptools-scm which provides the dynamic version metadata rejects the v6.0.0-nightly tag.
- I fixed this by creating a v6.0.0.dev0 tag to mark "start of v6 development". The tools were happy with that.
- Probably related: Unable to handle tags with dashes in the name, tag_regex config not helping pypa/setuptools-scm#825
- Resolved: New tag pushed
How to handle the protobuf bindings.
- I don't understand where the core proto messages would be used, especially after [python] python api fixed to work again based on new c++/c core api #1439
- IMO the apps are the most likely ones users could want / be using.
- In a pinch users can grab the .proto files and compile themselves to have generated code that matches the versions they want to use.
- Resolved: Deemed not an issue
The wheel includes multiple versioned SO's on Linux, namely the SOVERSION one and the VERSION one. Eg:
- libecal_core.so.6 and libecal_core.so.6.0.0
- libhdf5.so.103.4.1 and libhdf5.103
- I have no good idea on how to resolve this, at least perhaps until fix!: exclude installed files if listed in exclude scikit-build/scikit-build-core#652 is in a release and we can exclude *.so.*.*.* from the wheel?
cibuildwheel has been restricted to one architecture type (64-bit) as otherwise the second architecture to get built has linking problems
- Somehow we need to tell CMake the architecture
- Eg: Cannot link with default settings in win32/x86 pypa/cibuildwheel#844 (comment)
cibuildwheel does not build musl wheels. eCAL does not compile with musl libc.

Related issues

Partly resolves Rename python binding from pyhton3-ecal5 to eclipse-ecal #1394, the deb packaging isn't in this repo.
Fixes Cannot build Python wheel for RelWithDebugInfo on Windows #700 by deleting the problematic code

Related but may not close:

DownerCase · 2024-03-12T14:34:41Z

Ok. I'm being silly, I literally wrote that this won't work here due to the "v6.0.0-nightly" tag.

packaging.version.InvalidVersion: Invalid version: 'v6.0.0-nightly'

If you kindly push a tag called "v6.0.0.dev0" and rerun the wheels action everything should 🤞 be good.
Semver tags like "v6.0.0-alpha" or "v6.0.0-rc.1" also work as expected.

KerstinKeller · 2024-03-13T11:30:17Z

Hi, thanks so much for your PR, especially the great documentation of all issues.
I really tried gettings things to work myself for a day or two, but I ran into so many issues, that I moved on to other topics.

For me it's ok, not to ship the compiled protos.

We will try to retag with an alpha tag. But to my understanding, shouldn't 'v6.0.0-nightly' still be semver compliant?

DownerCase · 2024-03-13T11:47:49Z

Hi, thanks so much for your PR, especially the great documentation of all issues. I really tried gettings things to work myself for a day or two, but I ran into so many issues, that I moved on to other topics.

Yup, this took a lot longer than I expected too, but worth it.

We will try to retag with an alpha tag. But to my understanding, shouldn't 'v6.0.0-nightly' still be semver compliant?

Yes, but its not Python version specification compliant.
setuptools-scm uses this regex by default to match PEP440 style versions so for Python its strictly enforced for the version to meet this format ([N!]N(.N)*[{a|b|rc}N][.postN][.devN]). There is another section further down that talks about schemes for developmental release (eg: X.Y.devN)

DownerCase · 2024-03-23T12:34:38Z

@KerstinKeller Do you need anything further from me here? Everything is passing green now with the new tag (thank you!) except the cherry picker due to the missing label. If some pytests would lighten the validation load for you all and help get this merged I can do that, no problem.

KerstinKeller · 2024-03-25T09:01:59Z

Sorry I was really busy last week. The PR looks already really good. We wanted to do some lokal tests before merging.
Pytests would totally make sense (we can also help write some), I guess they can also be executed as part of the wheel building process?
Anyways, we really appreciate your contribution, and we will likely merge it later this weeks.

KerstinKeller · 2024-03-25T09:04:12Z

One more quick question: is there a reason that the ubuntu builds are done on Ubuntu 20.04 which is soon going to be deprecated. I think cibuildwheel will execute the actual build in a docker container? Or did I get something wrong here?

DownerCase · 2024-03-25T11:08:01Z

One more quick question: is there a reason that the ubuntu builds are done on Ubuntu 20.04 which is soon going to be deprecated. I think cibuildwheel will execute the actual build in a docker container? Or did I get something wrong here?

Correct. cibuildwheel does all build in manylinux docker containers. https://cibuildwheel.pypa.io/en/stable/#how-it-works
Those platforms are chosen because I didn't change them when importing what you had previously done.

Will update to ubuntu-latest and windows-latest as it shouldn't affect anything 😄

DownerCase · 2024-03-25T11:21:09Z

Sorry I was really busy last week. The PR looks already really good. We wanted to do some lokal tests before merging. Pytests would totally make sense (we can also help write some), I guess they can also be executed as part of the wheel building process? Anyways, we really appreciate your contribution, and we will likely merge it later this weeks.

No worries. Yes, the tests would run as part of cibuildwheel for every wheel

FlorianReimold · 2024-03-25T11:52:56Z

I tested the whl files for Windows Python 3.9 and Ubuntu 22.04 Python 3.10 and everything worked out of the box. I tested the HelloWorld and HelloWorld Protobuf sample from the documentation. The systems were clean and didn't have eCAL installed.

KerstinKeller

I added two remarks, but I think they should prevent us from merging the PR.

KerstinKeller · 2024-03-25T14:19:39Z

CMakeLists.txt

@@ -159,7 +158,18 @@ option(CPACK_PACK_WITH_INNOSETUP               "Create Innosetup installer for t

 set(ECAL_INSTALL_PYTHON_DIR "bin" CACHE PATH "Location to install the Python extension modules. Might be set by setupdtools install.")

-set(ECAL_BUILD_VERSION "0.0.0" CACHE STRING "Inject a build version if not building from a git repository")
+if(DEFINED SKBUILD_PROJECT_VERSION)


I'm not a 100% friend of this, i'd prefer if the build could just inject ECAL_BUILD_VERSION from the outside.
But this may be cleaned up later on

As far as I could tell, this is the only way to get the package version from setuptools-scm into CMake, let scikit-core handle it.

lang/python/CMakeLists.txt

FlorianReimold · 2024-03-28T08:58:16Z

Hi @DownerCase,

I tried to build the python wheel locally on my PC and realized that I have no idea how to do that. Is there a guide somewhere how that can be done?

DownerCase · 2024-03-28T13:10:07Z

There are a few methods but I do

python -m pip install build
python -m build

cibuildwheels can also be run locally if you desire.

https://packaging.python.org/en/latest/tutorials/packaging-projects/#generating-distribution-archives

FlorianReimold · 2024-04-03T14:38:10Z

Thanks, works like a charm!

Currently, we have the following python packages:

.whl files for Ubuntu amd64 and Windows amd64
.deb files for Ubuntu amd64 / x86 / arm64 / armhf

I checked whether I can reuse the old scripts to generate the .deb packages and that did not work (as expected). While I wouldn't really mind dropping the amd64 .deb installers as those users can switch to the manylinux pypi packages, I would still like to offer the arm64 users a way to install the python binding.

So, do you by any chance know how to
a) build arm64 whl files with the GH Actions or
b) build .deb packages for this python binding?

In the past I used the following script for generating the deb package:

override_dh_auto_install-arch:
	dh_auto_install --arch

	# Python install.
	set -e; \
	cd _build/python && for python in $(shell py3versions -r); do \
		$$python setup.py install \
			--install-layout=deb --no-compile \
			--root=$(CURDIR)/debian/python3-ecal5; \
	done
	find $(CURDIR)/debian/python3-ecal5 -name 'ecal-*-nspkg.pth' -delete

DownerCase · 2024-04-03T15:45:57Z

a) build arm64 whl files with the GH Actions

Yes! cibuildwheel can do it: docs. I'll set that up here too

b) build .deb packages for this python binding?

That is outside of my knowledge. Theoretically that method could work on new enough versions of pip to support the build backend standard. You may need to add the minimal stub setup.py.

DownerCase · 2024-04-03T20:13:06Z

Yikes, building arm64 in emulation is really slow.

I had a quick look at your deb situation, doesn't look great. For a start you'd be needing to disable the RPATH/install shenanigans as you'd want to depend on the system library from the main eCAL apt package.
On top you have to figure out how to get Ubuntu 20.04 to use new tools. The system packages on there have no idea about pyproject.toml as the ecosystem was stuck on setuptools and setup.py as the standard for so long... Doesn't seem impossible, just a lot of extra work... (Will be easier in the future when the debian packaging tools have fully caught up)

DownerCase · 2024-04-04T18:58:34Z

Hmm. Over an hour to get all green CI ticks is unacceptable. Will rework to only trigger the emulated compilation on releases? IMO, I only need to beat the 37m windows build for daily PR workflows.

FlorianReimold · 2024-04-05T05:24:38Z

Is there a way to have them trigger on releases and on "manual trigger" or something similar? I was also thinking about the same thing.

DownerCase · 2024-04-06T08:52:35Z

Is there a way to have them trigger on releases and on "manual trigger" or something similar? I was also thinking about the same thing.

Theoretically I've done just that now 😄 It's set to only do the arm64 wheels on (pre-)releases being published or when the workflow is manually started.
See here for how to manually trigger a workflow: https://docs.github.com/en/actions/using-workflows/manually-running-a-workflow

Properly handling the protobuf bindings in Python is very challenging. So for now it has been removed, users may fetch the proto files from the repo and generate the bindings themselves if needed.

DownerCase force-pushed the feature/pypi_wheels branch from fc92105 to bb33c6d Compare March 12, 2024 14:33

DownerCase force-pushed the feature/pypi_wheels branch from 5fd545c to ffb4802 Compare March 13, 2024 19:47

KerstinKeller approved these changes Mar 25, 2024

View reviewed changes

FlorianReimold added the cherry-pick-to-NONE Don't cherry-pick these changes label Apr 3, 2024

DownerCase force-pushed the feature/pypi_wheels branch from df3ebdd to 698ce8a Compare April 3, 2024 19:24

DownerCase and others added 8 commits April 6, 2024 09:54

build[python]: Remove protobuf binding generation

4714b12

Properly handling the protobuf bindings in Python is very challenging. So for now it has been removed, users may fetch the proto files from the repo and generate the bindings themselves if needed.

build[python]: scikit-build-core wheel building

348cb05

fix: Restrict setuptools_scm version

a3cacf8

Update build_wheels.yml OSes

c02e301

Add arm64 wheel builds

5c791ea

Drop i686 linux wheels

ec229b8

ci: Only build arm64 wheels on releases/manual trigger

23cc7ed

fix: Use submodule dependency provider

eed19a3

DownerCase force-pushed the feature/pypi_wheels branch from 83d9da3 to eed19a3 Compare April 6, 2024 09:14

FlorianReimold mentioned this pull request Apr 26, 2024

[GH Actions] Cleaned Windows and Ubuntu Actions #1571

Merged

FlorianReimold approved these changes May 7, 2024

View reviewed changes

FlorianReimold merged commit e78be41 into eclipse-ecal:master May 7, 2024
11 checks passed

DownerCase deleted the feature/pypi_wheels branch May 7, 2024 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[python] Scikit-build-core driven wheel creation #1454

[python] Scikit-build-core driven wheel creation #1454

DownerCase commented Mar 12, 2024 •

edited

Loading

DownerCase commented Mar 12, 2024 •

edited

Loading

KerstinKeller commented Mar 13, 2024 •

edited

Loading

DownerCase commented Mar 13, 2024

DownerCase commented Mar 23, 2024

KerstinKeller commented Mar 25, 2024

KerstinKeller commented Mar 25, 2024

DownerCase commented Mar 25, 2024

DownerCase commented Mar 25, 2024

FlorianReimold commented Mar 25, 2024

KerstinKeller left a comment

KerstinKeller Mar 25, 2024

DownerCase Mar 25, 2024

FlorianReimold commented Mar 28, 2024

DownerCase commented Mar 28, 2024

FlorianReimold commented Apr 3, 2024 •

edited

Loading

DownerCase commented Apr 3, 2024

DownerCase commented Apr 3, 2024

DownerCase commented Apr 4, 2024

FlorianReimold commented Apr 5, 2024

DownerCase commented Apr 6, 2024

[python] Scikit-build-core driven wheel creation #1454

[python] Scikit-build-core driven wheel creation #1454

Conversation

DownerCase commented Mar 12, 2024 • edited Loading

Description

Removed

Updated

Added

Identified issues:

Related issues

DownerCase commented Mar 12, 2024 • edited Loading

KerstinKeller commented Mar 13, 2024 • edited Loading

DownerCase commented Mar 13, 2024

DownerCase commented Mar 23, 2024

KerstinKeller commented Mar 25, 2024

KerstinKeller commented Mar 25, 2024

DownerCase commented Mar 25, 2024

DownerCase commented Mar 25, 2024

FlorianReimold commented Mar 25, 2024

KerstinKeller left a comment

Choose a reason for hiding this comment

KerstinKeller Mar 25, 2024

Choose a reason for hiding this comment

DownerCase Mar 25, 2024

Choose a reason for hiding this comment

FlorianReimold commented Mar 28, 2024

DownerCase commented Mar 28, 2024

FlorianReimold commented Apr 3, 2024 • edited Loading

DownerCase commented Apr 3, 2024

DownerCase commented Apr 3, 2024

DownerCase commented Apr 4, 2024

FlorianReimold commented Apr 5, 2024

DownerCase commented Apr 6, 2024

DownerCase commented Mar 12, 2024 •

edited

Loading

DownerCase commented Mar 12, 2024 •

edited

Loading

KerstinKeller commented Mar 13, 2024 •

edited

Loading

FlorianReimold commented Apr 3, 2024 •

edited

Loading