Skip to content

Commit

Permalink
Refactor tools/actions_local_runner.py to allow custom remote/branch …
Browse files Browse the repository at this point in the history
…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: pytorch#73387
Approved by: malfet
  • Loading branch information
Thiago Crepaldi authored and pytorchmergebot committed Mar 3, 2022
1 parent b955a04 commit cf143bd
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 5 deletions.
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -110,6 +113,7 @@ cmakelint:
clang-tidy:
@$(PYTHON) tools/actions_local_runner.py \
$(CHANGED_ONLY) \
$(REF_BRANCH) \
--job 'clang-tidy'

toc:
Expand Down
13 changes: 8 additions & 5 deletions tools/actions_local_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]):
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down

0 comments on commit cf143bd

Please sign in to comment.