Local lint fixes - missing steps, pin to bash (#56752)

Summary:
Fixes #56738

* `setup_lint` now installs mypy / shellcheck
* the shell used to execute commands is pinned to `bash` (on Ubuntu the default is `dash`, which was causing the false positives in #56738)
* the emoji check marks don't always work, so use more basic ones instead
* adds `Run autogen` step for mypy (for the `lint` step only since it's pretty slow)
](https://our.intern.facebook.com/intern/diff/27972006/)
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56752

Pulled By: driazati

Reviewed By: samestep

Differential Revision: D27972006

fbshipit-source-id: 624e6c1af2d4f7c8623f420516744922b6b829a5

.github/workflows/lint.yml

@@ -38,8 +38,11 @@ jobs:
           pip install ruamel.yaml==0.17.4
           .github/scripts/lint_native_functions.py
       - name: Extract scripts from GitHub Actions workflows
-        run: tools/extract_scripts.py --out=.extracted_scripts
+        run: |
-      - name: ShellCheck
+          # For local lints, remove the .extracted_scripts folder if it was already there
+          rm -rf .extracted_scripts
+          tools/extract_scripts.py --out=.extracted_scripts
+      - name: Install ShellCheck
         # https://github.com/koalaman/shellcheck/tree/v0.7.2#installing-a-pre-compiled-binary
         run: |
           set -x
@@ -48,6 +51,8 @@ jobs:
           sudo cp "shellcheck-${scversion}/shellcheck" /usr/bin/
           rm -r "shellcheck-${scversion}"
           shellcheck --version
+      - name: Run ShellCheck
+        run: |
           tools/run_shellcheck.sh .jenkins/pytorch .extracted_scripts
       - name: Ensure correct trailing newlines
         run: |
@@ -204,8 +209,10 @@ jobs:
           path: flake8-output/
       - name: Fail if there were any warnings
         run: |
-          cat flake8-output.txt
+          set -eux
-          [ ! -s flake8-output.txt ]
+          # Re-output flake8 status so GitHub logs show it on the step that actually failed
+          cat "${GITHUB_WORKSPACE}"/flake8-output.txt
+          [ ! -s "${GITHUB_WORKSPACE}"/flake8-output.txt ]
 
   clang-tidy:
     if: github.event_name == 'pull_request'

Makefile

@@ -33,13 +33,25 @@ generate-gha-workflows:
 
 setup_lint:
 	python tools/actions_local_runner.py --file .github/workflows/lint.yml \
-	 	--job 'flake8-py3' --step 'Install dependencies'
+	 	--job 'flake8-py3' --step 'Install dependencies' --no-quiet
 	python tools/actions_local_runner.py --file .github/workflows/lint.yml \
-	 	--job 'cmakelint' --step 'Install dependencies'
+	 	--job 'cmakelint' --step 'Install dependencies' --no-quiet
+	python tools/actions_local_runner.py --file .github/workflows/lint.yml \
+	 	--job 'mypy' --step 'Install dependencies' --no-quiet
+
+# TODO: This is broken on MacOS (it downloads a Linux binary)
+	python tools/actions_local_runner.py --file .github/workflows/lint.yml \
+	 	--job 'quick-checks' --step 'Install ShellCheck' --no-quiet
 	pip install jinja2
 
 quick_checks:
-# TODO: This is broken when 'git config submodule.recurse' is 'true'
+	@python tools/actions_local_runner.py \
+		--file .github/workflows/lint.yml \
+		--job 'quick-checks' \
+		--step 'Extract scripts from GitHub Actions workflows'
+
+# TODO: This is broken when 'git config submodule.recurse' is 'true' since the
+# lints will descend into third_party submodules
 	@python tools/actions_local_runner.py \
 		--file .github/workflows/lint.yml \
 		--job 'quick-checks' \
@@ -50,6 +62,7 @@ quick_checks:
 		--step 'Ensure no unqualified noqa' \
 		--step 'Ensure no unqualified type ignore' \
 		--step 'Ensure no direct cub include' \
+		--step 'Run ShellCheck' \
 		--step 'Ensure correct trailing newlines'
 
 flake8:
@@ -59,8 +72,19 @@ flake8:
 		$(CHANGED_ONLY) \
 		--job 'flake8-py3' \
 		--step 'Run flake8'
+	@python tools/actions_local_runner.py \
+		--file .github/workflows/lint.yml \
+		--file-filter '.py' \
+		$(CHANGED_ONLY) \
+		--job 'flake8-py3' \
+		--step 'Fail if there were any warnings'
 
 mypy:
+	@if [ -z "$(CHANGED_ONLY)" ]; then \
+		python tools/actions_local_runner.py --file .github/workflows/lint.yml --job 'mypy' --step 'Run autogen'; \
+    else \
+        echo "mypy: Skipping typestub generation"; \
+    fi
 	@python tools/actions_local_runner.py \
 		--file .github/workflows/lint.yml \
 		--file-filter '.py' \

tools/actions_local_runner.py

@@ -6,6 +6,8 @@ import os
 import argparse
 import yaml
 import asyncio
+import shutil
+import re
 from typing import List, Dict, Any, Optional
 
 
@@ -68,7 +70,7 @@ def find_changed_files() -> List[str]:
     return [x.strip() for x in all_files if x.strip() != ""]
 
 
-async def run_step(step: Dict[str, Any], job_name: str, files: Optional[List[str]]) -> bool:
+async def run_step(step: Dict[str, Any], job_name: str, files: Optional[List[str]], quiet: bool) -> bool:
     env = os.environ.copy()
     env["GITHUB_WORKSPACE"] = "/tmp"
     if files is None:
@@ -77,17 +79,19 @@ async def run_step(step: Dict[str, Any], job_name: str, files: Optional[List[str
         env["LOCAL_FILES"] = " ".join(files)
     script = step["run"]
 
-    PASS = "\U00002705"
+    PASS = color(col.GREEN, '\N{check mark}')
-    FAIL = "\U0000274C"
+    FAIL = color(col.RED, 'x')
-    # We don't need to print the commands for local running
+
-    # TODO: Either lint that GHA scripts only use 'set -eux' or make this more
+    if quiet:
-    # resilient
+        # TODO: Either lint that GHA scripts only use 'set -eux' or make this more
-    script = script.replace("set -eux", "set -eu")
+        # resilient
+        script = script.replace("set -eux", "set -eu")
+        script = re.sub(r"^time ", "", script, flags=re.MULTILINE)
     name = f'{job_name}: {step["name"]}'
 
     def header(passed: bool) -> None:
         icon = PASS if passed else FAIL
-        cprint(col.BLUE, f"{icon} {name}")
+        print(f"{icon} {color(col.BLUE, name)}")
 
     try:
         proc = await asyncio.create_subprocess_shell(
@@ -97,6 +101,7 @@ async def run_step(step: Dict[str, Any], job_name: str, files: Optional[List[str
             env=env,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
+            executable=shutil.which("bash"),
         )
 
         stdout_bytes, stderr_bytes = await proc.communicate()
@@ -118,8 +123,8 @@ async def run_step(step: Dict[str, Any], job_name: str, files: Optional[List[str
     return proc.returncode == 0
 
 
-async def run_steps(steps: List[Dict[str, Any]], job_name: str, files: Optional[List[str]]) -> None:
+async def run_steps(steps: List[Dict[str, Any]], job_name: str, files: Optional[List[str]], quiet: bool) -> None:
-    coros = [run_step(step, job_name, files) for step in steps]
+    coros = [run_step(step, job_name, files, quiet) for step in steps]
     await asyncio.gather(*coros)
 
 
@@ -158,6 +163,7 @@ def main() -> None:
     parser.add_argument("--file-filter", help="only pass through files with this extension", default='')
     parser.add_argument("--changed-only", help="only run on changed files", action='store_true', default=False)
     parser.add_argument("--job", help="job name", required=True)
+    parser.add_argument("--no-quiet", help="output commands", action='store_true', default=False)
     parser.add_argument("--step", action="append", help="steps to run (in order)")
     parser.add_argument(
         "--all-steps-after", help="include every step after this one (non inclusive)"
@@ -165,6 +171,7 @@ def main() -> None:
     args = parser.parse_args()
 
     relevant_files = None
+    quiet = not args.no_quiet
 
     if args.changed_only:
         changed_files: Optional[List[str]] = None
@@ -206,7 +213,8 @@ def main() -> None:
         relevant_steps = grab_all_steps_after(args.all_steps_after, job)
 
     if sys.version_info > (3, 7):
-        asyncio.run(run_steps(relevant_steps, args.job, relevant_files))
+        loop = asyncio.get_event_loop()
+        loop.run_until_complete(run_steps(relevant_steps, args.job, relevant_files, quiet))
     else:
         raise RuntimeError("Only Python >3.7 is supported")