diff --git a/.gitignore b/.gitignore index 3049273..07ba580 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,4 @@ __pycache__/ containers/workspace **/.DS_Store **/.ipynb_checkpoints -scripts/buildkite/cache.json +scripts/metrics/cache.json diff --git a/docs/development.md b/docs/development.md index 56a47d2..54dfdf7 100644 --- a/docs/development.md +++ b/docs/development.md @@ -78,7 +78,7 @@ shadowing any Buildkite environment variable). "ph_scripts_refspec" parameter defines refspec of llvm-premerge-checks to use ("master" by default). **diff-checks** pipeline -([create_branch_pipeline.py](../scripts/buildkite/create_branch_pipeline.py)) +([create_branch_pipeline.py](../scripts/create_branch_pipeline.py)) downloads a patch (or series of patches) and applies it to a fork of the llvm-project repository. Then it pushes a new state as a new branch (e.g. "phab-diff-288211") and triggers "premerge-checks" on it (all "ph_" env @@ -87,7 +87,7 @@ build or by another tooling. Periodical **cleanup-branches** pipeline deletes branches older than 30 days. **premerge-checks** pipeline -([build_branch_pipeline.py](../scripts/buildkite/build_branch_pipeline.py)) +([build_branch_pipeline.py](../scripts/build_branch_pipeline.py)) builds and tests changes on Linux and Windows agents. Then it uploads a combined result to Phabricator. diff --git a/docs/playbooks.md b/docs/playbooks.md index 3b59577..85bfecf 100644 --- a/docs/playbooks.md +++ b/docs/playbooks.md @@ -159,7 +159,7 @@ schtasks.exe /create /tn "Start Buildkite agent" /ru SYSTEM /SC ONSTART /DELAY 0 ## Custom environment variables Buildkite pipelines have a number of custom environment variables one can set to change their behavior. That is useful to debug issues -or test changes. They are mostly used by pipleine generators, e.g. [build_master_pipeline](../scripts/buildkite/build_master_pipeline.py), +or test changes. They are mostly used by pipleine generators, e.g. [build_master_pipeline](../scripts/build_master_pipeline.py), please refer to the source code for the details. These variables have `ph_` prefix and can be set with URL parameters in Harbormaster build. Most commonly used are: diff --git a/scripts/phabtalk/add_url_artifact.py b/scripts/add_phabricator_artifact.py similarity index 65% rename from scripts/phabtalk/add_url_artifact.py rename to scripts/add_phabricator_artifact.py index 32da386..9e4c5d6 100755 --- a/scripts/phabtalk/add_url_artifact.py +++ b/scripts/add_phabricator_artifact.py @@ -16,21 +16,9 @@ import argparse import logging import os -import sys -import uuid -if __name__ == '__main__': - sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from phabtalk.phabtalk import PhabTalk - -def maybe_add_url_artifact(phab: PhabTalk, phid: str, url: str, name: str): - if phid is None: - logging.warning('PHID is not provided, cannot create URL artifact') - return - phab.create_artifact(phid, str(uuid.uuid4()), 'uri', {'uri': url, 'ui.external': True, 'name': name}) - - if __name__ == '__main__': parser = argparse.ArgumentParser(description='Runs premerge checks8') parser.add_argument('--url', type=str) @@ -40,5 +28,5 @@ if __name__ == '__main__': args = parser.parse_args() logging.basicConfig(level=args.log_level, format='%(levelname)-7s %(message)s') - phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'), 'https://reviews.llvm.org/api/', False) - maybe_add_url_artifact(phabtalk, args.phid, args.url, args.name) + PhabTalk(os.getenv('CONDUIT_TOKEN')).maybe_add_url_artifact(args.phid, args.url, args.name) + diff --git a/scripts/buildkite/apply_patch.sh b/scripts/apply_patch.sh similarity index 87% rename from scripts/buildkite/apply_patch.sh rename to scripts/apply_patch.sh index d07f597..2d845f5 100755 --- a/scripts/buildkite/apply_patch.sh +++ b/scripts/apply_patch.sh @@ -30,8 +30,8 @@ scripts/phabtalk/apply_patch2.py $ph_buildable_diff \ EXIT_STATUS=$? if [ $EXIT_STATUS -ne 0 ]; then - scripts/phabtalk/add_url_artifact.py --phid="$ph_target_phid" --url="$BUILDKITE_BUILD_URL" --name="Buildkite apply patch" - scripts/buildkite/set_build_status.py + scripts/add_phabricator_artifact.py --phid="$ph_target_phid" --url="$BUILDKITE_BUILD_URL" --name="Buildkite apply patch" + scripts/set_build_status.py echo failed fi diff --git a/scripts/buildkite/utils.py b/scripts/buildkite_utils.py similarity index 100% rename from scripts/buildkite/utils.py rename to scripts/buildkite_utils.py diff --git a/scripts/clang_tidy_report.py b/scripts/clang_tidy_report.py index abdb1e4..5dcd465 100755 --- a/scripts/clang_tidy_report.py +++ b/scripts/clang_tidy_report.py @@ -22,7 +22,7 @@ from typing import Optional import pathspec import ignore_diff -from buildkite.utils import format_url +from buildkite_utils import format_url from phabtalk.phabtalk import Report, Step diff --git a/scripts/buildkite/analyze_jobs.ipynb b/scripts/metrics/analyze_jobs.ipynb similarity index 100% rename from scripts/buildkite/analyze_jobs.ipynb rename to scripts/metrics/analyze_jobs.ipynb diff --git a/scripts/phabtalk/phabtalk.py b/scripts/phabtalk/phabtalk.py index 1908fc7..b9b1d73 100755 --- a/scripts/phabtalk/phabtalk.py +++ b/scripts/phabtalk/phabtalk.py @@ -21,6 +21,7 @@ import time import uuid from typing import Optional, List, Dict from phabricator import Phabricator +import logging class PhabTalk: @@ -28,7 +29,7 @@ class PhabTalk: See https://secure.phabricator.com/conduit/method/harbormaster.sendmessage/ """ - def __init__(self, token: Optional[str], host: Optional[str], dryrun: bool): + def __init__(self, token: Optional[str], host: Optional[str] = 'https://reviews.llvm.org/api/', dryrun: bool = False): self._phab = None # type: Optional[Phabricator] if not dryrun: self._phab = Phabricator(token=token, host=host) @@ -118,16 +119,6 @@ class PhabTalk: print('Uploaded build status {}, {} test results and {} lint results'.format( result_type, len(unit), len(lint_messages))) - # TODO: deprecate - def add_artifact(self, phid: str, file: str, name: str, results_url: str): - artifactKey = str(uuid.uuid4()) - artifactType = 'uri' - artifactData = {'uri': '{}/{}'.format(results_url, file), - 'ui.external': True, - 'name': name} - self.create_artifact(phid, artifactKey, artifactType, artifactData) - print('Created artifact "{}"'.format(name)) - def create_artifact(self, phid, artifact_key, artifact_type, artifact_data): if self.dryrun: print('harbormaster.createartifact =================') @@ -141,6 +132,12 @@ class PhabTalk: artifactType=artifact_type, artifactData=artifact_data)) + def maybe_add_url_artifact(self, phid: str, url: str, name: str): + if phid is None: + logging.warning('PHID is not provided, cannot create URL artifact') + return + self.create_artifact(phid, str(uuid.uuid4()), 'uri', {'uri': url, 'ui.external': True, 'name': name}) + class Step: def __init__(self): diff --git a/scripts/buildkite/create_branch_pipeline.py b/scripts/pipeline_create_branch.py similarity index 59% rename from scripts/buildkite/create_branch_pipeline.py rename to scripts/pipeline_create_branch.py index 2ed9e77..5ffc295 100755 --- a/scripts/buildkite/create_branch_pipeline.py +++ b/scripts/pipeline_create_branch.py @@ -22,11 +22,15 @@ if __name__ == '__main__': log_level = os.getenv('ph_log_level', 'INFO') base_commit = os.getenv('ph_base_commit', 'auto') run_build = os.getenv('ph_skip_build') is None + trigger = os.getenv('ph_trigger_pipeline') + if trigger is None: + trigger = 'premerge-checks' + steps = [] create_branch_step = { 'label': 'create branch', 'key': 'create-branch', - 'commands': ['scripts/buildkite/apply_patch.sh'], + 'commands': ['scripts/apply_patch.sh'], 'agents': {'queue': f'{queue_prefix}linux'}, 'timeout_in_minutes': 20, 'env': { @@ -34,23 +38,23 @@ if __name__ == '__main__': 'BASE_COMMIT': base_commit, } } - build_linux_step = { - 'trigger': 'premerge-checks', - 'label': ':rocket: build and test', - 'async': False, - 'depends_on': 'create-branch', - 'build': { - 'branch': f'phab-diff-{diff_id}', - 'env': {}, - }, - } - for e in os.environ: - if e.startswith('ph_'): - build_linux_step['build']['env'][e] = os.getenv(e) - # Set scripts source from the current build if it's not yet defined. - if 'ph_scripts_refspec' not in build_linux_step['build']['env']: - build_linux_step['build']['env']['ph_scripts_refspec'] = '${BUILDKITE_BRANCH}' - steps.append(create_branch_step) if run_build: - steps.append(build_linux_step) + trigger_build_step = { + 'trigger': trigger, + 'label': ':rocket: build and test', + 'async': False, + 'depends_on': 'create-branch', + 'build': { + 'branch': f'phab-diff-{diff_id}', + 'env': {}, + }, + } + for e in os.environ: + if e.startswith('ph_'): + trigger_build_step['build']['env'][e] = os.getenv(e) + # Set scripts source from the current build if it's not yet defined. + if 'ph_scripts_refspec' not in trigger_build_step['build']['env']: + trigger_build_step['build']['env']['ph_scripts_refspec'] = '${BUILDKITE_BRANCH}' + steps.append(trigger_build_step) + steps.append(create_branch_step) print(yaml.dump({'steps': steps})) diff --git a/scripts/buildkite/build_master_pipeline.py b/scripts/pipeline_master.py similarity index 100% rename from scripts/buildkite/build_master_pipeline.py rename to scripts/pipeline_master.py diff --git a/scripts/buildkite/build_branch_pipeline.py b/scripts/pipeline_premerge.py similarity index 95% rename from scripts/buildkite/build_branch_pipeline.py rename to scripts/pipeline_premerge.py index 86b87b3..937a56b 100755 --- a/scripts/buildkite/build_branch_pipeline.py +++ b/scripts/pipeline_premerge.py @@ -14,16 +14,12 @@ # limitations under the License. import json -import os -import yaml -import git -import sys import logging +import os -if __name__ == '__main__': - sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) - -import choose_projects +from choose_projects import ChooseProjects +import git +import yaml if __name__ == '__main__': scripts_refspec = os.getenv("ph_scripts_refspec", "master") @@ -38,7 +34,7 @@ if __name__ == '__main__': # List all affected projects. repo = git.Repo('.') patch = repo.git.diff("HEAD~1") - cp = choose_projects.ChooseProjects('.') + cp = ChooseProjects('.') affected_projects = cp.choose_projects(patch) print('# all affected projects') for p in affected_projects: @@ -91,7 +87,7 @@ if __name__ == '__main__': 'set +e', # Add link in review to the build. - '${SRC}/scripts/phabtalk/add_url_artifact.py ' + '${SRC}/scripts/add_phabricator_artifact.py ' '--phid="$ph_target_phid" ' '--url="$BUILDKITE_BUILD_URL" ' '--name="Buildkite build"', @@ -180,7 +176,7 @@ if __name__ == '__main__': 'echo "llvm-premerge-checks commit"', 'git rev-parse HEAD', 'cd "$BUILDKITE_BUILD_CHECKOUT_PATH"', - '${SRC}/scripts/buildkite/summary.py', + '${SRC}/scripts/summary.py', ], 'allow_dependency_failure': True, 'artifact_paths': ['artifacts/**/*'], diff --git a/scripts/premerge_checks.py b/scripts/premerge_checks.py index 85b7a5a..5cf3901 100755 --- a/scripts/premerge_checks.py +++ b/scripts/premerge_checks.py @@ -30,9 +30,8 @@ import clang_format_report import clang_tidy_report import run_cmake import test_results_report -from buildkite.utils import upload_file +from buildkite_utils import upload_file from exec_utils import watch_shell, if_not_matches, tee -from phabtalk.add_url_artifact import maybe_add_url_artifact from phabtalk.phabtalk import Report, PhabTalk, Step @@ -173,14 +172,14 @@ if __name__ == '__main__': ph_target_phid = os.getenv('ph_target_phid') ph_buildable_diff = os.getenv('ph_buildable_diff') if ph_target_phid is not None: - phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'), 'https://reviews.llvm.org/api/', False) + phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN')) for u in report.unit: u['engine'] = step_key phabtalk.update_build_status(ph_buildable_diff, ph_target_phid, True, report.success, report.lint, report.unit) for a in report.artifacts: url = upload_file(a['dir'], a['file']) if url is not None: - maybe_add_url_artifact(phabtalk, ph_target_phid, url, f'{a["name"]} ({step_key})') + phabtalk.maybe_add_url_artifact(ph_target_phid, url, f'{a["name"]} ({step_key})') else: logging.warning('No phabricator phid is specified. Will not update the build status in Phabricator') with open(report_path, 'w') as f: diff --git a/scripts/buildkite/set_build_status.py b/scripts/set_build_status.py similarity index 85% rename from scripts/buildkite/set_build_status.py rename to scripts/set_build_status.py index 602183e..717f224 100755 --- a/scripts/buildkite/set_build_status.py +++ b/scripts/set_build_status.py @@ -14,18 +14,11 @@ # limitations under the License. import argparse -import glob -import json import logging import os -import sys -import uuid - -if __name__ == '__main__': - sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from phabtalk.phabtalk import PhabTalk -from buildkite.utils import format_url +from buildkite_utils import format_url if __name__ == '__main__': parser = argparse.ArgumentParser() @@ -33,7 +26,7 @@ if __name__ == '__main__': parser.add_argument('--success', action='store_true') args = parser.parse_args() logging.basicConfig(level=args.log_level, format='%(levelname)-7s %(message)s') - phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'), 'https://reviews.llvm.org/api/', False) + phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN')) build_url = f'https://reviews.llvm.org/harbormaster/build/{os.getenv("ph_build_id")}' print(f'Reporting results to Phabricator build {format_url(build_url)}', flush=True) ph_buildable_diff = os.getenv('ph_buildable_diff') diff --git a/scripts/buildkite/summary.py b/scripts/summary.py similarity index 82% rename from scripts/buildkite/summary.py rename to scripts/summary.py index e003eea..d48b57f 100755 --- a/scripts/buildkite/summary.py +++ b/scripts/summary.py @@ -18,21 +18,9 @@ import glob import json import logging import os -import sys -import uuid - -if __name__ == '__main__': - sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from phabtalk.phabtalk import PhabTalk -from buildkite.utils import format_url - - -def maybe_add_url_artifact(phab: PhabTalk, phid: str, url: str, name: str): - if phid is None: - logging.warning('PHID is not provided, cannot create URL artifact') - return - phab.create_artifact(phid, str(uuid.uuid4()), 'uri', {'uri': url, 'ui.external': True, 'name': name}) +from buildkite_utils import format_url if __name__ == '__main__': @@ -59,7 +47,7 @@ if __name__ == '__main__': report = json.load(f) logging.info(report) success = success and report['success'] - phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'), 'https://reviews.llvm.org/api/', False) + phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN')) build_url = f'https://reviews.llvm.org/harbormaster/build/{os.getenv("ph_build_id")}' print(f'Reporting results to Phabricator build {format_url(build_url)}', flush=True) ph_buildable_diff = os.getenv('ph_buildable_diff')