Skip to content

Revert "CI: rework caching and build/test environment" #31

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 3, 2025
Merged

Revert "CI: rework caching and build/test environment" #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 39 additions & 33 deletions .github/actions/common-setup/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,49 @@ inputs:
description: "Python version to setup"
required: true
default: "3.13"
poetry_version:
description: "Poetry version to setup"
required: true
default: "2.1.3"
runs:
using: "composite"
steps:
- name: Install Poetry
run: pipx install poetry==${{ inputs.poetry_version }}
shell: bash
- name: Set up Python 🐍
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
with:
python-version: ${{ inputs.python_version }}

- name: Default shell
run: |
if [ "$RUNNER_OS" == "Linux" ]; then
echo shellos=bash >> $GITHUB_ENV
elif [ "$RUNNER_OS" == "Windows" ]; then
echo shellos=powershell >> $GITHUB_ENV
else
echo "$RUNNER_OS not supported"
exit 1
fi
shell: bash

- name: Set up Python 🐍
id: setup-python
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
with:
python-version: ${{ inputs.python_version }}
cache: poetry
cache-dependency-path: poetry.lock
- name: Install Poetry
uses: abatilo/actions-poetry@65c61eae400c65c9510a584af85138c1ae19bbc0 # v3.0.2
with:
poetry-version: 2.1.3
Copy link

@gemini-code-assist gemini-code-assist bot Aug 3, 2025

Choose a reason for hiding this comment

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

medium

The poetry.lock file indicates it was generated with Poetry version 2.1.1, but this step installs version 2.1.3. Using a different version of Poetry than the one used to generate the lock file can lead to unexpected behavior or dependency resolution issues. It's best practice to use the same version to ensure consistency and avoid potential problems.

poetry-version: 2.1.1


- name: Set Poetry environment
run: poetry env use ${{ inputs.python_version }}
shell: bash
# Cache your dependencies (i.e. all the stuff in your `pyproject.toml`). Note the cache
# key: if you're using multiple Python versions, or multiple OSes, you'd need to include
# them in the cache key. I'm not, so it can be simple and just depend on the poetry.lock.
- name: cache deps
id: cache-deps
uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
with:
path: ~/.cache/pypoetry
Copy link

@gemini-code-assist gemini-code-assist bot Aug 3, 2025

Choose a reason for hiding this comment

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

high

The cache path is hardcoded to ~/.cache/pypoetry, which is the default for Linux. This will not work correctly on Windows runners, where the path is typically %LOCALAPPDATA%\pypoetry\Cache. Since this workflow is designed to support Windows, this will cause caching to fail on that platform.

To fix this, you should dynamically determine the cache directory after Poetry is installed. For example, you could add a step before this one to get the path:

- name: Get poetry cache directory
  id: poetry-cache
  run: echo "dir=$(poetry config cache-dir)" >> $GITHUB_OUTPUT
  shell: bash

Then, you can use path: ${{ steps.poetry-cache.outputs.dir }} in this step.

key: pydeps-${{ inputs.python_version }}-${{ env.shellos }}-${{ hashFiles('**/poetry.lock') }}

- name: Default shell
run: |
if [ "$RUNNER_OS" == "Linux" ]; then
echo shellos=bash >> $GITHUB_ENV
elif [ "$RUNNER_OS" == "Windows" ]; then
echo shellos=powershell >> $GITHUB_ENV
else
echo "$RUNNER_OS not supported"
exit 1
fi
shell: bash
# Install dependencies. `--no-root` means "install all dependencies but not the project
# itself", which is what you want to avoid caching _your_ code. The `if` statement
# ensures this only runs on a cache miss.
- name: Install dependencies
run: poetry install --no-interaction --no-root
shell: ${{ env.shellos }}
if: steps.cache-deps.outputs.cache-hit != 'true'
Copy link

@gemini-code-assist gemini-code-assist bot Aug 3, 2025

Choose a reason for hiding this comment

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

high

This if condition causes the dependency installation to be skipped on a cache hit. However, the cache being used stores downloaded package artifacts, not the fully installed virtual environment. poetry install is still required to create the venv and install dependencies into it, even if the packages are already cached.

This logic is flawed:

  • On a cache hit, this step is skipped, and the subsequent Install project step must install all dependencies.
  • On a cache miss, poetry install runs here and again in the next step, which is inefficient.

Consider removing this condition to make the workflow more robust and efficient.


# The 'setup-python' action with 'cache: poetry' installs dependencies but not the project
# itself. This step installs the project package into the created virtual environment.
- name: Install project
run: poetry install --no-interaction
shell: ${{ env.shellos }}
- name: Install project
run: poetry install --no-interaction
shell: ${{ env.shellos }}
21 changes: 2 additions & 19 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,49 +16,37 @@ jobs:
strategy:
matrix:
task: ["fmt", "lint"]
python-version: ["3.10", "3.11", "3.12", "3.13"]

steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- uses: ./.github/actions/common-setup
with:
python_version: ${{ matrix.python-version }}

- name: check code
run: poetry run poe ${{ matrix.task }}

build:
needs: code_checks
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.10", "3.11", "3.12", "3.13"]

steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- uses: ./.github/actions/common-setup
with:
python_version: ${{ matrix.python-version }}

- name: Build 🔨
run: poetry build

build_windows:
needs: code_checks
runs-on: windows-latest
strategy:
matrix:
python-version: ["3.10", "3.11", "3.12", "3.13"]

steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- uses: ./.github/actions/common-setup
with:
python_version: ${{ matrix.python-version }}
poetry_version: 2.1.3
python_version: 3.13

- name: Add entrypoint to bypass issue with relative imports in PyInstaller
run: powershell -Command 'Invoke-WebRequest https://gist.githubusercontent.com/Wenzel/e38d227d94f16e026b3aed03ea6a6661/raw/383ec56d62c58e444f6c5962ee6940a5c583d341/stub.py -OutFile stub.py'
Expand All @@ -68,7 +56,6 @@ jobs:
shell: bash

- name: Upload Windows release artefact
if: ${{ matrix.python-version == 3.13 }}
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
with:
name: checksec.exe
Expand All @@ -80,12 +67,10 @@ jobs:
shell: bash

test:
needs: ['build', 'build_windows']

needs: build
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
python-version: ["3.10", "3.11", "3.12", "3.13"]
runs-on: ${{ matrix.os }}
defaults:
run:
Expand All @@ -97,8 +82,6 @@ jobs:
submodules: true

- uses: ./.github/actions/common-setup
with:
python_version: ${{ matrix.python-version }}

- name: Run tests
run: poetry run poe test_e2e
Expand Down
Loading