Fix lint-salt/lint-tests pre-commit hooks on non-Linux platforms - #69933
Open
BrianHa94 wants to merge 6 commits into
Open
Fix lint-salt/lint-tests pre-commit hooks on non-Linux platforms#69933BrianHa94 wants to merge 6 commits into
BrianHa94 wants to merge 6 commits into
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey. There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
_lint() hardcoded requirements/static/ci/<pydir>/linux.lock regardless of host OS, breaking the lint-salt and lint-tests pre-commit hooks on macOS (pip tries to build pyinotify, which is Linux-only). Use the existing _get_pip_requirements_file() helper instead, matching every other OS-aware nox session in this file.
The Darwin CI Py3.13 ZeroMQ Requirements hook was missing its own `- id: pip-compile` list marker, so YAML merged it into the preceding Py3.14 hook's mapping instead of creating a new list item. Duplicate keys (alias, name, files, args) meant the Py3.13 block silently overwrote the Py3.14 one, leaving no hook that regenerates requirements/static/ci/py3.14/darwin.lock. Also fixes a leftover py3.14 reference in the Py3.13 hook's files regex from the same copy-paste.
Found by testing a new key-duplicates check: the same missing `- id: pip-compile` merge bug that broke the Darwin Py3.14 zmq requirements hook also silently destroyed the Linux, FreeBSD, Windows, and Docs Py3.14 pip-compile hooks, each clobbered by the following Py3.13 block. Also fixes two FreeBSD Crypto hooks with a leftover duplicate `files:` key, and merges two duplicate `exclude:` keys on the enforce-tornado-imports hook that were silently dropping the salt/ext/.* exclusion. Adds a yamllint hook scoped to .pre-commit-config.yaml with only key-duplicates enabled, so this class of silent data loss can't recur unnoticed.
The .pre-commit-config.yaml hook-ordering and duplicate-key bugs fixed in prior commits left these lock files stale relative to what pip-compile actually produces (missing constraint references and outdated pins), which is what broke the Lint CI Py3.14 Requirements check. Regenerated via pre-commit run --all-files pip-compile.
_lint()/_lint_pre_commit() always installs a platform's own CI requirements lock alongside the single, Linux-pinned lint.lock. Since that lock re-declared the full common.txt/linux.txt/pkg/linux.txt dependency graph (ansible, boto3, kubernetes, moto, mercurial, etc.) as direct requirements, it could drift out of sync with what a Darwin/FreeBSD/Windows base lock resolves for the same packages, breaking lint-salt/lint-tests on those platforms with hard version conflicts (e.g. boto3, importlib-metadata, more-itertools). Add darwin-lint.lock/freebsd-lint.lock/windows-lint.lock alongside the renamed linux-lint.lock (all Python versions), each compiled with its own platform's pip-compile hook and matching noxfile.py's _get_lint_requirements_file() platform selection. Since .pylintrc disables import-error and no-member, pylint's findings are provably unaffected by whether these third-party integration libraries are installed at all (verified by running pylint over the full salt/, tools/, and tests/ trees with and without them). base_requirements_file already provides everything lint.lock was redeclaring, so trim every lint lock down to compile from just lint.txt, constrained against that platform's own CI+pkg lock. This also removes the entire class of cross-file version drift, since there is no longer any overlapping package declaration to drift. Add a check-lint-locks pre-commit hook (tools/precommit/lintlocks.py) that resolves each platform's CI lock together with its lint lock via uv, catching future conflicts without needing a real macOS/FreeBSD/ Windows runner.
check-lint-locks tries every platform combination regardless of which host it runs on, but resolving the linux/freebsd targets requires uv to build pyinotify's legacy sdist locally to read its metadata, and that build hard-checks the real host platform and aborts outside Linux. This made the hook fail with 12 false-positive "conflicts" on any macOS/Windows machine touching lock files, even when every actual version conflict was already resolved. Skip those combinations when not running on Linux, with an explicit warning so the gap in local coverage stays visible instead of silent. Also fixes a doc reference to the old single lint.lock path, which no longer exists now that it's split per platform.
BrianHa94
force-pushed
the
fix-69932-lint-pre-commit-cross-platform-support
branch
from
August 3, 2026 20:35
74a91f9 to
cd41e0f
Compare
twangboy
approved these changes
Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the
lint-salt/lint-testspre-commit hooks, which failed outrighton macOS (and would have hit the same issues on Windows/FreeBSD):
_lint()innoxfile.pyhardcodedrequirements/static/ci/<pydir>/linux.lockregardless of host OS instead of using the existing
_get_pip_requirements_file()helper, causing pip to attempt building
pyinotify(a Linux-only package) onother platforms.
pip-compilehooks in.pre-commit-config.yamlwere missing their own
- id: pip-compilelist marker, causing YAML tosilently merge them into the preceding hook's mapping instead of creating a
separate list item. This clobbered the Darwin/Linux/FreeBSD/Windows Py3.14
ZeroMQ requirements hooks and the Docs CI Py3.14 hook, meaning those lock
files could never actually be regenerated via
pre-commit run <alias>. Arelated duplicate-key instance had also silently dropped the
salt/ext/.*exclusion from the
enforce-tornado-importshook.lint.lockwas a single, Linux-pinned file compiled from the fullbase.txt/common.txt/linux.txt/pkg/linux.txtdependency graph ratherthan just the lint tooling itself, so it could pin different versions of
shared packages (observed with
boto3,aiohttp) than a platform's own CIlock, causing hard pip resolution conflicts on any platform other than
Linux.
check-lint-locksconsistency check (added as part of this fix)tried every platform combination regardless of host, but two of those
combinations require building
pyinotify's legacy sdist locally, whichonly succeeds on a real Linux host — causing false-positive failures on
macOS/Windows.
Adds a
yamllintpre-commit hook (key-duplicatesrule) scoped to.pre-commit-config.yamland atools/precommit/lintlocks.pycheck thatverifies every platform's base lock and lint lock resolve together, so these
classes of bugs can't recur silently.
What issues does this PR fix or reference?
Fixes #69932
Previous Behavior
Running
lint-salt/lint-testspre-commit hooks (ornox -e lint-salt/lint-tests) on macOS failed immediately trying to buildpyinotify. Workingaround that surfaced a second failure: a hard version conflict between the
platform's CI lock and the shared
lint.lock(e.g.boto3,aiohttp).Several lock-file regeneration hooks for Py3.14 were also silently
non-functional due to a YAML authoring bug, with no error indicating they
didn't exist.
New Behavior
lint-salt/lint-testspre-commit hooks run successfully on macOS (verifiedmanually) and Linux.
lint.lockis now split into per-platform variantscontaining only the lint tooling's own dependencies, eliminating the
cross-platform version drift entirely. All previously-clobbered pip-compile
hooks now exist and were used to regenerate their lock files. A new
yamllintcheck and a newcheck-lint-lockspre-commit check guard againstregressions in each of these areas going forward.
Merge requirements satisfied?
doc/topics/development/conventions/style.rst's stalelint.lockpath reference to reflect the new per-platform lock files.yamllintkey-duplicatespre-commithook to catch the duplicate-key YAML bug class, and a new
tools/precommit/lintlocks.pycheck (check-lint-locks) that verifiesevery platform's CI lock resolves against its lint lock, since CI's
Lint job only runs on Linux and wouldn't otherwise catch this.
Commits signed with GPG?
No