You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by GitBox <gi...@apache.org> on 2022/08/03 16:18:51 UTC
[GitHub] [buildstream] ssssam opened a new pull request, #1714: Draft: Build and release wheel packages
ssssam opened a new pull request, #1714:
URL: https://github.com/apache/buildstream/pull/1714
See https://github.com/apache/buildstream/issues/1712 for rationale.
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/ and use some GitHub Actions to call the relevant commandline tooling.
Before actually landing this, we would need to make some changes:
* confirm we do want to release wheels going forwards
* finish work to embed BuildBox in the wheels (see #1712)
* release to the real PyPI instead of [Test PyPI](https://test.pypi.org/)
* create and add a [PyPI API token](https://pypi.org/help/#apitoken) as a [repository secret](https://docs.github.com/en/codespaces/managing-codespaces-for-your-organization/managing-encrypted-secrets-for-your-repository-and-organization-for-github-codespaces)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946909679
##########
.github/workflows/ci.yml:
##########
@@ -108,3 +108,55 @@ jobs:
with:
name: docs
path: doc/build/html
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
Review Comment:
The issue that the `os` variable is not used at all, but a different variable named `test-name` :) fixed now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1214783550
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](https://peps.python.org/pep-0517/), 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](https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html) or [setup.cfg](https://setuptools.pypa.io/en/latest/userguide/declarative_config.html), 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] nanonyme commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1214175429
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946966693
##########
.github/workflows/merge.yml:
##########
@@ -6,8 +6,8 @@ on:
- master
jobs:
- build:
Review Comment:
That is fine to keep, but let’s have that in a commit which does explicitly that :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1213027079
pylint task is failing, with error about unrecognised option "no-space-check". Confusingly this was removed [in pylint 2.6](https://pylint.pycqa.org/en/latest/whatsnew/2/2.6/summary.html#summary-release-highlights).
pylint was updated in this branch, because:
1. i used 'packaging.version' in setup.py
2. i added 'packaging' to requirements/requirements.in
3. i ran `make` in requirements/
4. it generated a whole new requirements.txt which includes an update for pylint from 2.13 to 2.14
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1214926455
> [...]
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1208265000
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](https://gitlab.com/BuildGrid/buildbox/buildbox-integration/-/issues/1).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1214036135
> 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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] nanonyme commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1214883893
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1215418869
> 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)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r945960309
##########
src/buildstream/utils.py:
##########
@@ -575,20 +576,27 @@ def link_files(
return result
-def get_host_tool(name: str) -> str:
+def get_host_tool(
+ name: str,
+ search_subprojects_dir: Optional[str] = None,
Review Comment:
Great point! Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] nanonyme commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1213409809
I'm a bit concerned how the plan is to update buildbox components. Does any update mean making a new BuildStream release?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r945114668
##########
src/buildstream/utils.py:
##########
@@ -575,20 +576,27 @@ def link_files(
return result
-def get_host_tool(name: str) -> str:
+def get_host_tool(
+ name: str,
+ search_subprojects_dir: Optional[str] = None,
Review Comment:
Let's not expose this in public API.
Better to have a `_get_host_tool_internal()` with the added option for internal use and keep public facing `get_host_tool()` as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1206527116
## 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](https://setuptools.pypa.io/en/latest/build_meta.html) 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'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946880465
##########
.github/workflows/release.yml:
##########
@@ -32,9 +32,114 @@ jobs:
tar -C doc/build/html -zcf docs.tgz .
+ - uses: actions/upload-artifact@v3
+ with:
+ name: docs
+ path: docs.tgz
+
+ build_sdist:
+ name: "Build Python source distribution tarball"
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Build sdist
+ run: pipx run build --sdist
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: sdist
+ path: dist/*.tar.gz
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
+ needs: [build_wheels]
+ runs-on: ubuntu-20.04
+
+ strategy:
+ matrix:
+ # The names here should map to a valid service defined in
+ # "../compose/ci.docker-compose.yml"
+ test-name:
+ - wheels-manylinux_2_28-cp37
+ - wheels-manylinux_2_28-cp38
+ - wheels-manylinux_2_28-cp39
+ - wheels-manylinux_2_28-cp310
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse
+
+ - name: Run tests with Docker Compose
+ run: |
+ ${GITHUB_WORKSPACE}/.github/run-ci.sh ${{ matrix.test-name }}
+
+ upload_github_release:
+ name: Upload GitHub release assets
+ needs: [build_docs, build_sdist, build_wheels, test_wheels]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: docs
+
- name: Upload release assets
run: |
tag_name="${GITHUB_REF##*/}"
hub release create -a "docs.tgz" -m "$tag_name" "$tag_name"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+ upload_pypi_release:
+ name: Upload TEST PyPI release assets
+ needs: [build_docs, build_sdist, build_wheels, test_wheels]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/download-artifact@v3
+ with:
+ name: sdist
+ path: dist
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: dist
+
+ - name: Upload to TEST PyPI
+ run: |
+ pipx run twine upload --repository testpypi --username __token__ --password "${{ secrets.TEST_PYPI_TOKEN }}" dist/*
Review Comment:
Sure. For the token, follow https://pypi.org/help/#apitoken using the PyPI account you use to release BuildStream. Then, save it as a [repository secret](https://docs.github.com/en/codespaces/managing-codespaces-for-your-organization/managing-encrypted-secrets-for-your-repository-and-organization-for-github-codespaces) named `secrets.PYPI_TOKEN`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946905158
##########
.github/workflows/merge.yml:
##########
@@ -6,8 +6,8 @@ on:
- master
jobs:
- build:
Review Comment:
It seems clearer to me to name this build_docs and publish_docs, the old name "build" is unclear. But we can remove if you want to avoid unrelated changes in this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r945114361
##########
.github/workflows/release.yml:
##########
@@ -32,9 +32,114 @@ jobs:
tar -C doc/build/html -zcf docs.tgz .
+ - uses: actions/upload-artifact@v3
+ with:
+ name: docs
+ path: docs.tgz
+
+ build_sdist:
+ name: "Build Python source distribution tarball"
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Build sdist
+ run: pipx run build --sdist
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: sdist
+ path: dist/*.tar.gz
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
+ needs: [build_wheels]
+ runs-on: ubuntu-20.04
+
+ strategy:
+ matrix:
+ # The names here should map to a valid service defined in
+ # "../compose/ci.docker-compose.yml"
+ test-name:
+ - wheels-manylinux_2_28-cp37
+ - wheels-manylinux_2_28-cp38
+ - wheels-manylinux_2_28-cp39
+ - wheels-manylinux_2_28-cp310
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse
+
+ - name: Run tests with Docker Compose
+ run: |
+ ${GITHUB_WORKSPACE}/.github/run-ci.sh ${{ matrix.test-name }}
+
+ upload_github_release:
+ name: Upload GitHub release assets
+ needs: [build_docs]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: docs
+
- name: Upload release assets
run: |
tag_name="${GITHUB_REF##*/}"
hub release create -a "docs.tgz" -m "$tag_name" "$tag_name"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+ upload_pypi_release:
+ name: Upload TEST PyPI release assets
+ needs: [build_sdist, build_wheels, test_wheels]
Review Comment:
Same here, probably best to also depend on the successful documentation build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r945633394
##########
.github/workflows/release.yml:
##########
@@ -32,9 +32,114 @@ jobs:
tar -C doc/build/html -zcf docs.tgz .
+ - uses: actions/upload-artifact@v3
+ with:
+ name: docs
+ path: docs.tgz
+
+ build_sdist:
+ name: "Build Python source distribution tarball"
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Build sdist
+ run: pipx run build --sdist
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: sdist
+ path: dist/*.tar.gz
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
+ needs: [build_wheels]
+ runs-on: ubuntu-20.04
+
+ strategy:
+ matrix:
+ # The names here should map to a valid service defined in
+ # "../compose/ci.docker-compose.yml"
+ test-name:
+ - wheels-manylinux_2_28-cp37
+ - wheels-manylinux_2_28-cp38
+ - wheels-manylinux_2_28-cp39
+ - wheels-manylinux_2_28-cp310
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse
+
+ - name: Run tests with Docker Compose
+ run: |
+ ${GITHUB_WORKSPACE}/.github/run-ci.sh ${{ matrix.test-name }}
+
+ upload_github_release:
+ name: Upload GitHub release assets
+ needs: [build_docs]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: docs
+
- name: Upload release assets
run: |
tag_name="${GITHUB_REF##*/}"
hub release create -a "docs.tgz" -m "$tag_name" "$tag_name"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+ upload_pypi_release:
+ name: Upload TEST PyPI release assets
+ needs: [build_sdist, build_wheels, test_wheels]
Review Comment:
Good point, resolved. See new test release: https://github.com/ssssam/buildstream/actions/runs/2860225838
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1216844632
One final update 5700177c5095d82c07bf4ef881b01afff5f442ee which splits the ".github/workflows/merge.yml: Rename 'build' and 'publish'" commit out from "Build and release wheels packages as part of GitHub Actions".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1205065084
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1204185640
Apologies, i also didn't make this a draft correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1212976352
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946866531
##########
.github/workflows/release.yml:
##########
@@ -32,9 +32,114 @@ jobs:
tar -C doc/build/html -zcf docs.tgz .
+ - uses: actions/upload-artifact@v3
+ with:
+ name: docs
+ path: docs.tgz
+
+ build_sdist:
+ name: "Build Python source distribution tarball"
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Build sdist
+ run: pipx run build --sdist
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: sdist
+ path: dist/*.tar.gz
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
+ needs: [build_wheels]
+ runs-on: ubuntu-20.04
+
+ strategy:
+ matrix:
+ # The names here should map to a valid service defined in
+ # "../compose/ci.docker-compose.yml"
+ test-name:
+ - wheels-manylinux_2_28-cp37
+ - wheels-manylinux_2_28-cp38
+ - wheels-manylinux_2_28-cp39
+ - wheels-manylinux_2_28-cp310
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse
+
+ - name: Run tests with Docker Compose
+ run: |
+ ${GITHUB_WORKSPACE}/.github/run-ci.sh ${{ matrix.test-name }}
+
+ upload_github_release:
+ name: Upload GitHub release assets
+ needs: [build_docs, build_sdist, build_wheels, test_wheels]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: docs
+
- name: Upload release assets
run: |
tag_name="${GITHUB_REF##*/}"
hub release create -a "docs.tgz" -m "$tag_name" "$tag_name"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+ upload_pypi_release:
+ name: Upload TEST PyPI release assets
+ needs: [build_docs, build_sdist, build_wheels, test_wheels]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/download-artifact@v3
+ with:
+ name: sdist
+ path: dist
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: dist
+
+ - name: Upload to TEST PyPI
+ run: |
+ pipx run twine upload --repository testpypi --username __token__ --password "${{ secrets.TEST_PYPI_TOKEN }}" dist/*
Review Comment:
Can we change this to `pypi` (instead of `testpypi`) before merging, and perhaps let me know what I need to know to setup the token ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946866531
##########
.github/workflows/release.yml:
##########
@@ -32,9 +32,114 @@ jobs:
tar -C doc/build/html -zcf docs.tgz .
+ - uses: actions/upload-artifact@v3
+ with:
+ name: docs
+ path: docs.tgz
+
+ build_sdist:
+ name: "Build Python source distribution tarball"
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Build sdist
+ run: pipx run build --sdist
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: sdist
+ path: dist/*.tar.gz
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
+ needs: [build_wheels]
+ runs-on: ubuntu-20.04
+
+ strategy:
+ matrix:
+ # The names here should map to a valid service defined in
+ # "../compose/ci.docker-compose.yml"
+ test-name:
+ - wheels-manylinux_2_28-cp37
+ - wheels-manylinux_2_28-cp38
+ - wheels-manylinux_2_28-cp39
+ - wheels-manylinux_2_28-cp310
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse
+
+ - name: Run tests with Docker Compose
+ run: |
+ ${GITHUB_WORKSPACE}/.github/run-ci.sh ${{ matrix.test-name }}
+
+ upload_github_release:
+ name: Upload GitHub release assets
+ needs: [build_docs, build_sdist, build_wheels, test_wheels]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: docs
+
- name: Upload release assets
run: |
tag_name="${GITHUB_REF##*/}"
hub release create -a "docs.tgz" -m "$tag_name" "$tag_name"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+ upload_pypi_release:
+ name: Upload TEST PyPI release assets
+ needs: [build_docs, build_sdist, build_wheels, test_wheels]
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/download-artifact@v3
+ with:
+ name: sdist
+ path: dist
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: dist
+
+ - name: Upload to TEST PyPI
+ run: |
+ pipx run twine upload --repository testpypi --username __token__ --password "${{ secrets.TEST_PYPI_TOKEN }}" dist/*
Review Comment:
Can we change this to `pypi` before merging, and perhaps let me know what I need to know to setup the token ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1216784656
Updates in [9e97321](https://github.com/apache/buildstream/commit/9e97321f8519018703fe89737dbcdb87dd887ddd) tip:
* Release to real PyPI (dependent on secrets.PYPI_TOKEN being set)
* Fix the 'Test wheels' action names
* Squash fixups into main commits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1217688940
This is replaced by #1723, just finishing up there since @ssssam is off for the rest of the week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan closed pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan closed pull request #1714: Draft: Build and release wheel packages
URL: https://github.com/apache/buildstream/pull/1714
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946863445
##########
.github/workflows/ci.yml:
##########
@@ -108,3 +108,55 @@ jobs:
with:
name: docs
path: doc/build/html
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
Review Comment:
This is not working correctly, I wonder if `Test Python wheel packages on "${{ matrix.os }}"` might work.
Currently jobs displayed on this PR display 4 different instances of just `Test Python wheel packages on` which looks confusing.
Minor issue, but would be useful to see it in the UI nonetheless.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r945114314
##########
.github/workflows/release.yml:
##########
@@ -32,9 +32,114 @@ jobs:
tar -C doc/build/html -zcf docs.tgz .
+ - uses: actions/upload-artifact@v3
+ with:
+ name: docs
+ path: docs.tgz
+
+ build_sdist:
+ name: "Build Python source distribution tarball"
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Build sdist
+ run: pipx run build --sdist
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: sdist
+ path: dist/*.tar.gz
+
+ build_wheels:
+ name: Build Python wheel packages on ${{ matrix.os }}
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os: [ubuntu-20.04]
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - name: Fetch latest BuildBox release
+ run: ${GITHUB_WORKSPACE}/.github/wheel-helpers/fetch-latest-buildbox-release.sh
+
+ - name: Build wheels
+ run: pipx run cibuildwheel==2.8.1
+
+ - uses: actions/upload-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse/*.whl
+
+ test_wheels:
+ name: Test Python wheel packages on ${{ matrix.os }}
+ needs: [build_wheels]
+ runs-on: ubuntu-20.04
+
+ strategy:
+ matrix:
+ # The names here should map to a valid service defined in
+ # "../compose/ci.docker-compose.yml"
+ test-name:
+ - wheels-manylinux_2_28-cp37
+ - wheels-manylinux_2_28-cp38
+ - wheels-manylinux_2_28-cp39
+ - wheels-manylinux_2_28-cp310
+
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: wheels
+ path: ./wheelhouse
+
+ - name: Run tests with Docker Compose
+ run: |
+ ${GITHUB_WORKSPACE}/.github/run-ci.sh ${{ matrix.test-name }}
+
+ upload_github_release:
+ name: Upload GitHub release assets
+ needs: [build_docs]
Review Comment:
I think that uploading the `docs.tgz` and publishing the wheels should be contingent on the same steps, we don't want to upload the docs if testing the releases fail and vice versa.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1214049069
@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...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] cs-shadow commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
cs-shadow commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r947275564
##########
.github/wheel-helpers/fetch-latest-buildbox-release.sh:
##########
@@ -0,0 +1,16 @@
+#!/bin/bash
+
+# Download latest release binaries of BuildBox. These are statically linked
+# binaries produced by the buildbox-integration GitLab project, which we
+# bundle into BuildStream wheel packages.
+
+set -eux
+
+wget https://gitlab.com/buildgrid/buildbox/buildbox-integration/-/releases/permalink/latest/downloads/binaries.tgz
+
+mkdir -p src/buildstream/subprojects/buildbox
Review Comment:
minor: I wonder if `_vendor` would be a better name for what this directory is actually doing.
##########
.github/wheel-helpers/fetch-latest-buildbox-release.sh:
##########
@@ -0,0 +1,16 @@
+#!/bin/bash
+
+# Download latest release binaries of BuildBox. These are statically linked
+# binaries produced by the buildbox-integration GitLab project, which we
+# bundle into BuildStream wheel packages.
+
+set -eux
+
+wget https://gitlab.com/buildgrid/buildbox/buildbox-integration/-/releases/permalink/latest/downloads/binaries.tgz
Review Comment:
```suggestion
curl -O https://gitlab.com/buildgrid/buildbox/buildbox-integration/-/releases/permalink/latest/downloads/binaries.tgz
```
minor: curl may be slightly more ubiquitous
##########
src/buildstream/utils.py:
##########
@@ -575,20 +576,27 @@ def link_files(
return result
-def get_host_tool(name: str) -> str:
+def get_host_tool(
+ name: str,
+ search_subprojects_dir: Optional[str] = None,
Review Comment:
I suppose the `search_subprojects_dir` argument (along with the line in the docstring below) should be removed now
##########
setup.py:
##########
@@ -64,6 +104,59 @@
sys.exit(1)
+############################################################
+# List the BuildBox binaries to ship in the wheel packages #
+############################################################
+#
+# BuildBox isn't widely available in OS distributions. To enable a "one click"
+# install for BuildStream, we bundle prebuilt BuildBox binaries in our binary
+# wheel packages.
+#
+# The binaries are provided by the buildbox-integration Gitlab project:
+# https://gitlab.com/BuildGrid/buildbox/buildbox-integration
+#
+# If you want to build a wheel with the BuildBox binaries included, set the
+# env var "BST_BUNDLE_BUILDBOX=1" when running setup.py.
+
+try:
+ BUNDLE_BUILDBOX = int(os.environ.get("BST_BUNDLE_BUILDBOX", "0"))
+except ValueError:
+ print("BST_BUNDLE_BUILDBOX must be an integer. Please set it to '1' to enable, '0' to disable", file=sys.stderr)
+ raise SystemExit(1)
+
+
+def list_buildbox_binaries():
+ expected_binaries = [
+ "buildbox-casd",
+ "buildbox-fuse",
+ "buildbox-run",
+ ]
+
+ if BUNDLE_BUILDBOX:
+ bst_package_dir = Path(__file__).parent.joinpath("src/buildstream")
+ buildbox_dir = bst_package_dir.joinpath("subprojects", "buildbox")
+ buildbox_binaries = [buildbox_dir.joinpath(name) for name in expected_binaries]
+
+ missing_binaries = [path for path in buildbox_binaries if not path.is_file()]
+ if missing_binaries:
+ paths_text = "\n".join([" * {}".format(path) for path in missing_binaries])
+ print(
+ "Expected BuildBox binaries were not found. "
+ "Set BST_BUNDLE_BUILDBOX=0 or provide:\n\n"
+ "{}\n".format(paths_text)
Review Comment:
```suggestion
"{}\n".format(paths_text),
file=sys.stderr,
```
##########
setup.py:
##########
@@ -64,6 +104,59 @@
sys.exit(1)
+############################################################
+# List the BuildBox binaries to ship in the wheel packages #
+############################################################
+#
+# BuildBox isn't widely available in OS distributions. To enable a "one click"
+# install for BuildStream, we bundle prebuilt BuildBox binaries in our binary
+# wheel packages.
+#
+# The binaries are provided by the buildbox-integration Gitlab project:
+# https://gitlab.com/BuildGrid/buildbox/buildbox-integration
+#
+# If you want to build a wheel with the BuildBox binaries included, set the
+# env var "BST_BUNDLE_BUILDBOX=1" when running setup.py.
+
+try:
+ BUNDLE_BUILDBOX = int(os.environ.get("BST_BUNDLE_BUILDBOX", "0"))
+except ValueError:
+ print("BST_BUNDLE_BUILDBOX must be an integer. Please set it to '1' to enable, '0' to disable", file=sys.stderr)
+ raise SystemExit(1)
+
+
+def list_buildbox_binaries():
+ expected_binaries = [
+ "buildbox-casd",
+ "buildbox-fuse",
+ "buildbox-run",
+ ]
+
+ if BUNDLE_BUILDBOX:
+ bst_package_dir = Path(__file__).parent.joinpath("src/buildstream")
+ buildbox_dir = bst_package_dir.joinpath("subprojects", "buildbox")
+ buildbox_binaries = [buildbox_dir.joinpath(name) for name in expected_binaries]
+
+ missing_binaries = [path for path in buildbox_binaries if not path.is_file()]
+ if missing_binaries:
+ paths_text = "\n".join([" * {}".format(path) for path in missing_binaries])
+ print(
+ "Expected BuildBox binaries were not found. "
+ "Set BST_BUNDLE_BUILDBOX=0 or provide:\n\n"
+ "{}\n".format(paths_text)
+ )
+ raise SystemExit(1)
+
+ for path in buildbox_binaries:
+ if path.is_symlink():
+ print("Bundled BuildBox binaries must not be symlinks. Please fix {}".format(path))
Review Comment:
```suggestion
print("Bundled BuildBox binaries must not be symlinks. Please fix {}".format(path), file=sys.stderr)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] cs-shadow commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
cs-shadow commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r947285404
##########
requirements/requirements.in:
##########
@@ -1,6 +1,7 @@
Click >= 7.0
grpcio
Jinja2 >= 2.10
+packaging
Review Comment:
oh I missed one thing, I don't think this is the right place for this requirement as we don't need it at runtime.
Since it's only needed at build time, it should only be in the `requires` section of `pyproject.toml`. And it seems like you've already added it there so I believe you can just remove this line
(this should also have an agreeable side-effect of removing all the dependency version changes from the diff)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1216389520
[...]
>
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1216650909
Updates today:
* Move build_wheel and test_wheel to ci.yml from merge.yml - you can see them in the 'PR Checks' section above. (I also did a new test release at https://github.com/ssssam/buildstream/actions/runs/2868187250)
* Use BuildBox binaries from [BuildGrid/buildbox/buildbox-integration](https://gitlab.com/BuildGrid/buildbox/buildbox-integration/-/releases) now they are available (we were using [tristanvb/buildbox-integration](https://gitlab.com/tristanvb/buildbox-integration/-/releases/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946979556
##########
.github/workflows/merge.yml:
##########
@@ -6,8 +6,8 @@ on:
- master
jobs:
- build:
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1213070025
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on a diff in pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
gtristan commented on code in PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#discussion_r946864311
##########
.github/workflows/merge.yml:
##########
@@ -6,8 +6,8 @@ on:
- master
jobs:
- build:
Review Comment:
Maybe we can remove the changes made to `.github/workflows/merge.yml` now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] ssssam commented on pull request #1714: Draft: Build and release wheel packages
Posted by GitBox <gi...@apache.org>.
ssssam commented on PR #1714:
URL: https://github.com/apache/buildstream/pull/1714#issuecomment-1213019030
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org