Skip to content

CI: Publish each master commit with a unique version on TestPyPI#728

Merged
adrienverge merged 1 commit intomasterfrom
ci/publish-to-testpypi-avoid-duplications
Mar 21, 2025
Merged

CI: Publish each master commit with a unique version on TestPyPI#728
adrienverge merged 1 commit intomasterfrom
ci/publish-to-testpypi-avoid-duplications

Conversation

@adrienverge
Copy link
Copy Markdown
Owner

This follows commit 7d52df7 "CI: Ignore version duplicates when publishing to TestPyPI" with a better design:

  • Only publish builds from master branch on TestPyPI.
  • Version every non-tag commit with a .devN suffix, e.g. 1.36.2.dev1. This prevents duplicates on TestPyPI.
  • twine check built packages.

See discussion at
#721 (comment) for more details.

Comment thread .github/workflows/publish.yaml Outdated
- name: Build a binary wheel and a source tarball
run: python -m build
- name: Twine check the distribution packages
run: python -m twine check --strict dist/yamllint-*
Copy link
Copy Markdown
Owner Author

@adrienverge adrienverge Mar 21, 2025

Choose a reason for hiding this comment

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

@webknjaz python -I (that you suggested at #721 (comment)) didn't work because in isolated mode, python doesn't find user-installed libraries:

/opt/hostedtoolcache/Python/3.13.2/x64/bin/python: No module named twine

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, I didn't realize. In general, it's discouraged to use --user because it's a global location with some things (like installed binaries) shared across several Python interpreters which may override each other.
I should really drop that from the guide… Nowadays, I default to having -I in all (or most of) my python invocations for safety reasons.

Technically, GH-provided VMs have pipx installed. So you could get away with just having pipx run build followed by a pipx run twine check --strict dists/*. If you want to pin versions, you can add --spec there.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Indeed I copied --user from the guide, but it's not needed :)

Commit modified with:

@@ -19,7 +19,7 @@ jobs:
         with:
           python-version: "3.x"
       - name: Install pypa/build
-        run: python -m pip install --user build twine
+        run: python -m pip install build twine
       - name: Add '.devN' to version for non-tag builds
         if: ${{ ! startsWith(github.ref, 'refs/tags/') }}
         run:
@@ -29,7 +29,7 @@ jobs:
       - name: Build a binary wheel and a source tarball
         run: python -m build
       - name: Twine check the distribution packages
-        run: python -m twine check --strict dist/yamllint-*
+        run: python -Im twine check --strict dist/yamllint-*
       - name: Store the distribution packages
         uses: actions/upload-artifact@v4
         with:

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 21, 2025

Coverage Status

coverage: 99.825%. remained the same
when pulling 1fb9da3 on ci/publish-to-testpypi-avoid-duplications
into cd96692 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 99.825%. remained the same
when pulling 400e595 on ci/publish-to-testpypi-avoid-duplications
into cd96692 on master.

Comment thread .github/workflows/publish.yaml Outdated

publish-to-testpypi:
name: Publish distribution package to TestPyPI
if: github.ref == 'refs/heads/master'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like using the github.ref_name var (it was added later than I first wrote the guide, I think: github.ref_name and github.ref_type are a bit more convenient). Also, there's a way to get the repo default branch w/o having to hardcode it, been using it for years.

Suggested change
if: github.ref == 'refs/heads/master'
if: >-
github.ref_name == github.event.repository.default_branch

Comment thread .github/workflows/publish.yaml Outdated
run: python -m pip install --user build
run: python -m pip install --user build twine
- name: Add '.devN' to version for non-tag builds
if: ${{ ! startsWith(github.ref, 'refs/tags/') }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I find it more convenient to use >- for escaping stuff and making it less nested. This allows us to drop the mustache:

Suggested change
if: ${{ ! startsWith(github.ref, 'refs/tags/') }}
if: >-
!startsWith(github.ref, 'refs/tags/')

Though, there's a nicer way to check for the same thing nowadays, more elegant:

Suggested change
if: ${{ ! startsWith(github.ref, 'refs/tags/') }}
if: github.ref_type != 'tag'

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

These are 2 good points.

Updated with your last suggestion.

(I also changed to if: github.ref_type == 'tag' in job publish-to-pypi for consistency)

- uses: actions/checkout@v4
with:
persist-credentials: false
- name: Fetch tags
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This can also be skipped for release requests:

Suggested change
- name: Fetch tags
- name: Fetch tags
if: github.ref_type != 'tag'

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good idea, done.

Comment on lines 12 to +17
- uses: actions/checkout@v4
with:
persist-credentials: false
- name: Fetch tags
run: git fetch --prune --unshallow --tags
Copy link
Copy Markdown

@webknjaz webknjaz Mar 21, 2025

Choose a reason for hiding this comment

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

Although, I just tend to build this right into the checkout step:

Suggested change
- uses: actions/checkout@v4
with:
persist-credentials: false
- name: Fetch tags
run: git fetch --prune --unshallow --tags
- uses: actions/checkout@v4
with:
fetch-depth: >-
${{
github.ref_type == 'tag'
&& 1
|| 0
}}
persist-credentials: false

(it's a ternary in GHA)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Smart, it would certainly work, but isn't more lightweight to only git fetch --prune --unshallow --tags (I think it fetches all ancestors of the current branch + all tags), rather to pull all history for all branches and tags (that's what fetch-depth: 0 does)?

By the way, it would be better to use fetch-tags: true in actions/checkout@v4, but I tried and it didn't work. According to the internet it might be currently bugged.

Copy link
Copy Markdown

@webknjaz webknjaz Mar 21, 2025

Choose a reason for hiding this comment

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

Smart, it would certainly work, but isn't more lightweight to only git fetch --prune --unshallow --tags (I think it fetches all ancestors of the current branch + all tags), rather to pull all history for all branches and tags (that's what fetch-depth: 0 does)?

It might be a little more efficient. I don't remember if --unshallow would get other branches or not.

Anyway, this only happens once in the workflow run. So it's not like it'd influence a lot.

By the way, it would be better to use fetch-tags: true in actions/checkout@v4, but I tried and it didn't work. According to the internet it might be currently bugged.

I don't think having tags detached from the tree would work. They have to be grafted and exist in the context. git describe --tags would need all the commits between FETCH_HEAD and the last tag to (1) count them, but also to know which of those tags is closest on the tree. And whether those tags are reachable from FETCH_HEAD. Timestamps mean nothing, it should be a tag of the branch. Imagine there's a tag on some other branch — that wouldn't be what git-describe finds. Similarly, imagine there's a tag on the very first initial commit in the repo that is dated 2099 — it also won't be what git-describe finds.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I don't think having tags detached from the tree would work. They have to be grafted and exist in the context. git describe --tags would need all the commits between FETCH_HEAD and the last tag to (1) count them, but also to know which of those tags is closest on the tree. And whether those tags are reachable from FETCH_HEAD. Timestamps mean nothing, it should be a tag of the branch. Imagine there's a tag on some other branch — that wouldn't be what git-describe finds. Similarly, imagine there's a tag on the very first initial commit in the repo that is dated 2099 — it also won't be what git-describe finds.

👍

Copy link
Copy Markdown

@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.

@adrienverge so it all looks functional, but I've also left a few improvement suggestions that you can choose to accept or not.

This follows commit 7d52df7 "CI: Ignore version duplicates when
publishing to TestPyPI" with a better design:
- Only publish builds from `master` branch on TestPyPI.
- Version every non-tag commit with a `.devN` suffix, e.g. `1.36.2.dev1`.
  This prevents duplicates on TestPyPI.
- `twine check` built packages.

See discussion at
#721 (comment)
for more details.
@adrienverge adrienverge force-pushed the ci/publish-to-testpypi-avoid-duplications branch from 400e595 to 1fb9da3 Compare March 21, 2025 13:05
Copy link
Copy Markdown
Owner Author

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

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

Thanks again @webknjaz!

Comment thread .github/workflows/publish.yaml Outdated
- name: Build a binary wheel and a source tarball
run: python -m build
- name: Twine check the distribution packages
run: python -m twine check --strict dist/yamllint-*
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Indeed I copied --user from the guide, but it's not needed :)

Commit modified with:

@@ -19,7 +19,7 @@ jobs:
         with:
           python-version: "3.x"
       - name: Install pypa/build
-        run: python -m pip install --user build twine
+        run: python -m pip install build twine
       - name: Add '.devN' to version for non-tag builds
         if: ${{ ! startsWith(github.ref, 'refs/tags/') }}
         run:
@@ -29,7 +29,7 @@ jobs:
       - name: Build a binary wheel and a source tarball
         run: python -m build
       - name: Twine check the distribution packages
-        run: python -m twine check --strict dist/yamllint-*
+        run: python -Im twine check --strict dist/yamllint-*
       - name: Store the distribution packages
         uses: actions/upload-artifact@v4
         with:

- uses: actions/checkout@v4
with:
persist-credentials: false
- name: Fetch tags
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good idea, done.

Comment on lines 12 to +17
- uses: actions/checkout@v4
with:
persist-credentials: false
- name: Fetch tags
run: git fetch --prune --unshallow --tags
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Smart, it would certainly work, but isn't more lightweight to only git fetch --prune --unshallow --tags (I think it fetches all ancestors of the current branch + all tags), rather to pull all history for all branches and tags (that's what fetch-depth: 0 does)?

By the way, it would be better to use fetch-tags: true in actions/checkout@v4, but I tried and it didn't work. According to the internet it might be currently bugged.

Comment thread .github/workflows/publish.yaml Outdated
run: python -m pip install --user build
run: python -m pip install --user build twine
- name: Add '.devN' to version for non-tag builds
if: ${{ ! startsWith(github.ref, 'refs/tags/') }}
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

These are 2 good points.

Updated with your last suggestion.

(I also changed to if: github.ref_type == 'tag' in job publish-to-pypi for consistency)

Copy link
Copy Markdown

@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.

Looks ready.

@adrienverge adrienverge merged commit 325fafa into master Mar 21, 2025
28 checks passed
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.

3 participants