From cf143bdc57826025f27017093e5d2a4d119b80d1 Mon Sep 17 00:00:00 2001 From: Thiago Crepaldi Date: Thu, 3 Mar 2022 22:05:59 +0000 Subject: [PATCH] Refactor tools/actions_local_runner.py to allow custom remote/branch names Currently this script assumes origin/master as the branch for comparison when --changed-only flag is used. Although this is reasonable for PyTorch build infrastructure, external contributors that fork PyTorch may have different remotes/branch names, such as upstream/master Assuming users have a `origin` remote for their PyTorch fork and a `upstream` remote pointing to the original PyTorch repo For external contributions, by running ``` make quick_checks CHANGED_ONLY=--changed-only make flake8 CHANGED_ONLY=--changed-only make mypy CHANGED_ONLY=--changed-only make cmakelint CHANGED_ONLY=--changed-only make shellcheck CHANGED_ONLY=--changed-only make clang-tidy CHANGED_ONLY=--changed-only make quicklint -j ${CPUS} CHANGED_ONLY=--changed-only ``` would result in an incorrect git diff, forcing users to run a full test as opposed to just test changed files This PR introduces a --ref_branch argument that allows users to specify an arbitrary branch to detect changes in their own PyTorch forks which may have a remote for the upstream PyTorch repo. Given the proposed changes, this is how users could compare their changes to an arbitrary remote/branch ``` make quick_checks CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master make flake8 CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master make mypy CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master make cmakelint CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master make shellcheck CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master make clang-tidy CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master make quicklint -j ${CPUS} CHANGED_ONLY=--changed-only REF_BRANCH=--ref_branch=upstream/master ``` This change is BC, as it assigns origin/master as default argument to all existing methods that handle branches Pull Request resolved: https://github.com/pytorch/pytorch/pull/73387 Approved by: malfet --- Makefile | 4 ++++ tools/actions_local_runner.py | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 3d18c2b463819..e74f5715a8200 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,7 @@ shellcheck: --step 'Extract scripts from GitHub Actions workflows' @$(PYTHON) tools/actions_local_runner.py \ $(CHANGED_ONLY) \ + $(REF_BRANCH) \ --job 'shellcheck' setup_lint: @@ -94,11 +95,13 @@ quick_checks: flake8: @$(PYTHON) tools/actions_local_runner.py \ $(CHANGED_ONLY) \ + $(REF_BRANCH) \ --job 'flake8-py3' mypy: @$(PYTHON) tools/actions_local_runner.py \ $(CHANGED_ONLY) \ + $(REF_BRANCH) \ --job 'mypy' cmakelint: @@ -110,6 +113,7 @@ cmakelint: clang-tidy: @$(PYTHON) tools/actions_local_runner.py \ $(CHANGED_ONLY) \ + $(REF_BRANCH) \ --job 'clang-tidy' toc: diff --git a/tools/actions_local_runner.py b/tools/actions_local_runner.py index 0509059341338..1d2b0455a266b 100755 --- a/tools/actions_local_runner.py +++ b/tools/actions_local_runner.py @@ -59,7 +59,7 @@ def git(args: List[str]) -> List[str]: return [line.strip() for line in lines] -def find_changed_files() -> List[str]: +def find_changed_files(ref_branch : str = "origin/master") -> List[str]: untracked = [] for line in git(["status", "--porcelain"]): @@ -74,7 +74,7 @@ def find_changed_files() -> List[str]: cached = git(["diff", "--cached", "--name-only"]) # Committed - merge_base = git(["merge-base", "origin/master", "HEAD"])[0] + merge_base = git(["merge-base", ref_branch, "HEAD"])[0] diff_with_origin = git(["diff", "--name-only", merge_base, "HEAD"]) # De-duplicate @@ -334,10 +334,10 @@ async def full(self) -> CommandResult: return await shell_cmd(script, env=env) -def changed_files() -> Optional[List[str]]: +def changed_files(ref_branch : str = "origin/master") -> Optional[List[str]]: changed_files: Optional[List[str]] = None try: - changed_files = sorted(find_changed_files()) + changed_files = sorted(find_changed_files(ref_branch)) except Exception: # If the git commands failed for some reason, bail out and use the whole list print( @@ -381,6 +381,9 @@ def main() -> None: "--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("--ref_branch", + default="origin/master", + help="remote/branch used during comparison for --changed-only (default=origin/master") args = parser.parse_args() quiet = not args.no_quiet @@ -396,7 +399,7 @@ def main() -> None: files = None if args.changed_only: - files = changed_files() + files = changed_files(args.ref_branch) checks = [ad_hoc_steps[args.job](files, quiet)] else: