Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use constraints file for sdist/wheel build + get version using build.util #3167

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

graingert
Copy link
Member

@graingert graingert commented Dec 23, 2024

stuff that I wanted to sneak into the zizmor PR, but decided should be in another PR

@graingert graingert changed the title add zizmor ci fixes after zizmor Dec 23, 2024
Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00000%. Comparing base (3260974) to head (c479af6).

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3167   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             124          124           
  Lines           18427        18427           
  Branches         1215         1215           
===============================================
  Hits            18427        18427           

@graingert graingert changed the title ci fixes after zizmor use contraints file for sdist/wheel build + get version using build.util Dec 24, 2024
@graingert graingert marked this pull request as ready for review December 24, 2024 08:13
@graingert graingert requested a review from webknjaz December 24, 2024 08:30
@webknjaz
Copy link
Member

If you're going this route already, let's also set PIP_CONSTRAINT in the pypa/build invocation step. I like using a separate constraint file generated with pip-tools, containing the build deps pins like setuptools+wheel. This will be able to pin the ephemeral PEP 517 build env.

@A5rocks A5rocks changed the title use contraints file for sdist/wheel build + get version using build.util use constraints file for sdist/wheel build + get version using build.util Dec 24, 2024
@@ -40,14 +40,19 @@ jobs:
with:
persist-credentials: false

- name: Install build
run: python -Im pip install build -c test-requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

This would leak into the underlying PEP 517 ephemeral build envs.

Suggested change
run: python -Im pip install build -c test-requirements.txt
env:
PIP_CONSTRAINT: test-requirements.txt
run: python -Im pip install build

@@ -58,9 +63,6 @@ jobs:
DIST_NAME: ${{ env.dist-name }}
VERSION: ${{ steps.dist-version.outputs.version }}

- name: Install build
run: python -Im pip install build

- name: Build dists
run: python -Im build
Copy link
Member

Choose a reason for hiding this comment

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

Assuming, you'll do something like pip-compile --only-build-deps --output-file=build-requirements.txt pyproject.toml:

Suggested change
run: python -Im build
env:
PIP_CONSTRAINT: build-requirements.txt
run: python -Im build

@@ -41,7 +41,7 @@ python -m pip install -U pip uv -c test-requirements.txt
python -m pip --version
python -m uv --version

python -m uv pip install build
python -m uv pip install build -c test-requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

This could also use an env var:

Suggested change
python -m uv pip install build -c test-requirements.txt
PIP_CONSTRAINT=test-requirements.txt python -m uv pip install build

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Additionally, I realized that this project doesn't have a combined workflow and so this https://github.com/python-trio/trio/blob/main/.github/workflows/release.yml also needs to be handled similarly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants