Skip to content

Stabilize the pip check lint step - #9803

Open
camd wants to merge 1 commit into
masterfrom
camd/stabilize-pip-check
Open

Stabilize the pip check lint step#9803
camd wants to merge 1 commit into
masterfrom
camd/stabilize-pip-check

Conversation

@camd

@camd camd commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

Dependabot's pip-tools 7.6.1 bump (#9788) fails the builds CI job. The failure is a true positive from the pip check step in tox -e linters: recompiling requirements/dev.txt with pip-tools' default settings drops the setuptools pin (pip-compile treats pip/setuptools as "unsafe" and omits them unless --allow-unsafe is passed). Since CI installs requirements with --no-deps into a Python 3.13 venv — where virtualenv no longer seeds setuptools — the zope-* packages and pip-tools itself end up with an unmet setuptools requirement.

Separately, the pip check invocation had grown a fragile bash construct that filters expected output with grep -v and fails if any output remains. It already needed one follow-up fix (ignoring the "No broken requirements found." success message), and its original justification is stale: PyQt5 5.15.11 ships cp38-abi3 wheels that install fine on Python 3.13, and pyqt5 is pinned in common.txt (it installs successfully in CI today).

Changes

  • Bump pip-tools 7.5.3 → 7.6.1 (supersedes chore(deps-dev): bump pip-tools from 7.5.3 to 7.6.1 #9788; 7.5.x cannot run under pip ≥ 26, which removed pip._internal.utils.compat.stdlib_pkgs)
  • Recompile requirements/dev.txt with --allow-unsafe so pip and setuptools stay pinned. The flag is recorded in the file header, so dependabot (which re-runs the header command) keeps it on future bumps.
  • Add --allow-unsafe to the requirements/common.txt header as well (header-only change — its dependency graph contains no unsafe packages today, but future recompiles stay consistent).
  • Replace the grep-filtered bash construct in tox.ini with a plain, exit-code-driven pip check — no suppression regexps left to go stale.

Verification

Reproduced the CI linters flow in a python:3.13 linux/amd64 container: fresh venv, pip install --no-deps of both compiled requirements files, then pip checkNo broken requirements found. (exit 0).

- Bump pip-tools 7.5.3 -> 7.6.1 (7.5.x cannot run under pip >= 26, which
  removed pip._internal.utils.compat.stdlib_pkgs)
- Recompile requirements/dev.txt with --allow-unsafe so pip and setuptools
  stay pinned. Without the flag, pip-compile silently drops the setuptools
  pin; since CI installs with --no-deps into a Python 3.13 venv (where
  virtualenv no longer seeds setuptools), zope-* and pip-tools then fail
  pip check with 'requires setuptools, which is not installed'. The flag is
  recorded in the file header, so dependabot keeps it on future bumps.
- Replace the grep-filtered pip check in tox.ini with a plain pip check.
  The mozdetect/pyqt5 suppression is stale: PyQt5 5.15.11 ships cp38-abi3
  wheels that install fine on Python 3.13, and pyqt5 is pinned in
  common.txt. With no suppression needed, the exit-code-driven pip check
  replaces the fragile output-emptiness bash construct (which had already
  needed one fix to ignore the 'No broken requirements found.' message).
@netlify

netlify Bot commented Aug 23, 2026

Copy link
Copy Markdown

Deploy Preview for treeherder ready!

Name Link
🔨 Latest commit b6e8381
🔍 Latest deploy log https://app.netlify.com/projects/treeherder/deploys/6a8b2d4dbdbf1a000813fbf1
😎 Deploy Preview https://deploy-preview-9803--treeherder.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@camd

camd commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

A note on --allow-unsafe, since the name looks alarming: it has nothing to do with security. It's a historical pip-tools label for the packaging toolchain itself (pip, setuptools, wheel, distribute). Pinning those in a requirements file was once considered risky because pip install -r could upgrade/downgrade the very tools performing the install, mid-run, and old pip/setuptools handled that badly.

That danger is essentially gone — modern pip handles self-upgrades fine, the pip-tools docs recommend the flag, and the maintainers have discussed making it the default. Meanwhile the default (omit those pins from the output) is actively harmful for our setup:

  • CI installs with pip install --no-deps, so the compiled files must be the complete dependency closure — nothing arrives transitively.
  • Packages in our graph (zope-*, pip-tools) genuinely require setuptools at runtime, and Python 3.12+ venvs no longer seed setuptools. So an omitted pin doesn't mean "the environment provides it"; it means it's absent, and pip check rightly fails — which is exactly the failure dependabot's recompile triggered in chore(deps-dev): bump pip-tools from 7.5.3 to 7.6.1 #9788.
  • Pinning pip/setuptools like everything else (hashed, exact version) is strictly more reproducible: without the flag, whether setuptools exists at all depends on which Python/virtualenv created the venv.

So the flag really means "let the lockfile pin the packaging toolchain too," which for a hash-pinned --no-deps workflow is what we want. It's recorded in the file header commands, so dependabot's future recompiles keep it automatically.

@camd camd self-assigned this Aug 23, 2026
@camd
camd requested a review from Archaeopteryx August 23, 2026 17:33
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.67%. Comparing base (dc3080d) to head (b6e8381).

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #9803    +/-   ##
========================================
  Coverage   83.67%   83.67%            
========================================
  Files         640      640            
  Lines       38864    38864            
  Branches     3488     3488            
========================================
  Hits        32518    32518            
+ Misses       6199     5956   -243     
- Partials      147      390   +243     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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