[skip ci] Add simple local actions runner by driazati · Pull Request #56439 · pytorch/pytorch

driazati · 2021-04-20T01:40:57Z

This pulls out shell scripts from an action and runs them locally as a first pass at #55847. A helper script extracts specific steps in some order and runs them:

$ time -p make lint -j 5  # run lint with 5 CPUs
python scripts/actions_local_runner.py \
        --file .github/workflows/lint.yml \
        --job 'flake8-py3' \
        --step 'Run flake8'
python scripts/actions_local_runner.py \
        --file .github/workflows/lint.yml \
        --job 'mypy' \
        --step 'Run mypy'
python scripts/actions_local_runner.py \
        --file .github/workflows/lint.yml \
        --job 'quick-checks' \
        --step 'Ensure no trailing spaces' \
        --step 'Ensure no tabs' \
        --step 'Ensure no non-breaking spaces' \
        --step 'Ensure canonical include' \
        --step 'Ensure no unqualified noqa' \
        --step 'Ensure no direct cub include' \
        --step 'Ensure correct trailing newlines'
python scripts/actions_local_runner.py \
        --file .github/workflows/lint.yml \
        --job 'cmakelint' \
        --step 'Run cmakelint'
quick-checks: Ensure no direct cub include
quick-checks: Ensure canonical include
quick-checks: Ensure no unqualified noqa
quick-checks: Ensure no non-breaking spaces
quick-checks: Ensure no tabs
quick-checks: Ensure correct trailing newlines
cmakelint: Run cmakelint
quick-checks: Ensure no trailing spaces
mypy: Run mypy
Success: no issues found in 1316 source files
Success: no issues found in 56 source files
flake8-py3: Run flake8
./test.py:1:1: F401 'torch' imported but unused
real 13.89
user 199.63
sys 6.08

Mypy/flake8 are by far the slowest, but that's mostly just because they're wasting a bunch of work linting the entire repo.

In followup, we could/should:

Improve ergonomics (i.e. no output unless there are errors)
Speed up lint by only linting files changes between origin and HEAD
Add clang-tidy

Differential Revision: D27888027

facebook-github-bot · 2021-04-20T01:41:03Z

💊 CI failures summary and remediations

As of commit ff2d71c (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

.github/workflows/lint.yml

samestep · 2021-04-20T17:52:10Z

Could you also modify CONTRIBUTING.md to describe how to use this?

.github/workflows/lint.yml

Makefile

scripts/actions_local_runner.py

samestep

Is there a reason you put actions_local_runner.py in scripts instead of tools? (See #52688.) It might be better to put it in tools instead, since then it'd be easier to write unit tests for it.

.github/workflows/lint.yml

walterddr · 2021-04-20T18:05:27Z

scripts/actions_local_runner.py

i dont know if there's a rule on what gets put into .github/scripts and what in scripts/. if we dont have one we should

Well according to scripts/README.md:

This directory contains the useful tools. ...

but then in the tools/README.md:

This folder contains a number of scripts which are used as ...

so it looks like it's made to confuse

lol yep, as noted in #52688

This pulls out shell scripts from an action and runs them locally

janeyx99

🥳

janeyx99 · 2021-04-20T18:43:56Z

Makefile

+	echo "clang-tidy local lint is not yet implemented"
+	exit 1
+
+lint: flake8 mypy quick_checks cmakelint shellcheck-gha generate-gha-workflows


Should this also include setup_lint?

I think since it's mostly going to be regular PyTorch devs using this it doesn't need to do everything (i.e spend the time running those steps). pip also still spits out a bunch of output even when everything is already installed so it doesn't look great

We could have another wrapper script that checks which things are installed and only runs the actual pip install if it needs to, but I'll leave that for later

Yeah I think leaving that to be a followup step is the best approach here

… manually

facebook-github-bot · 2021-04-20T19:02:18Z

@driazati has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-04-20T19:19:13Z

@driazati merged this pull request in 43eb21b.

seemethere · 2021-04-20T19:17:22Z

Makefile

+
+quick_checks:
+# TODO: This is broken when 'git config submodule.recurse' is 'true'
+	@python tools/actions_local_runner.py \


I don't think it's actually necessary to hide the command being executed here, if anything I think it's more valuable to the user to know exactly what command is being executed in the makefile

Any extra output makes it harder to parse for errors, especially since all these tools have disparate ways of reporting their results, here I was thinking that for a successful run there should be no output at all, whereas outputting all the makefile commands and the scripts creates a lot of spew that most users probably don't care about (assuming the tool doesn't break):

./.github/scripts/generate_linux_ci_workflows.py quick-checks: Ensure no unqualified noqa quick-checks: Ensure no direct cub include quick-checks: Ensure canonical include quick-checks: Ensure no tabs quick-checks: Ensure no non-breaking spaces quick-checks: Ensure correct trailing newlines cmakelint: Run cmakelint quick-checks: Ensure no trailing spaces mypy: Run mypy Success: no issues found in 1316 source files Success: no issues found in 56 source files flake8-py3: Run flake8

vs

python tools/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'flake8-py3' \ --step 'Run flake8' python tools/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'mypy' \ --step 'Run mypy' python tools/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'quick-checks' \ --step 'Ensure no trailing spaces' \ --step 'Ensure no tabs' \ --step 'Ensure no non-breaking spaces' \ --step 'Ensure canonical include' \ --step 'Ensure no unqualified noqa' \ --step 'Ensure no direct cub include' \ --step 'Ensure correct trailing newlines' python tools/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'cmakelint' \ --step 'Run cmakelint' ./.github/scripts/generate_linux_ci_workflows.py quick-checks: Ensure no unqualified noqa quick-checks: Ensure no direct cub include quick-checks: Ensure canonical include quick-checks: Ensure no non-breaking spaces quick-checks: Ensure no tabs quick-checks: Ensure correct trailing newlines cmakelint: Run cmakelint + git ls-files -z -- bootstrap '*.cmake' '*.cmake.in' '*CMakeLists.txt' + grep -E -z -v '^(cmake/Modules/|cmake/Modules_CUDA_fix/|cmake/Caffe2Config.cmake.in|aten/src/ATen/ATenConfig.cmake.in|cmake/Caffe2ConfigVersion.cmake.in|cmake/TorchConfig.cmake.in|cmake/TorchConfigVersion.cmake.in|cmake/cmake_uninstall.cmake.in)' + xargs -0 cmakelint --config=.cmakelintrc --spaces=2 --quiet quick-checks: Ensure no trailing spaces mypy: Run mypy + for CONFIG in mypy*.ini + mypy --config=mypy.ini + for CONFIG in mypy*.ini + mypy --config=mypy-strict.ini Success: no issues found in 1316 source files Success: no issues found in 56 source files flake8-py3: Run flake8 + flake8 + tee /tmp/flake8-output.txt

It's also possible to force the commands to be shown (at least on the Makefile), though the ergonomics aren't great

make lint SHELL='sh -x'

seemethere · 2021-04-20T19:17:31Z

Makefile

+
+quick_checks:
+# TODO: This is broken when 'git config submodule.recurse' is 'true'
+	@python tools/actions_local_runner.py \


Suggested change

@python tools/actions_local_runner.py \

python tools/actions_local_runner.py \

seemethere · 2021-04-20T19:17:40Z

Makefile

+		--step 'Ensure correct trailing newlines'
+
+flake8:
+	@python tools/actions_local_runner.py \


Suggested change

@python tools/actions_local_runner.py \

python tools/actions_local_runner.py \

seemethere · 2021-04-20T19:17:49Z

Makefile

+		--step 'Run flake8'
+
+mypy:
+	@python tools/actions_local_runner.py \


Suggested change

@python tools/actions_local_runner.py \

python tools/actions_local_runner.py \

seemethere · 2021-04-20T19:18:02Z

Makefile

+		--step 'Run mypy'
+
+cmakelint:
+	@python tools/actions_local_runner.py \


Suggested change

@python tools/actions_local_runner.py \

python tools/actions_local_runner.py \

seemethere · 2021-04-20T19:19:18Z

tools/actions_local_runner.py

+
+async def run_step(step, job_name):
+    env = os.environ.copy()
+    env["GITHUB_WORKSPACE"] = "/tmp"


Should we just go ahead and create a tempdir here using TemporaryDirectory?

I'll roll this into one of the follow ups

seemethere · 2021-04-20T19:19:51Z

tools/actions_local_runner.py

+    # 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
+    # resilient
+    script = script.replace("set -eux", "set -eu")


I don't actually think it's a bad idea to output what commands you're running since it should give you a better idea how to debug if anything does go wrong

Summary: Followup to #56290 which adds the new lint to the local runner from #56439. Pull Request resolved: #56587 Test Plan: Same as #56439. Reviewed By: walterddr Differential Revision: D27909889 Pulled By: samestep fbshipit-source-id: 8b67f3bc36c9b5567fe5a9e49904f2cf23a9f135

Summary: This pulls out shell scripts from an action and runs them locally as a first pass at pytorch#55847. A helper script extracts specific steps in some order and runs them: ```bash $ time -p make lint -j 5 # run lint with 5 CPUs python scripts/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'flake8-py3' \ --step 'Run flake8' python scripts/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'mypy' \ --step 'Run mypy' python scripts/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'quick-checks' \ --step 'Ensure no trailing spaces' \ --step 'Ensure no tabs' \ --step 'Ensure no non-breaking spaces' \ --step 'Ensure canonical include' \ --step 'Ensure no unqualified noqa' \ --step 'Ensure no direct cub include' \ --step 'Ensure correct trailing newlines' python scripts/actions_local_runner.py \ --file .github/workflows/lint.yml \ --job 'cmakelint' \ --step 'Run cmakelint' quick-checks: Ensure no direct cub include quick-checks: Ensure canonical include quick-checks: Ensure no unqualified noqa quick-checks: Ensure no non-breaking spaces quick-checks: Ensure no tabs quick-checks: Ensure correct trailing newlines cmakelint: Run cmakelint quick-checks: Ensure no trailing spaces mypy: Run mypy Success: no issues found in 1316 source files Success: no issues found in 56 source files flake8-py3: Run flake8 ./test.py:1:1: F401 'torch' imported but unused real 13.89 user 199.63 sys 6.08 ``` Mypy/flake8 are by far the slowest, but that's mostly just because they're wasting a bunch of work linting the entire repo. In followup, we could/should: * Improve ergonomics (i.e. no output unless there are errors) * Speed up lint by only linting files changes between origin and HEAD * Add clang-tidy Pull Request resolved: pytorch#56439 Reviewed By: samestep Differential Revision: D27888027 Pulled By: driazati fbshipit-source-id: d6f2a59a45e9d725566688bdac8e909210175996

Summary: Followup to pytorch#56290 which adds the new lint to the local runner from pytorch#56439. Pull Request resolved: pytorch#56587 Test Plan: Same as pytorch#56439. Reviewed By: walterddr Differential Revision: D27909889 Pulled By: samestep fbshipit-source-id: 8b67f3bc36c9b5567fe5a9e49904f2cf23a9f135

facebook-github-bot added the cla signed label Apr 20, 2021

driazati changed the title ~~[skip ci][wip] Add simple local actions runner~~ [skip ci] Add simple local actions runner Apr 20, 2021

driazati requested review from a team, janeyx99, samestep and walterddr and removed request for janeyx99, samestep and walterddr April 20, 2021 17:46