Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 58 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,54 @@ on:
pull_request:
branches: [ master ]

defaults:
run:
shell: bash

jobs:
build:
name: ${{ matrix.task.name}} - ${{ matrix.os.name }} ${{ matrix.python.name }}
runs-on: ${{ matrix.os.runs-on }}
strategy:
fail-fast: false
matrix:
os:
- name: Linux
runs-on: ubuntu-latest
python:
- name: CPython 3.8
tox: py38
action: 3.8
task:
- name: Build
tox: build

steps:
- uses: actions/checkout@v2

- name: Set up ${{ matrix.python.name }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python.action }}

- name: Install dependencies
run: python -m pip install --upgrade pip tox

- uses: twisted/python-info-action@v1

- name: Tox
run: tox -c tox.ini -e ${{ matrix.task.tox }}

- name: Publish
uses: actions/upload-artifact@v2
with:
name: dist
path: dist/

test:
name: ${{ matrix.task.name}} - ${{ matrix.os.name }} ${{ matrix.python.name }}
runs-on: ${{ matrix.os.runs-on }}
needs: build
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -55,18 +99,24 @@ jobs:
steps:
- uses: actions/checkout@v2

- name: Download package files
uses: actions/download-artifact@v2
with:
name: dist
path: dist/

- name: Set up ${{ matrix.python.name }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python.action }}

- uses: twisted/python-info-action@v1.0.1

- name: Install dependencies
run: python -m pip install --upgrade pip tox codecov

- name: Test
run: tox -c tox.ini -e ${{ matrix.python.tox }}-tests
- uses: twisted/python-info-action@v1

- name: Tox
run: tox -c tox.ini --installpkg dist/*.whl -e ${{ matrix.python.tox }}-tests

- name: Codecov
run: |
Expand Down Expand Up @@ -102,18 +152,19 @@ jobs:
with:
python-version: ${{ matrix.python.action }}

- uses: twisted/python-info-action@v1.0.1

- name: Install dependencies
run: python -m pip install --upgrade pip tox

- name: Check
- uses: twisted/python-info-action@v1

- name: Tox
run: tox -c tox.ini -e ${{ matrix.task.tox }}

all:
name: All
runs-on: ubuntu-latest
needs:
- build
- test
- check
steps:
Expand Down
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ include LICENSE
include CODE_OF_CONDUCT.md
include pyproject.toml
include tox.ini
include tox_build.sh
recursive-include src *.rst

exclude bin
Expand Down
8 changes: 8 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,11 @@ exclude = '''
)/
)
'''

[build-system]
requires = [
"setuptools ~= 44.1.1",
"wheel ~= 0.36.2",
"incremental == 17.5.0",
]
build-backend = "setuptools.build_meta"
6 changes: 5 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

from setuptools import setup, find_packages

# If incremental is not present then setuptools just silently uses v0.0.0 so
# let's import it and fail instead.
import incremental


setup(
name="towncrier",
maintainer="Amber Brown",
Expand Down Expand Up @@ -36,7 +41,6 @@
"Programming Language :: Python :: Implementation :: PyPy",
],
use_incremental=True,
setup_requires=["incremental"],
install_requires=["click", "click-default-group", "incremental", "jinja2", "toml"],
extras_require={"dev": ["packaging"]},
package_dir={"": "src"},
Expand Down
1 change: 1 addition & 0 deletions src/towncrier/newsfragments/314.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support building with PEP 517.
18 changes: 16 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[tox]
envlist = cov-erase, flake8, {pypy27,pypy36,pypy37,py27,py35,py36,py37,py38,py39}-{tests,flake8,check-manifest,check-newsfragment}, cov-report
isolated_build=true
skip_missing_envs = true

[testenv:flake8]
Expand Down Expand Up @@ -40,18 +41,31 @@ commands =
coverage run -p --module twisted.trial {posargs:towncrier}
coverage combine -a

[testenv:build]
allowlist_externals =
bash
changedir = {envtmpdir}
deps =
build
setenv =
toxinidir={toxinidir}
skip_install = true
commands =
# could be brought inside tox.ini after https://github.com/tox-dev/tox/issues/1571
bash {toxinidir}/tox_build.sh

[testenv:cov-report]
deps =
coverage

skip_install = true
commands =
coverage html
coverage report

[testenv:cov-erase]
deps =
coverage

skip_install = true
commands =
coverage erase

Expand Down
6 changes: 6 additions & 0 deletions tox_build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# build the sdist
python -m build --sdist --outdir ${toxinidir}/dist/ ${toxinidir}
tar -xvf ${toxinidir}/dist/*
cd *
# build the wheel from the sdist

Choose a reason for hiding this comment

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

Why do we need to build the wheel from the sdist, and not build the wheel directly? Feels like you don't need this indirection, you're only testing the correctness of setuptools backend at best.

Copy link
Member Author

Choose a reason for hiding this comment

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

It makes sure that the sdist was in the path of what is tested. Trio does it. Made enough sense to me to do it too. I don't know what errors I might be able to have that could make a wheel work but an sdist be broken. It may well be 'excessive' but it do you see harm that it could do? The overall goal is to make sure that the actual files to be published are what is tested. Not just that 'some wheel built in some situation is tested'. Likewise for sdist.

Copy link
Member

Choose a reason for hiding this comment

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

https://twistedmatrix.com/trac/ticket/10103 would have been caught if I'd have used this strategy

Choose a reason for hiding this comment

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

If you go down this path why not also test that direct wheel build works? I'm sure they'll be some edge case in some situation when src to sdist works, sdist to wheel also, but src to wheel does not 🤷‍♂️

Choose a reason for hiding this comment

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

And it's not some wheel in some situation. The builder ensures it's a pep 517/518 situation.

Choose a reason for hiding this comment

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

How do you generate your packages to upload to pypi?

Copy link
Member Author

Choose a reason for hiding this comment

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

This code that we are discussing. They are built, archived in GitHub Actions. Downloaded for tests and checks. If all is still well, they are downloaded and then published to PyPI. At least that is the presently-being-developed workflow. This PR does the first bit, #315 is adding the automated PyPI upload.

Choose a reason for hiding this comment

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

I see, thanks 👍🏻

Choose a reason for hiding this comment

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

Perhaps you want a wheel-from-sdist option for build project so you don't have to do this dance 🤷🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. We'll see what everyone else thinks I guess. pypa/build#257

python -m build --wheel --outdir ${toxinidir}/dist/ .