Use uv run for lintrunner Python deps #159735
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/159735
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 1 Unrelated FailureAs of commit 8d6f51f with merge base 9a680e1 ( NEW FAILURES - The following jobs have failed:
UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| @@ -1,3 +1,9 @@ | |||
| # /// script | |||
| # dependencies = [ | |||
| # "cmakelint==1.4.1", | |||
There was a problem hiding this comment.
I haven't audited Codex didn't mess up putting in the deps, if we agree the approach is good I will audit these
| @@ -24,7 +31,7 @@ | |||
| from pathlib import Path | |||
| from typing import Any, Callable, NamedTuple, Optional | |||
|
|
|||
| from yaml import load | |||
| import yaml # type: ignore[import-untyped] | |||
There was a problem hiding this comment.
I'll revert these changes if we agree to do it this way
|
@ZainRizvi merged https://github.com/pytorch/pytorch/blob/main/scripts/run_lintrunner.py to do this via github hooks by having lintrunner as a tool rather than changing each one. |
|
@ZainRizvi I read through #158389 and I don't really understand the approach. It seems you are installing the lintrunner deps into the venv that uv allocates for the tool itself. But this is wrong: what if I want to use lintrunner on a different repo? You need a separate venv PER project, not just one attached to lintrunner itself! |
@ezyang that's not what the #158389 did. The pre-commit pip package creates a separate venv to use for the hook for each repo. That's the env the lintrunner deps are being installed into, not into a universal lintrunner environment. Every repo would get it's own venv. (#160048 does that more clearly) That said, the goal of that PR was to setup reliable pre-push hooks, and getting a consistent lintrunner environment was incidental. If this PR works out we can easily update the pre-push hooks script to take advantage of this new setup instead. |
There was a problem hiding this comment.
I really like the auto-updating aspect of this so that there's no dependency on the currently active venv.
It would be even better if the uv also fixes the python version everything gets run using (today CI runs lintrunner using python 3.9, that would be the best one to match)
Some linter failures only manifest in specific python versions
I asked Codex to whip up the proof of concept. The idea is to make use of uv's anonymous venvs instead of forcing everything to be installed in the current venv, which is helpful when you are working on multiple projects which have different pins for various formatting tools (hi autoparallel). If people like the idea I'll clean it up and land it.
Summary
uv runin lintrunner config to invoke Python linters without a priorlintrunner inituvinstalls them on demandlintrunner initis not requireduv runin fallback messages when lintrunner or shellcheck are missingTesting
lintrunner tools/linter/adapters/no_workflows_on_fork.py tools/linter/adapters/workflow_consistency_linter.py(fails: Failed to fetchhttps://pypi.org/simple/ruff/)https://chatgpt.com/codex/tasks/task_e_688f81faeae48323a3aa0c7ac1cd452c