workflow and automation cleanups by sarnold · Pull Request #309 · cpplint/cpplint

sarnold · 2025-03-01T02:08:52Z

update workflow action/python versions and packaging, remove errors from setup.py
add workaround for py39 on macos installed with broken setuptools
update python matrix and expand tox file with build/check/clean cmds

Sorry, I couldn't wait for open PRs to merge so I tested the 2 C-compat PRs (#306 and #308) and this cleanup PR in the relevant workflow and our project CI is Green again. This PR also contains a GH release workflow to publish GH release from a tag push. Thanks!

* add workaround for py39 on macos installed with broken setuptools Signed-off-by: Stephen L Arnold <[email protected]>

aaronliu0130 · 2025-03-01T04:07:56Z

I don't understand the pupose of the tox.ini changes. And I don't like the idea of a release workflow as we also need to concurrently release on PyPI.

I actually plan to finish another refactoring of the workflow as you can see at #307 . Could you test that on non-Windows stuff and disregard the "deprecated module" error?

cclauss · 2025-03-02T05:58:30Z

.github/workflows/ci.yml

-        os: [ubuntu-latest, windows-latest]
      fail-fast: false
+      matrix:
+        os: [ubuntu-22.04, macos-latest, windows-latest]


Suggested change

os: [ubuntu-22.04, macos-latest, windows-latest]

# macOS in Intel, macOS on ARM, Ubuntu on x64, Ubuntu on ARM, Windows on X64

os: [macos-13, macos-latest, ubuntu-latest, ubuntu-24.04-arm, windows-latest, windows-2025]

cclauss · 2025-03-02T13:21:22Z

.github/workflows/ci.yml

          cache: 'pip'
          cache-dependency-path: '**/test-requirements'
+        env:
+          SETUPTOOLS_USE_DISTUTILS: ${{ matrix.os == 'macos-latest' && matrix.python-version == '3.9' && 'stdlib' || 'local' }}


Distutils is history. Let’s drop it.

Still required until py39 and py310 are history

cclauss · 2025-03-02T13:22:44Z

.github/workflows/ci.yml

          python -m pip install --upgrade pip
-          python -m pip install -e .[dev]
-          python -m pip install 'tox-gh-actions<4.0.0'
+          pip install tox tox-gh-actions


Not really needed with tox -e py in GitHub Actions.

tox -e py is implicitly selecting whatever python version is the environment default and we don't want that.

We specified the Python environment in the python action step.

cclauss · 2025-03-02T13:24:02Z

.github/workflows/release.yml

+        cache: 'pip'
+        cache-dependency-path: '**/test-requirements'
+      env:
+        SETUPTOOLS_USE_DISTUTILS: ${{ matrix.os == 'macos-latest' && matrix.python-version == '3.9' && 'stdlib' || 'local' }}


Distutils is history.

This is about specifically what versions of bundled python modules are installed with the (now older) python on the Github CI runners. It has nothing to do with the project packaging.

Yes, and we don't need distutils in any Python version.

aaronliu0130 · 2025-03-02T20:15:52Z

I can confirm that is not true; we use setuptools.

…

-- Cheers, Aᴀʀᴏɴ

…lease * py313 is not quite ready yet but everything else should be green Signed-off-by: Stephen L Arnold <[email protected]>

sarnold · 2025-03-03T02:27:01Z

I don't understand the pupose of the tox.ini changes. And I don't like the idea of a release workflow as we also need to concurrently release on PyPI.

It's just release automation, and should work for pypi if you uncomment the last job.

I actually plan to finish another refactoring of the workflow as you can see at #307 . Could you test that on non-Windows stuff and disregard the "deprecated module" error?

That ^^ refactor branch is broken in CI for the py313 issue, as well as on the windows CI runners. Works on linux and macos with the same distutils workaround.

aaronliu0130 · 2025-03-04T02:00:58Z

I fixed the Windows problem today.
This doesn't fix the py313 thing either.

aaronliu0130

Thanks for taking the time to write all this! We definitely could use release automation, but as it stands I feel like this is too overcomplicated. You'll also probably need to rework this after the tox.ini changes in #317 are merged.

aaronliu0130 · 2025-03-04T02:02:47Z

tox.ini

+    bash
+
+deps =
+    -e .[test,dev]


Is there an advantage for this over "extras"?

Visibility? otherwise it's just -e .

Does that change anything? AFAIK tox sandboxes the environment anyways.

aaronliu0130 · 2025-03-04T02:03:28Z

tox.ini

+passenv =
+    DISPLAY
+    XAUTHORITY
+    HOME
+    USERNAME
+    USER
+    XDG_*
+    CI
+    OS
+    PYTHONIOENCODING
+    PIP_DOWNLOAD_CACHE
+
+setenv =
+    PYTHONPATH = {toxinidir}
+
+allowlist_externals =
+    bash


Are these really needed?

aaronliu0130 · 2025-03-04T02:07:55Z

.github/workflows/release.yml

+    strategy:
+      fail-fast: false
+      matrix:
+        os: [macos-15, macos-latest, ubuntu-latest, ubuntu-22.04, ubuntu-24.04-arm, windows-latest, windows-2025]


I thought wheels were platform-independent.

non-platform wheels are, but the point here is the platform environments are different.

Hm, I don't quite get what you're saying. Could you elaborate on the possible differences?

I don't quite get what your objection and/or alternative is. Can you elaborate more on what you want? I'm not trying to be weird here, but the really simple/short workflows don't seem to "last" very long due to arbitrary changes in runner environments. I just had to fix coverage workflows in several different repos this week after GitHub suddenly decided to stop finding the right version ov llvm-cov (after the same workflows were working fine for literally years).

aaronliu0130 · 2025-03-04T02:08:06Z

tox.ini

@@ -1,20 +1,100 @@
 [tox]
-envlist = py38, py39, py3.10, py311, py312, pypy3
+envlist = pypy3,py3{9,10,11,12,13}-{linux,macos,windows}


I'm fairly sure a machine where you run tox can only have one OS loaded at a time. Is this needed?

That ^^ matrix is how tox constructs env commands, including PLATFORM matrix.

What is the PLATFORM matrix for?

aaronliu0130 · 2025-03-04T02:09:50Z

tox.ini

-  {envpython} -m pytest {posargs:}
-  {envpython} -m pylint cpplint.py
-  {envpython} -m flake8 cpplint.py
+    bash -c 'rm -rf build/ dist/ __pycache__/ *.egg-info/ .coverage'


Is there a reason to specifically use bash here? Why not

Suggested change

bash -c 'rm -rf build/ dist/ __pycache__/ *.egg-info/ .coverage'

rm -rf build/ dist/ __pycache__/ *.egg-info/ .coverage

In this case either bash or rm needs to be in the allow list. Bash is usually my default "allow" and that gets used to run any other shell commands. In this it's just "rm".

GitHub workflows have an allowlist for commands?

aaronliu0130 · 2025-03-04T02:10:29Z

.github/workflows/ci.yml

    runs-on: ${{ matrix.os }}
+    defaults:
+      run:
+        shell: bash


I don't see any bash-specific features used.

Better that than dash. It's generally better to set what you want and not let GH runner environment decide.

But we're only using sh features.

IMO this is a very good addition to any GHA workflow. Keeping to strictly sh is not normal behaviour for devs now - and there are some subtle gotchas when not using an explicit shell , c.f. rhysd/actionlint#374

cclauss · 2025-03-07T11:02:43Z

Please rebase because setup.py has been removed.

aaronliu0130 · 2025-03-07T20:13:00Z

There's too many problems and conflicts now that Christian's pyproject.toml has been merged. Please separate out the release workflow into a new PR if you wish to pursue it.

fix: dev: update action/python versions, packaging, and tox file

ae50765

* add workaround for py39 on macos installed with broken setuptools Signed-off-by: Stephen L Arnold <[email protected]>

cclauss reviewed Mar 2, 2025

View reviewed changes

sarnold force-pushed the workflows-packaging-tox branch 2 times, most recently from 574de9c to 2d52dec Compare March 3, 2025 02:20

chg: address PR feedback, update ci matrix, add pypi upload job to re…

5503276

…lease * py313 is not quite ready yet but everything else should be green Signed-off-by: Stephen L Arnold <[email protected]>

sarnold force-pushed the workflows-packaging-tox branch from 2d52dec to 5503276 Compare March 3, 2025 02:23

aaronliu0130 requested changes Mar 4, 2025

View reviewed changes

aaronliu0130 closed this Mar 7, 2025

	os: [ubuntu-22.04, macos-latest, windows-latest]
	# macOS in Intel, macOS on ARM, Ubuntu on x64, Ubuntu on ARM, Windows on X64
	os: [macos-13, macos-latest, ubuntu-latest, ubuntu-24.04-arm, windows-latest, windows-2025]

	bash -c 'rm -rf build/ dist/ __pycache__/ *.egg-info/ .coverage'
	rm -rf build/ dist/ __pycache__/ *.egg-info/ .coverage

Conversation

sarnold commented Mar 1, 2025

Uh oh!

aaronliu0130 commented Mar 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 commented Mar 2, 2025 via email

Uh oh!

sarnold commented Mar 3, 2025

Uh oh!

aaronliu0130 commented Mar 4, 2025

Uh oh!

aaronliu0130 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnold Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cclauss commented Mar 7, 2025

Uh oh!

aaronliu0130 commented Mar 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

sarnold Mar 7, 2025 •

edited

Loading