From 6d66907a307ae0de7545a02039e0a2a6bd5b55d8 Mon Sep 17 00:00:00 2001 From: driazati <9407960+driazati@users.noreply.github.com> Date: Thu, 11 Aug 2022 12:54:48 -0700 Subject: [PATCH 1/2] [skip ci][ci] Fix Jenkinsfile (#12387) This got out of date after merging #12178 Co-authored-by: driazati --- Jenkinsfile | 23 +++++- ci/jenkins/Prepare.groovy.j2 | 21 ++++- ci/scripts/git_skip_ci.py | 2 +- tests/scripts/check_pr.py | 151 +++++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 tests/scripts/check_pr.py diff --git a/Jenkinsfile b/Jenkinsfile index 50eee01fa974..364c3b99b985 100755 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -45,7 +45,7 @@ // 'python3 jenkins/generate.py' // Note: This timestamp is here to ensure that updates to the Jenkinsfile are // always rebased on main before merging: -// Generated at 2022-08-30T11:58:06.036509 +// Generated at 2022-08-30T15:23:03.836398 import org.jenkinsci.plugins.pipeline.modeldefinition.Utils // NOTE: these lines are scanned by docker/dev_common.sh. Please update the regex as needed. --> @@ -288,7 +288,7 @@ def should_skip_ci(pr_number) { } withCredentials([string( credentialsId: 'tvm-bot-jenkins-reader', - variable: 'TOKEN', + variable: 'GITHUB_TOKEN', )]) { // Exit code of 1 means run full CI (or the script had an error, so run // full CI just in case). Exit code of 0 means skip CI. @@ -301,12 +301,31 @@ def should_skip_ci(pr_number) { return git_skip_ci_code == 0 } +def check_pr(pr_number) { + if (env.BRANCH_NAME == null || !env.BRANCH_NAME.startsWith('PR-')) { + // never skip CI on build sourced from a branch + return false + } + withCredentials([string( + credentialsId: 'tvm-bot-jenkins-reader', + variable: 'GITHUB_TOKEN', + )]) { + sh ( + script: "python3 tests/scripts/check_pr.py --pr ${pr_number}", + label: 'Check PR title and body', + ) + } + +} + def prepare() { stage('Prepare') { node('CPU-SMALL') { ws("workspace/exec_${env.EXECUTOR_NUMBER}/tvm/prepare") { init_git() + check_pr(env.CHANGE_ID) + if (env.DETERMINE_DOCKER_IMAGES == 'yes') { sh( script: "./ci/scripts/determine_docker_images.py ci_arm=${ci_arm} ci_cortexm=${ci_cortexm} ci_cpu=${ci_cpu} ci_gpu=${ci_gpu} ci_hexagon=${ci_hexagon} ci_i386=${ci_i386} ci_lint=${ci_lint} ci_minimal=${ci_minimal} ci_riscv=${ci_riscv} ci_wasm=${ci_wasm} ", diff --git a/ci/jenkins/Prepare.groovy.j2 b/ci/jenkins/Prepare.groovy.j2 index 94575a7b4b64..7dbbb291fdf4 100644 --- a/ci/jenkins/Prepare.groovy.j2 +++ b/ci/jenkins/Prepare.groovy.j2 @@ -138,7 +138,7 @@ def should_skip_ci(pr_number) { } withCredentials([string( credentialsId: 'tvm-bot-jenkins-reader', - variable: 'TOKEN', + variable: 'GITHUB_TOKEN', )]) { // Exit code of 1 means run full CI (or the script had an error, so run // full CI just in case). Exit code of 0 means skip CI. @@ -151,12 +151,31 @@ def should_skip_ci(pr_number) { return git_skip_ci_code == 0 } +def check_pr(pr_number) { + if (env.BRANCH_NAME == null || !env.BRANCH_NAME.startsWith('PR-')) { + // never skip CI on build sourced from a branch + return false + } + withCredentials([string( + credentialsId: 'tvm-bot-jenkins-reader', + variable: 'GITHUB_TOKEN', + )]) { + sh ( + script: "python3 tests/scripts/check_pr.py --pr ${pr_number}", + label: 'Check PR title and body', + ) + } + +} + def prepare() { stage('Prepare') { node('CPU-SMALL') { ws("workspace/exec_${env.EXECUTOR_NUMBER}/tvm/prepare") { init_git() + check_pr(env.CHANGE_ID) + if (env.DETERMINE_DOCKER_IMAGES == 'yes') { sh( script: "./ci/scripts/determine_docker_images.py {% for image in images %}{{ image.name }}={% raw %}${{% endraw %}{{ image.name }}{% raw %}}{% endraw %} {% endfor %}", diff --git a/ci/scripts/git_skip_ci.py b/ci/scripts/git_skip_ci.py index 1e02fcb964fc..162e513275c4 100755 --- a/ci/scripts/git_skip_ci.py +++ b/ci/scripts/git_skip_ci.py @@ -46,7 +46,7 @@ def check_pr_title(): if args.pr_title: title = args.pr_title else: - github = GitHubRepo(token=os.environ["TOKEN"], user=user, repo=repo) + github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) pr = github.get(f"pulls/{args.pr}") title = pr["title"] logging.info(f"pr title: {title}") diff --git a/tests/scripts/check_pr.py b/tests/scripts/check_pr.py new file mode 100644 index 000000000000..94a9180c83b4 --- /dev/null +++ b/tests/scripts/check_pr.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import argparse +import re +import os +import textwrap +from dataclasses import dataclass +from typing import Any, List, Callable + + +from git_utils import GitHubRepo, parse_remote, git +from cmd_utils import init_log, tags_from_title + + +GITHUB_USERNAME_REGEX = re.compile(r"(@[a-zA-Z0-9-]+)", flags=re.MULTILINE) +OK = object() + + +@dataclass +class Check: + # check to run, returning OK means it passed, anything else means it failed + check: Callable[[str], Any] + + # function to call to generate the error message + error_fn: Callable[[Any], str] + + +def non_empty(s: str): + if len(s) == 0: + return False + return OK + + +def usernames(s: str): + m = GITHUB_USERNAME_REGEX.findall(s) + if m and len(m) > 0: + return m + return OK + + +def tags(s: str): + items = tags_from_title(s) + if len(items) == 0: + return False + return OK + + +def trailing_period(s: str): + if s.endswith("."): + return False + return OK + + +title_checks = [ + Check(check=non_empty, error_fn=lambda d: "PR must have a title but title was empty"), + Check(check=trailing_period, error_fn=lambda d: "PR must not end in a tailing '.'"), + Check( + check=usernames, + error_fn=lambda d: f"PR title must not tag anyone but found these usernames: {d}", + ), + Check( + check=tags, + error_fn=lambda d: f"PR title must have a topic tag like [the_topic] (e.g. [tir], [relay], etc.) but found none", + ), +] +body_checks = [ + Check(check=non_empty, error_fn=lambda d: "PR must have a body but body was empty"), + Check( + check=usernames, + error_fn=lambda d: f"PR body must not tag anyone but found these usernames: {d}", + ), +] + + +def run_checks(checks: List[Check], s: str, name: str) -> bool: + errors = [(c, c.check(s)) for c in checks] + errors = [item for item in errors if item[1] != OK] + errors = [] + print(f"Running checks for {name}") + print(textwrap.indent(s, prefix=" ")) + passed = True + print(" Checks:") + for i, check in enumerate(checks): + result = check.check(s) + if result == OK: + print(f" [{i+1}] {check.check.__name__}: PASSED") + else: + passed = False + msg = check.error_fn(result) + print(f" [{i+1}] {check.check.__name__}: FAILED: {msg}") + + return passed + + +if __name__ == "__main__": + init_log() + help = "Check a PR's title and body for conformance to guidelines" + parser = argparse.ArgumentParser(description=help) + parser.add_argument("--pr", required=True) + parser.add_argument("--remote", default="origin", help="ssh remote to parse") + parser.add_argument( + "--pr-body", help="(testing) PR body to use instead of fetching from GitHub" + ) + parser.add_argument( + "--pr-title", help="(testing) PR title to use instead of fetching from GitHub" + ) + args = parser.parse_args() + + try: + pr = int(args.pr) + except ValueError: + print(f"PR was not a number: {args.pr}") + exit(0) + + if args.pr_body: + body = args.pr_body + title = args.pr_title + else: + remote = git(["config", "--get", f"remote.{args.remote}.url"]) + user, repo = parse_remote(remote) + + github = GitHubRepo(token=os.environ["GITHUB_TOKEN"], user=user, repo=repo) + pr = github.get(f"pulls/{args.pr}") + body = pr["body"] + title = pr["title"] + + title_passed = run_checks(checks=title_checks, s=title, name="PR title") + print("") + body_passed = run_checks(checks=body_checks, s=body, name="PR body") + + if title_passed and body_passed: + print("All checks passed!") + exit(0) + else: + print("Some checks failed, please review the logs above") + exit(1) From 24b4440d0ac27ca80625d4262a61ff9fd6c8ea8c Mon Sep 17 00:00:00 2001 From: driazati Date: Thu, 11 Aug 2022 13:14:42 -0700 Subject: [PATCH 2/2] Address comments --- Jenkinsfile | 4 +-- ci/jenkins/Prepare.groovy.j2 | 2 +- {tests => ci}/scripts/check_pr.py | 43 +++++++++++++++---------------- 3 files changed, 24 insertions(+), 25 deletions(-) rename {tests => ci}/scripts/check_pr.py (83%) diff --git a/Jenkinsfile b/Jenkinsfile index 364c3b99b985..2b73508da0d3 100755 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -45,7 +45,7 @@ // 'python3 jenkins/generate.py' // Note: This timestamp is here to ensure that updates to the Jenkinsfile are // always rebased on main before merging: -// Generated at 2022-08-30T15:23:03.836398 +// Generated at 2022-08-30T15:26:50.100067 import org.jenkinsci.plugins.pipeline.modeldefinition.Utils // NOTE: these lines are scanned by docker/dev_common.sh. Please update the regex as needed. --> @@ -311,7 +311,7 @@ def check_pr(pr_number) { variable: 'GITHUB_TOKEN', )]) { sh ( - script: "python3 tests/scripts/check_pr.py --pr ${pr_number}", + script: "python3 ci/scripts/check_pr.py --pr ${pr_number}", label: 'Check PR title and body', ) } diff --git a/ci/jenkins/Prepare.groovy.j2 b/ci/jenkins/Prepare.groovy.j2 index 7dbbb291fdf4..6d0c0ec9c4b6 100644 --- a/ci/jenkins/Prepare.groovy.j2 +++ b/ci/jenkins/Prepare.groovy.j2 @@ -161,7 +161,7 @@ def check_pr(pr_number) { variable: 'GITHUB_TOKEN', )]) { sh ( - script: "python3 tests/scripts/check_pr.py --pr ${pr_number}", + script: "python3 ci/scripts/check_pr.py --pr ${pr_number}", label: 'Check PR title and body', ) } diff --git a/tests/scripts/check_pr.py b/ci/scripts/check_pr.py similarity index 83% rename from tests/scripts/check_pr.py rename to ci/scripts/check_pr.py index 94a9180c83b4..45d502c6a72e 100644 --- a/tests/scripts/check_pr.py +++ b/ci/scripts/check_pr.py @@ -29,6 +29,7 @@ GITHUB_USERNAME_REGEX = re.compile(r"(@[a-zA-Z0-9-]+)", flags=re.MULTILINE) OK = object() +FAIL = object() @dataclass @@ -42,55 +43,48 @@ class Check: def non_empty(s: str): if len(s) == 0: - return False + return FAIL return OK def usernames(s: str): m = GITHUB_USERNAME_REGEX.findall(s) - if m and len(m) > 0: - return m - return OK + return m if m else OK def tags(s: str): items = tags_from_title(s) if len(items) == 0: - return False + return FAIL return OK def trailing_period(s: str): if s.endswith("."): - return False + return FAIL return OK title_checks = [ Check(check=non_empty, error_fn=lambda d: "PR must have a title but title was empty"), Check(check=trailing_period, error_fn=lambda d: "PR must not end in a tailing '.'"), - Check( - check=usernames, - error_fn=lambda d: f"PR title must not tag anyone but found these usernames: {d}", - ), - Check( - check=tags, - error_fn=lambda d: f"PR title must have a topic tag like [the_topic] (e.g. [tir], [relay], etc.) but found none", - ), + # TODO(driazati): enable this check once https://github.com/apache/tvm/issues/12637 is done + # Check( + # check=usernames, + # error_fn=lambda d: f"PR title must not tag anyone but found these usernames: {d}", + # ), ] body_checks = [ Check(check=non_empty, error_fn=lambda d: "PR must have a body but body was empty"), - Check( - check=usernames, - error_fn=lambda d: f"PR body must not tag anyone but found these usernames: {d}", - ), + # TODO(driazati): enable this check once https://github.com/apache/tvm/issues/12637 is done + # Check( + # check=usernames, + # error_fn=lambda d: f"PR body must not tag anyone but found these usernames: {d}", + # ), ] def run_checks(checks: List[Check], s: str, name: str) -> bool: - errors = [(c, c.check(s)) for c in checks] - errors = [item for item in errors if item[1] != OK] - errors = [] print(f"Running checks for {name}") print(textwrap.indent(s, prefix=" ")) passed = True @@ -139,6 +133,9 @@ def run_checks(checks: List[Check], s: str, name: str) -> bool: body = pr["body"] title = pr["title"] + body = body.strip() + title = title.strip() + title_passed = run_checks(checks=title_checks, s=title, name="PR title") print("") body_passed = run_checks(checks=body_checks, s=body, name="PR body") @@ -147,5 +144,7 @@ def run_checks(checks: List[Check], s: str, name: str) -> bool: print("All checks passed!") exit(0) else: - print("Some checks failed, please review the logs above") + print( + "Some checks failed, please review the logs above and edit your PR on GitHub accordingly" + ) exit(1)