Draft: Build and release wheel packages #1714

ssssam · 2022-08-03T16:18:49Z

See #1712 for rationale.

This updates the CI config to:

build x86_64 wheel packages on each update of 'master', which can be downloaded as Action artifacts. The packages contain release binaries of BuildBox casd/fuse/run-bubblewrap.
build sdist and wheel packages on each release tag, test the wheels, and upload them to Test PyPI at https://test.pypi.org/project/BuildStream/

Before actually landing this, we need to make some changes:

update documentation around release process and installation (https://gitlab.com/BuildStream/website/-/merge_requests/153)
release to the real PyPI instead of Test PyPI (in release.yml)
create and add a PyPI API token as a repository secret

ssssam · 2022-08-03T16:22:12Z

Apologies, i also didn't make this a draft correctly.

ssssam · 2022-08-04T10:27:13Z

Updated to avoid use of thirdparty Github actions as these are prohibited in apache/ namespace.

Not working use of pypa/cibuildwheel action: https://github.com/apache/buildstream/actions/runs/2790846103
Working when calling cibuildwheel binary directly: https://github.com/apache/buildstream/actions/runs/2795849927

ssssam · 2022-08-05T14:35:02Z

Bunding buildbox binaries

Building the binaries is still an open task - static linking with cmake is not simple, see https://gitlab.com/BuildGrid/buildbox/buildbox-integration/-/issues/1 for tracking issue.

A second blocker is a confusing setuptools issue. Behaviour changes depending whether it is called on the commandline or via the pep517 'build_wheel' hook.

Called as follows in this branch of buildstream.git:

 rm -Rf dist/ build/
 env BST_BUNDLE_BUILDBOX=1 python3 setup.py bdist_wheel > log-bdist_wheel.txt 2>&1

Shows logs:

> grep subprojects log-bdist_wheel.txt 
creating build/lib.linux-x86_64-3.10/buildstream/subprojects
creating build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox
copying src/buildstream/subprojects/buildbox/buildbox-casd -> build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox
copying src/buildstream/subprojects/buildbox/buildbox-fuse -> build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox
copying src/buildstream/subprojects/buildbox/buildbox-run -> build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox
creating build/bdist.linux-x86_64/wheel/buildstream/subprojects
creating build/bdist.linux-x86_64/wheel/buildstream/subprojects/buildbox
copying build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox/buildbox-casd -> build/bdist.linux-x86_64/wheel/buildstream/subprojects/buildbox
copying build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox/buildbox-fuse -> build/bdist.linux-x86_64/wheel/buildstream/subprojects/buildbox
copying build/lib.linux-x86_64-3.10/buildstream/subprojects/buildbox/buildbox-run -> build/bdist.linux-x86_64/wheel/buildstream/subprojects/buildbox
adding 'buildstream/subprojects/buildbox/buildbox-casd'
adding 'buildstream/subprojects/buildbox/buildbox-fuse'
adding 'buildstream/subprojects/buildbox/buildbox-run'

Called via the build_meta hook as done by build and cibuildwheel:

 rm -Rf dist/ build/
 env BST_BUNDLE_BUILDBOX=1 python3 -c 'import setuptools.build_meta; setuptools.build_meta.build_wheel(".")' > build_meta.log 2>&1

Shows logs:

> grep subprojects build_meta.log 
warning: no previously-included files matching '*' found under directory 'src/buildstream/subprojects'

The warning seems to be as the MANIFEST.in excludes those subprojects/ files from the sdist. Its not clear why the files are suddenly not found...

ssssam · 2022-08-08T15:17:53Z

I have now produced wheels containing BuildBox in CI, see : https://test.pypi.org/project/BuildStream/1.999.666.dev2/#files

Installing these into a venv where no buildbox-casd binary is in PATH, produces an error that buildbox-casd was found and segfaulted on startup. So, the setuptools part of the work is complete, we now need to focus on working static buildbox binaries.

This allows using [cibuildwheel](https://cibuildwheel.readthedocs.io) locally to produce various Wheel packages of BuildStream. These Wheel packages contain prebuilt binaries of Cython modules, so `pip3 install buildstream` on compatible systems does not require a C compiler and Python headers on the target machine. apache#1712

ssssam · 2022-08-12T10:45:18Z

This is now working in ssssam/buildstream fork:

release actions for a test "1.999.670" release: https://github.com/ssssam/buildstream/actions/runs/2846160400
BuildStream+BuildBox wheels for 1.999.670 on test.pypi.org: https://test.pypi.org/project/BuildStream/1.999.670.dev0/#files

The only items remaining on my todo list are updating release process and installation docs. (Plus dealing with review comments and CI failures :-)

ssssam · 2022-08-12T11:36:13Z

Note that the changes in setup.py to add '.dev0' suffix to the BuildStream version affect bst --version output as well as PyPI output. Does that matter?

ssssam · 2022-08-12T11:46:35Z

pylint task is failing, with error about unrecognised option "no-space-check". Confusingly this was removed in pylint 2.6.

pylint was updated in this branch, because:

i used 'packaging.version' in setup.py
i added 'packaging' to requirements/requirements.in
i ran make in requirements/
it generated a whole new requirements.txt which includes an update for pylint from 2.13 to 2.14

is there a way to add 'packaging' as a dep without implicitly updating pylint etc. too?

~~anyway, here are fixes for said pylint issues: #1717~~

for now I have pushed fixes for pylint 2.14 to this branch.

gtristan · 2022-08-12T12:40:36Z

Note that the changes in setup.py to add '.dev0' suffix to the BuildStream version affect bst --version output as well as PyPI output. Does that matter?

I don’t think it matters, we’ll still be able to know what version it is, and should not break any integrations.

nanonyme · 2022-08-12T18:40:20Z

I'm a bit concerned how the plan is to update buildbox components. Does any update mean making a new BuildStream release?

.github/workflows/release.yml

src/buildstream/utils.py

gtristan · 2022-08-13T08:51:49Z

I'm a bit concerned how the plan is to update buildbox components. Does any update mean making a new BuildStream release?

Updating buildbox components does not necessarily mean that we will create a new BuildStream release just for the sake of publishing wheels which conveniently contain the new binary, although it might happen.

That said, if buildbox components change in a way that fixes a BuildStream issue, then we will likely internally bump our runtime requirement on the buildbox-casd version and publish a new release which depends on the new version with the fixed bug (consequently publishing new conveniently packaged wheels in the process).

gtristan · 2022-08-13T09:00:31Z

@ssssam overall it looks good to me, I have some concerns.

This adds a lot of complexity to setup.py, I wonder if we need to worry about upstream python deprecating setup.py and if so, whether their alternatives will provide the flexibility we now require.
This will run CI on the wheels we upload only at tag/release time, which presents some workflow complexity
- Currently we test everything that is required when merging a commit to master, so we already know that CI will pass on a tag
- I don't think we want to build wheels and run CI on wheels for every merge to master, CI is already quite lengthy

Maybe if building wheels and running the single test is not too onerous, we should still run this in normal CI (without publishing) in addition to when we tag a release ?

Otherwise, perhaps we could have a special tag semantic especially for the purpose of running this CI in advance of tagging ? like 1.95.2-test1 could be a test run of the release without actually publishing a release ? Just a thought, open to any other ideas of how we could solve this...

nanonyme · 2022-08-13T15:17:37Z

I'm not sure if you need to care about upstream Python. distutils is gone in Python 3.12, you are fully and directly only a customer of setuptools project.

ssssam · 2022-08-15T09:08:54Z

Useful comments, will update this soon and include doc updates.

This adds a lot of complexity to setup.py, I wonder if we need to worry about upstream python deprecating setup.py and if so, whether their alternatives will provide the flexibility we now require.

When the Python community talk about deprecating setup.py, they do not mean "everyone has to remove their setup.py file." They are talking about having a standard build API for Python.

In the old days, the official way to build a Python project was, "run a file named ./setup.py", and if that file didn't exist then it was considered a bad project doing non-standard things. Except there was no written standard anywhere, and some people wanted to use tools that weren't setuptools like Flit or Hatch.

There is now a written standard, the official way to build a Python project is via this well-defined API configured in the pyproject.toml file. The configuration can continue to say "build this project by running setuptools", and setuptools is not deprecated. What is deprecated is the assumption that every project uses setuptools, and the assumption that running setup.py directly should work in every Python package.

Meanwhile setuptools are adding support for declarative configuration via pyproject.toml or setup.cfg, which is nice and we should use this where possible. AFAICS not everything can be done declaratively, e.g. Cython still requires writing code in setup.py. I've not seen any hint that setuptools are planning to deprecate use of setup.py.

That said, there is quite some code added to setup.py :) Adding support for custom version styles to versioneer would help us reduce that, but i didn't have time to dig into it.

This will run CI on the wheels we upload only at tag/release time, which presents some workflow complexity
Currently we test everything that is required when merging a commit to master, so we already know that CI will pass on a tag
I don't think we want to build wheels and run CI on wheels for every merge to master, CI is already quite lengthy

The PR currently runs wheel build + test on each merge to master. I thought the same thing that the release manager will not want to discover horrifically broken wheels only at the point on Friday night when they push a new tag.

nanonyme · 2022-08-15T10:55:31Z

Right, typically you are supposed to be using both setup.py and pyproject.toml with setuptools. Former declares entrypoint that executes the latter. The thing that is getting deprecated is invoking certain setup.py commands directly

gtristan · 2022-08-15T11:51:10Z

[...]
That said, there is quite some code added to setup.py :) Adding support for custom version styles to versioneer would help us reduce that, but i didn't have time to dig into it.

Not much of a concern really, I was mostly worried about betting on setup.py too heavily, but you've put my concerns to rest :)

This will run CI on the wheels we upload only at tag/release time, which presents some workflow complexity
Currently we test everything that is required when merging a commit to master, so we already know that CI will pass on a tag
I don't think we want to build wheels and run CI on wheels for every merge to master, CI is already quite lengthy

The PR currently runs wheel build + test on each merge to master. I thought the same thing that the release manager will not want to discover horrifically broken wheels only at the point on Friday night when they push a new tag.

~~Great, I didn't notice from my reading of the patch.~~

Aha, so the test happens post merge... this is something I never find myself checking.

I wonder if it would be reasonable to test this pre-merge ?

Currently the only reason we run CI post-merge is to update the master docs.

ssssam · 2022-08-15T17:16:38Z

I wonder if it would be reasonable to test this pre-merge ?

It's a trade-off of resource usage and time vs convenience. For reference, the "Build wheels" stage takes 4m 8s in my testing, and the "Test wheels" stage runs 4 jobs of <1m30 each. (Via: https://github.com/ssssam/buildstream/actions/runs/2860225838)

Meanwhile the CI "tests" job takes about 30 minutes. So I don't see any big issue moving the wheel builds into that, other than a small increase in CI resource usage (people update PRs more often than they merge to master)

gtristan · 2022-08-16T09:27:30Z

[...]

Meanwhile the CI "tests" job takes about 30 minutes. So I don't see any big issue moving the wheel builds into that, other than a small increase in CI resource usage (people update PRs more often than they merge to master)

Then I think it would be very desirable to move this to pre merge, I think we will likely not be actively checking for failures in post-merge CI.

ssssam · 2022-08-16T13:37:40Z

Updates today:

Move build_wheel and test_wheel to ci.yml from merge.yml - you can see them in the 'PR Checks' section below. (I also did a new test release at https://github.com/ssssam/buildstream/actions/runs/2868187250)
Use official BuildBox binaries from BuildGrid/buildbox/buildbox-integration now they are available (we were using tristanvb/buildbox-integration)

.github/workflows/ci.yml

.github/workflows/merge.yml

.github/workflows/release.yml

ssssam · 2022-08-16T15:20:32Z

Updates in [a5da072] tip:

Release to real PyPI (dependent on secrets.PYPI_TOKEN being set)
Fix the 'Test wheels' action names
Squash fixups into main commits

We are going to build and publish more than just docs, so add _docs to these job names to make it clear.

This updates the CI config to: * build wheel packages on each update of 'master', which can be downloaded as Action artifacts * build sdist and wheel packages on each release tag, and upload them to Test PyPI at https://test.pypi.org/project/BuildStream/ The new workflows are based on examples at https://cibuildwheel.readthedocs.io/en/stable/setup/, avoiding use of the GitHub Actions from `pypa/*` as these are not permitted to be used in apache/buildstream project.

BuildBox is not widely distributed in binary form yet. For convience, add a mechanism to bundle prebuilt binaries in the Python wheel packages. Setting BST_BUNDLE_BUILDBOX=1 when setup.py runs, causes the bundled binaries to be included in the binary package. Its up to the packager to make appropriate binaries available in `src/buildstream/subprojects/buildbox/`. BuildStream will search the package subprojects/ dir when looking for BuildBox binaries on the host in all cases, prioritizing any bundled binaries above other ones on the host.

We don't run the entire integration test suite here as we can assume the release commit was already tested by `ci.yml` and `merge.yml`. We just install the wheel and run a simple build to show that the Python build infra and the BuildBox binaries are up to standard. Wheels are also tested pre-merge so that breakages are detected early rather than ruining the afternoon of whoever makes the release. The full release is gated on the testing, we don't want to release to PyPI if the docs didn't build.

This will be used in setup.py to process PEP440 version numbers. It's a small helper library which is already a transitive dependency of various tools we use including Tox, Sphinx, and Pytest. Regenerate *requirements.txt using requirements/Makefile.

PyPI and Pip use the PEP440 version standard, which uses explicit '.dev', '.a', '.b' and '.rc' suffixes to mark "unstable" releases. BuildStream uses the even/odd convention. When running `pip install` we want Pip to prefer a stable 2.0.0 release over an unstable 2.1.0 release. To make this work, all unstable releases now have a .dev0 suffix automatically added in setup.py, e.g. `2.1.0.dev0`. This is necessary for us to fully automate releases to PyPI from GitHub Actions.

Merge two strings on the same line into one.

The no-space-check option was removed: https://pylint.pycqa.org/en/latest/whatsnew/2/2.6/summary.html#other-changes The no-self-use and bad-continuation checks are now in plugins, so are already disabled by default. Putting the `##############` header on the same line as `disable=` leads to it being processed as a value rather than a comment.

ssssam · 2022-08-16T16:05:47Z

One final update 5700177 which splits the ".github/workflows/merge.yml: Rename 'build' and 'publish'" commit out from "Build and release wheels packages as part of GitHub Actions".

cs-shadow

I'm pretty happy with this change. I really like the idea of being able to pip install buildstream and have it just work.

I've left some (very) minor comments inline but I think you've already covered most of packaging-related caveats on the whole.

.github/wheel-helpers/fetch-latest-buildbox-release.sh

setup.py

src/buildstream/utils.py

requirements/requirements.in

gtristan · 2022-08-17T08:36:55Z

This is replaced by #1723, just finishing up there since @ssssam is off for the rest of the week.

ssssam requested review from gtristan, juergbi, BenjaminSchubert, cs-shadow and abderrahim as code owners August 3, 2022 16:18

ssssam marked this pull request as draft August 3, 2022 16:22

ssssam mentioned this pull request Aug 3, 2022

Distribute buildstream Wheel packages that contain BuildBox #1712

Closed

ssssam force-pushed the sam/build-wheels branch from 4184388 to 43e7ab8 Compare August 4, 2022 10:06

ssssam force-pushed the sam/build-wheels branch from 43e7ab8 to 365484d Compare August 12, 2022 10:39

ssssam force-pushed the sam/build-wheels branch 2 times, most recently from 5e588f2 to 7eab684 Compare August 12, 2022 11:33

ssssam mentioned this pull request Aug 12, 2022

Update pylint config and fix some trivial issues #1717

Closed

gtristan reviewed Aug 13, 2022

View reviewed changes

.github/workflows/release.yml Outdated Show resolved Hide resolved

gtristan reviewed Aug 13, 2022

View reviewed changes

.github/workflows/release.yml Outdated Show resolved Hide resolved

gtristan reviewed Aug 13, 2022

View reviewed changes

src/buildstream/utils.py Show resolved Hide resolved

ssssam force-pushed the sam/build-wheels branch from 171d9c9 to ba74a50 Compare August 15, 2022 11:27

gtristan reviewed Aug 16, 2022

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

gtristan reviewed Aug 16, 2022

View reviewed changes

.github/workflows/merge.yml Show resolved Hide resolved

gtristan reviewed Aug 16, 2022

View reviewed changes

.github/workflows/release.yml Outdated Show resolved Hide resolved

ssssam force-pushed the sam/build-wheels branch from 550d1ac to 9e97321 Compare August 16, 2022 15:19

ssssam marked this pull request as ready for review August 16, 2022 15:20

ssssam force-pushed the sam/build-wheels branch from 9e97321 to a5da072 Compare August 16, 2022 15:21

ssssam added 11 commits August 16, 2022 18:02

.github/workflows/merge.yml: Rename 'build' and 'publish'

203995c

We are going to build and publish more than just docs, so add _docs to these job names to make it clear.

Download BuildBox release binaries during wheel build

6c347a7

Add 'packaging' to requirements

ffd2413

This will be used in setup.py to process PEP440 version numbers. It's a small helper library which is already a transitive dependency of various tools we use including Tox, Sphinx, and Pytest. Regenerate *requirements.txt using requirements/Makefile.

tests/frontend/logging.py: Fix trivial pylint warning

8191928

Merge two strings on the same line into one.

docs: Update release process

4d120be

tox: Remove obsolete '-e release' mode

5700177

ssssam force-pushed the sam/build-wheels branch from a5da072 to 5700177 Compare August 16, 2022 16:04

cs-shadow reviewed Aug 16, 2022

View reviewed changes

.github/wheel-helpers/fetch-latest-buildbox-release.sh Show resolved Hide resolved

.github/wheel-helpers/fetch-latest-buildbox-release.sh Show resolved Hide resolved

setup.py Show resolved Hide resolved

setup.py Show resolved Hide resolved

src/buildstream/utils.py Show resolved Hide resolved

cs-shadow reviewed Aug 16, 2022

View reviewed changes

requirements/requirements.in Show resolved Hide resolved

gtristan mentioned this pull request Aug 17, 2022

Build and release wheel packages #1723

Merged

6 tasks

gtristan closed this Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Build and release wheel packages #1714

Draft: Build and release wheel packages #1714

ssssam commented Aug 3, 2022 •

edited

Loading

ssssam commented Aug 3, 2022

ssssam commented Aug 4, 2022 •

edited

Loading

ssssam commented Aug 5, 2022 •

edited

Loading

ssssam commented Aug 8, 2022

ssssam commented Aug 12, 2022 •

edited

Loading

ssssam commented Aug 12, 2022

ssssam commented Aug 12, 2022 •

edited

Loading

gtristan commented Aug 12, 2022

nanonyme commented Aug 12, 2022

gtristan commented Aug 13, 2022

gtristan commented Aug 13, 2022

nanonyme commented Aug 13, 2022

ssssam commented Aug 15, 2022

nanonyme commented Aug 15, 2022

gtristan commented Aug 15, 2022 •

edited

Loading

ssssam commented Aug 15, 2022

gtristan commented Aug 16, 2022

ssssam commented Aug 16, 2022 •

edited

Loading

ssssam commented Aug 16, 2022 •

edited

Loading

ssssam commented Aug 16, 2022

cs-shadow left a comment

gtristan commented Aug 17, 2022

Draft: Build and release wheel packages #1714

Draft: Build and release wheel packages #1714

Conversation

ssssam commented Aug 3, 2022 • edited Loading

ssssam commented Aug 3, 2022

ssssam commented Aug 4, 2022 • edited Loading

ssssam commented Aug 5, 2022 • edited Loading

Bunding buildbox binaries

ssssam commented Aug 8, 2022

ssssam commented Aug 12, 2022 • edited Loading

ssssam commented Aug 12, 2022

ssssam commented Aug 12, 2022 • edited Loading

gtristan commented Aug 12, 2022

nanonyme commented Aug 12, 2022

gtristan commented Aug 13, 2022

gtristan commented Aug 13, 2022

nanonyme commented Aug 13, 2022

ssssam commented Aug 15, 2022

nanonyme commented Aug 15, 2022

gtristan commented Aug 15, 2022 • edited Loading

ssssam commented Aug 15, 2022

gtristan commented Aug 16, 2022

ssssam commented Aug 16, 2022 • edited Loading

ssssam commented Aug 16, 2022 • edited Loading

ssssam commented Aug 16, 2022

cs-shadow left a comment

Choose a reason for hiding this comment

gtristan commented Aug 17, 2022

ssssam commented Aug 3, 2022 •

edited

Loading

ssssam commented Aug 4, 2022 •

edited

Loading

ssssam commented Aug 5, 2022 •

edited

Loading

ssssam commented Aug 12, 2022 •

edited

Loading

ssssam commented Aug 12, 2022 •

edited

Loading

gtristan commented Aug 15, 2022 •

edited

Loading

ssssam commented Aug 16, 2022 •

edited

Loading

ssssam commented Aug 16, 2022 •

edited

Loading