Skip to content

Commit 5e4dfd0

Browse files
Add quicklint make target (pytorch#56559)
Summary: This queries the local git repo for changed files (any changed files, not just committed ones) and sends them to mypy/flake8 instead of the default (which is the whole repo, defined the .flake8 and mypy.ini files). This brings a good speedup (from 15 seconds with no cache to < 1 second from my local testing on this PR). ```bash make quicklint -j 6 ``` It should be noted that the results of this aren’t exactly what’s in the CI, since mypy and flake8 ignore the `include` and `exclude` parts of their config when an explicit list of files is passed in. ](https://our.intern.facebook.com/intern/diff/27901577/) Pull Request resolved: pytorch#56559 Pulled By: driazati Reviewed By: malfet Differential Revision: D27901577 fbshipit-source-id: 99f351cdfe5aba007948aea2b8a78f683c5d8583
1 parent 12b2bc9 commit 5e4dfd0

File tree

6 files changed

+179
-22
lines changed

6 files changed

+179
-22
lines changed

.github/workflows/lint.yml

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ on:
66
- master
77
pull_request:
88

9+
# note [local lint]
10+
# LOCAL_FILES in the jobs below is intended to be filled by local (i.e. on a
11+
# developer's machine) job in tools/actions_local_runner.py to pass only the
12+
# changed files to whatever tool is consuming the files. This helps speed up
13+
# local lints while keeping the linting code unified between local / CI.
14+
915
jobs:
1016
quick-checks:
1117
runs-on: ubuntu-18.04
@@ -128,15 +134,15 @@ jobs:
128134
uses: actions/checkout@v2
129135
- name: Install markdown-toc
130136
run: npm install -g markdown-toc
131-
- name: Regenerate ToCs
137+
- name: Regenerate ToCs and check that they didn't change
132138
run: |
133139
set -eux
134140
export PATH=~/.npm-global/bin:"$PATH"
135141
for FILE in $(git grep -Il '<!-- toc -->' -- '**.md'); do
136142
markdown-toc --bullets='-' -i "$FILE"
137143
done
138-
- name: Assert that regenerating the ToCs didn't change them
139-
run: .github/scripts/report_git_status.sh
144+
145+
.github/scripts/report_git_status.sh
140146
141147
flake8-py3:
142148
runs-on: ubuntu-18.04
@@ -164,9 +170,13 @@ jobs:
164170
pip install -r requirements-flake8.txt
165171
flake8 --version
166172
- name: Run flake8
173+
env:
174+
# See note [local lint]
175+
LOCAL_FILES: ""
167176
run: |
168177
set -eux
169-
flake8 | tee "${GITHUB_WORKSPACE}"/flake8-output.txt
178+
# shellcheck disable=SC2086
179+
flake8 $LOCAL_FILES | tee "${GITHUB_WORKSPACE}"/flake8-output.txt
170180
- name: Translate annotations
171181
if: github.event_name == 'pull_request'
172182
env:
@@ -342,6 +352,10 @@ jobs:
342352
time python -mtools.codegen.gen -s aten/src/ATen -d build/aten/src/ATen
343353
time python -mtools.pyi.gen_pyi --native-functions-path aten/src/ATen/native/native_functions.yaml --deprecated-functions-path "tools/autograd/deprecated.yaml"
344354
- name: Run mypy
355+
env:
356+
# See note [local lint]
357+
LOCAL_FILES: ""
345358
run: |
346359
set -eux
347-
for CONFIG in mypy*.ini; do mypy --config="$CONFIG"; done
360+
# shellcheck disable=SC2086
361+
for CONFIG in mypy*.ini; do mypy --config="$CONFIG" $LOCAL_FILES; done

CONTRIBUTING.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,11 @@ command runs tests such as `TestNN.test_BCELoss` and
364364
You can run the same linting steps that are used in CI locally via `make`:
365365

366366
```bash
367+
# Lint all files
367368
make lint -j 6 # run lint (using 6 parallel jobs)
369+
370+
# Lint only the files you have changed
371+
make quicklint -j 6
368372
```
369373

370374
These jobs may require extra dependencies that aren't dependencies of PyTorch

Makefile

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,16 @@ quick_checks:
5555
flake8:
5656
@python tools/actions_local_runner.py \
5757
--file .github/workflows/lint.yml \
58+
--file-filter '.py' \
59+
$(CHANGED_ONLY) \
5860
--job 'flake8-py3' \
5961
--step 'Run flake8'
6062

6163
mypy:
6264
@python tools/actions_local_runner.py \
6365
--file .github/workflows/lint.yml \
66+
--file-filter '.py' \
67+
$(CHANGED_ONLY) \
6468
--job 'mypy' \
6569
--step 'Run mypy'
6670

@@ -74,4 +78,13 @@ clang_tidy:
7478
echo "clang-tidy local lint is not yet implemented"
7579
exit 1
7680

81+
toc:
82+
@python tools/actions_local_runner.py \
83+
--file .github/workflows/lint.yml \
84+
--job 'toc' \
85+
--step "Regenerate ToCs and check that they didn't change"
86+
7787
lint: flake8 mypy quick_checks cmakelint generate-gha-workflows
88+
89+
quicklint: CHANGED_ONLY=--changed-only
90+
quicklint: mypy flake8 mypy quick_checks cmakelint generate-gha-workflows

mypy-strict.ini

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ files =
5151
tools/test/test_mypy_wrapper.py,
5252
tools/test/test_test_history.py,
5353
tools/test/test_trailing_newlines.py,
54+
tools/test/test_actions_local_runner.py,
5455
tools/test/test_translate_annotations.py,
5556
tools/trailing_newlines.py,
5657
tools/translate_annotations.py,
58+
tools/actions_local_runner.py,
5759
torch/testing/_internal/framework_utils.py,
5860
torch/utils/benchmark/utils/common.py,
5961
torch/utils/benchmark/utils/timer.py,

tools/actions_local_runner.py

Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
#!/bin/python3
1+
#!/usr/bin/env python3
22

33
import subprocess
4+
import sys
45
import os
56
import argparse
67
import yaml
78
import asyncio
9+
from typing import List, Dict, Any, Optional
810

911

1012
REPO_ROOT = os.path.dirname(os.path.dirname(__file__))
@@ -21,23 +23,71 @@ class col:
2123
UNDERLINE = "\033[4m"
2224

2325

24-
def color(the_color, text):
26+
def color(the_color: str, text: str) -> str:
2527
return col.BOLD + the_color + str(text) + col.RESET
2628

2729

28-
def cprint(the_color, text):
29-
print(color(the_color, text))
30+
def cprint(the_color: str, text: str) -> None:
31+
if hasattr(sys.stdout, "isatty") and sys.stdout.isatty():
32+
print(color(the_color, text))
33+
else:
34+
print(text)
35+
36+
37+
def git(args: List[str]) -> List[str]:
38+
p = subprocess.run(
39+
["git"] + args,
40+
cwd=REPO_ROOT,
41+
stdout=subprocess.PIPE,
42+
stderr=subprocess.PIPE,
43+
check=True,
44+
)
45+
lines = p.stdout.decode().strip().split("\n")
46+
return [line.strip() for line in lines]
47+
48+
49+
def find_changed_files(merge_base: str = "origin/master") -> List[str]:
50+
untracked = []
51+
52+
for line in git(["status", "--porcelain"]):
53+
# Untracked files start with ??, so grab all of those
54+
if line.startswith("?? "):
55+
untracked.append(line.replace("?? ", ""))
56+
57+
# Modified, unstaged
58+
modified = git(["diff", "--name-only"])
3059

60+
# Modified, staged
61+
cached = git(["diff", "--cached", "--name-only"])
3162

32-
async def run_step(step, job_name):
63+
# Committed
64+
diff_with_origin = git(["diff", "--name-only", "--merge-base", merge_base, "HEAD"])
65+
66+
# De-duplicate
67+
all_files = set(untracked + cached + modified + diff_with_origin)
68+
return [x.strip() for x in all_files if x.strip() != ""]
69+
70+
71+
async def run_step(step: Dict[str, Any], job_name: str, files: Optional[List[str]]) -> bool:
3372
env = os.environ.copy()
3473
env["GITHUB_WORKSPACE"] = "/tmp"
74+
if files is None:
75+
env["LOCAL_FILES"] = ""
76+
else:
77+
env["LOCAL_FILES"] = " ".join(files)
3578
script = step["run"]
3679

80+
PASS = "\U00002705"
81+
FAIL = "\U0000274C"
3782
# We don't need to print the commands for local running
3883
# TODO: Either lint that GHA scripts only use 'set -eux' or make this more
3984
# resilient
4085
script = script.replace("set -eux", "set -eu")
86+
name = f'{job_name}: {step["name"]}'
87+
88+
def header(passed: bool) -> None:
89+
icon = PASS if passed else FAIL
90+
cprint(col.BLUE, f"{icon} {name}")
4191

4292
try:
4393
proc = await asyncio.create_subprocess_shell(
@@ -49,27 +99,31 @@ async def run_step(step, job_name):
4999
stderr=subprocess.PIPE,
50100
)
51101

52-
stdout, stderr = await proc.communicate()
53-
cprint(col.BLUE, f'{job_name}: {step["name"]}')
102+
stdout_bytes, stderr_bytes = await proc.communicate()
103+
104+
header(passed=proc.returncode == 0)
54105
except Exception as e:
55-
cprint(col.BLUE, f'{job_name}: {step["name"]}')
106+
header(passed=False)
56107
print(e)
108+
return False
57109

58-
stdout = stdout.decode().strip()
59-
stderr = stderr.decode().strip()
110+
stdout = stdout_bytes.decode().strip()
111+
stderr = stderr_bytes.decode().strip()
60112

61113
if stderr != "":
62114
print(stderr)
63115
if stdout != "":
64116
print(stdout)
65117

118+
return proc.returncode == 0
66119

67-
async def run_steps(steps, job_name):
68-
coros = [run_step(step, job_name) for step in steps]
120+
121+
async def run_steps(steps: List[Dict[str, Any]], job_name: str, files: Optional[List[str]]) -> None:
122+
coros = [run_step(step, job_name, files) for step in steps]
69123
await asyncio.gather(*coros)
70124

71125

72-
def grab_specific_steps(steps_to_grab, job):
126+
def grab_specific_steps(steps_to_grab: List[str], job: Dict[str, Any]) -> List[Dict[str, Any]]:
73127
relevant_steps = []
74128
for step in steps_to_grab:
75129
for actual_step in job["steps"]:
@@ -83,7 +137,7 @@ def grab_specific_steps(steps_to_grab, job):
83137
return relevant_steps
84138

85139

86-
def grab_all_steps_after(last_step, job):
140+
def grab_all_steps_after(last_step: str, job: Dict[str, Any]) -> List[Dict[str, Any]]:
87141
relevant_steps = []
88142

89143
found = False
@@ -96,18 +150,29 @@ def grab_all_steps_after(last_step, job):
96150
return relevant_steps
97151

98152

99-
def main():
153+
def main() -> None:
100154
parser = argparse.ArgumentParser(
101155
description="Pull shell scripts out of GitHub actions and run them"
102156
)
103157
parser.add_argument("--file", help="YAML file with actions", required=True)
158+
parser.add_argument("--file-filter", help="only pass through files with this extension", default='')
159+
parser.add_argument("--changed-only", help="only run on changed files", action='store_true', default=False)
104160
parser.add_argument("--job", help="job name", required=True)
105161
parser.add_argument("--step", action="append", help="steps to run (in order)")
106162
parser.add_argument(
107163
"--all-steps-after", help="include every step after this one (non inclusive)"
108164
)
109165
args = parser.parse_args()
110166

167+
changed_files = None
168+
if args.changed_only:
169+
changed_files = []
170+
for f in find_changed_files():
171+
for file_filter in args.file_filter:
172+
if f.endswith(file_filter):
173+
changed_files.append(f)
174+
break
175+
111176
if args.step is None and args.all_steps_after is None:
112177
raise RuntimeError("1+ --steps or --all-steps-after must be provided")
113178

@@ -129,8 +194,7 @@ def main():
129194
else:
130195
relevant_steps = grab_all_steps_after(args.all_steps_after, job)
131196

132-
# pprint.pprint(relevant_steps)
133-
asyncio.run(run_steps(relevant_steps, args.job))
197+
asyncio.run(run_steps(relevant_steps, args.job, changed_files)) # type: ignore[attr-defined]
134198

135199

136200
if __name__ == "__main__":
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import unittest
2+
import sys
3+
import contextlib
4+
import io
5+
from typing import List, Dict, Any
6+
7+
from tools import actions_local_runner
8+
9+
10+
if __name__ == '__main__':
11+
if sys.version_info >= (3, 8):
12+
# actions_local_runner uses asyncio features not available in 3.6, and
13+
# IsolatedAsyncioTestCase was added in 3.8, so skip testing on
14+
# unsupported systems
15+
class TestRunner(unittest.IsolatedAsyncioTestCase):
16+
def run(self, *args: List[Any], **kwargs: List[Dict[str, Any]]) -> Any:
17+
return super().run(*args, **kwargs)
18+
19+
def test_step_extraction(self) -> None:
20+
fake_job = {
21+
"steps": [
22+
{
23+
"name": "test1",
24+
"run": "echo hi"
25+
},
26+
{
27+
"name": "test2",
28+
"run": "echo hi"
29+
},
30+
{
31+
"name": "test3",
32+
"run": "echo hi"
33+
},
34+
]
35+
}
36+
37+
actual = actions_local_runner.grab_specific_steps(["test2"], fake_job)
38+
expected = [
39+
{
40+
"name": "test2",
41+
"run": "echo hi"
42+
},
43+
]
44+
self.assertEqual(actual, expected)
45+
46+
async def test_runner(self) -> None:
47+
fake_step = {
48+
"name": "say hello",
49+
"run": "echo hi"
50+
}
51+
f = io.StringIO()
52+
with contextlib.redirect_stdout(f):
53+
await actions_local_runner.run_steps([fake_step], "test", None)
54+
55+
result = f.getvalue()
56+
self.assertIn("say hello", result)
57+
self.assertIn("hi", result)
58+
59+
60+
unittest.main()

0 commit comments

Comments
 (0)