From 22ce8e96326f97de1963b79949a98da95ade78c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=BChnel?= Date: Tue, 18 Feb 2020 09:55:24 +0100 Subject: [PATCH 1/7] handling broken XML files --- scripts/phabtalk/phabtalk.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/phabtalk/phabtalk.py b/scripts/phabtalk/phabtalk.py index 1ad1bd5..5c587e4 100755 --- a/scripts/phabtalk/phabtalk.py +++ b/scripts/phabtalk/phabtalk.py @@ -233,7 +233,16 @@ class BuildReport: else: self.success = False - self.add_test_results() + try: + self.add_test_results() + except etree.XMLSyntaxError: + # Sometimes we get an incomplete XML file. + # In this case: + # - fail the build (the safe thing to do) + # - continue so the user gets some feedback. + print('Error parsing {}. Invalid XML syntax!'.format(self.test_result_file)) + self.success = False + self.add_clang_tidy() self.add_clang_format() self.api.update_build_status(self.diff_id, self.ph_id, self.working, self.success, self.lint, self.unit) From b17bd9343804b90daaa48ec199b2ca7b9f5ee7be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=BChnel?= Date: Tue, 18 Feb 2020 13:37:13 +0100 Subject: [PATCH 2/7] fixed division by zero problem --- scripts/metrics/repo.py | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/scripts/metrics/repo.py b/scripts/metrics/repo.py index 8868520..81a8f2b 100755 --- a/scripts/metrics/repo.py +++ b/scripts/metrics/repo.py @@ -16,7 +16,7 @@ import datetime from time import timezone import git -from typing import Dict +from typing import Dict, Optional from google.cloud import monitoring_v3 import re @@ -32,21 +32,35 @@ class RepoStats: self.reviewed = 0 # type: int @property - def percent_reverted(self) -> float: - return 100.0 * self.reverts / self.commits + def percent_reverted(self) -> Optional[float]: + try: + return 100.0 * self.reverts / self.commits + except ZeroDivisionError: + return None @property - def percent_reviewed(self) -> float: - return 100.0 * self.reviewed / (self.commits - self.reverts) + def percent_reviewed(self) -> Optional[float]: + try: + return 100.0 * self.reviewed / (self.commits - self.reverts) + except ZeroDivisionError: + return None def __str__(self): - return "\n".join([ + results = [ "commits: {}".format(self.commits), "reverts: {}".format(self.reverts), "reviewed: {}".format(self.reviewed), - "percent reverted: {:0.1f}".format(self.percent_reverted), - "percent reviewed: {:0.1f}".format(self.percent_reviewed), - ]) + ] + try: + results.append("percent reverted: {:0.1f}".format(self.percent_reverted)) + except TypeError: + pass + try: + results.append("percent reverted: {:0.1f}".format(self.percent_reverted)) + except TypeError: + pass + + return "\n".join(results) def get_reverts_per_day(repo_path: str, max_age: datetime.datetime) -> RepoStats: stats = RepoStats() @@ -78,6 +92,9 @@ def gcp_write_data(project_id: str, stats: RepoStats, now:datetime.datetime): ["reviewed", stats.reviewed], ["percent_reviewed", stats.percent_reviewed], ]: + if value is None: + continue + series = monitoring_v3.types.TimeSeries() series.metric.type = 'custom.googleapis.com/repository_{}'.format(desc_type) series.resource.type = 'global' From 4a19cec6e9383718e7eb27968f4b149eca3be530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=BChnel?= Date: Tue, 18 Feb 2020 13:41:29 +0100 Subject: [PATCH 3/7] fixed result url --- Jenkins/Phabricator-windows-pipeline/Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkins/Phabricator-windows-pipeline/Jenkinsfile b/Jenkins/Phabricator-windows-pipeline/Jenkinsfile index c44745f..99c70fe 100644 --- a/Jenkins/Phabricator-windows-pipeline/Jenkinsfile +++ b/Jenkins/Phabricator-windows-pipeline/Jenkinsfile @@ -25,7 +25,7 @@ pipeline { PHABRICATOR_HOST = 'https://reviews.llvm.org' PHAB_LOG = "${WORKSPACE}/build/.phabricator-comment" MY_BUILD_ID = "${JOB_BASE_NAME}-${BUILD_NUMBER}" - RESULT_URL = "https://pantheon.corp.google.com/storage/browser/llvm-premerge-checks/results/${MY_BUILD_ID}" + RESULT_URL = "https://storage.cloud.google.com/llvm-premerge-checks/results/${MY_BUILD_ID}" SCRIPT_DIR = "${WORKSPACE}/llvm-premerge-checks/scripts" // store all build results here, will be uploaded to GCS later RESULT_DIR = "${WORKSPACE}\\results" From a2aa62870615b71652014070745d1c016b19a944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=BChnel?= Date: Wed, 19 Feb 2020 09:21:21 +0100 Subject: [PATCH 4/7] added timeoue --- Jenkins/Phabricator-windows-pipeline/Jenkinsfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Jenkins/Phabricator-windows-pipeline/Jenkinsfile b/Jenkins/Phabricator-windows-pipeline/Jenkinsfile index 99c70fe..0d79d67 100644 --- a/Jenkins/Phabricator-windows-pipeline/Jenkinsfile +++ b/Jenkins/Phabricator-windows-pipeline/Jenkinsfile @@ -31,6 +31,9 @@ pipeline { RESULT_DIR = "${WORKSPACE}\\results" LLVM_DIR = "${WORKSPACE}\\llvm-project" } + options { + timeout(time:2, unit:'HOURS') + } stages { stage("build info"){ steps { From 63ad2d1be640d258a2545cbdc0d99813043a26fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=BChnel?= Date: Wed, 19 Feb 2020 12:08:18 +0100 Subject: [PATCH 5/7] retrying on phab.update_interfaces() --- scripts/phabtalk/phabtalk.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/phabtalk/phabtalk.py b/scripts/phabtalk/phabtalk.py index 5c587e4..27799df 100755 --- a/scripts/phabtalk/phabtalk.py +++ b/scripts/phabtalk/phabtalk.py @@ -40,7 +40,7 @@ class PhabTalk: self._phab = None # type: Optional[Phabricator] if not dryrun: self._phab = Phabricator(token=token, host=host) - self._phab.update_interfaces() + _try_call(self._phab.update_interfaces) @property def dryrun(self): From c1396546e5b7fff18d3dec2a489381ab30271486 Mon Sep 17 00:00:00 2001 From: Mikhail Goncharov Date: Wed, 19 Feb 2020 16:16:42 +0100 Subject: [PATCH 6/7] Remove empty lines at the end of clang-tidy report --- scripts/lint.sh | 2 +- scripts/phabtalk/phabtalk.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/lint.sh b/scripts/lint.sh index e54f1d8..f585e83 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -32,6 +32,6 @@ set -e git checkout -- . # clang-tidy -git diff -U0 HEAD | "${SCRIPT_DIR}/ignore_diff.py" "${SCRIPT_DIR}/clang-tidy.ignore" | clang-tidy-diff -p1 -quiet > "${TARGET_DIR}"/clang-tidy.txt +git diff -U0 HEAD | "${SCRIPT_DIR}/ignore_diff.py" "${SCRIPT_DIR}/clang-tidy.ignore" | clang-tidy-diff -p1 -quiet | sed '${/^[[:space:]]*$/d;}' > "${TARGET_DIR}"/clang-tidy.txt echo "linters completed ======================================" diff --git a/scripts/phabtalk/phabtalk.py b/scripts/phabtalk/phabtalk.py index 27799df..08b8910 100755 --- a/scripts/phabtalk/phabtalk.py +++ b/scripts/phabtalk/phabtalk.py @@ -323,7 +323,7 @@ class BuildReport: self.comments.append(section_title('clang-tidy', False, False)) return p = os.path.join(self.results_dir, self.clang_tidy_result) - if os.stat(p).st_size > 4: + if os.stat(p).st_size > 0: self.api.add_artifact(self.ph_id, self.clang_tidy_result, "clang-tidy", self.results_url) ignore = pathspec.PathSpec.from_lines(pathspec.patterns.GitWildMatchPattern, open(self.clang_tidy_ignore, 'r').readlines()) From 70df407e0e879af4f7bd67f4629c9ea0b7313bf4 Mon Sep 17 00:00:00 2001 From: Mikhail Goncharov Date: Wed, 19 Feb 2020 16:49:39 +0100 Subject: [PATCH 7/7] Add name of build agent to summary --- Jenkins/Phabricator-pipeline/Jenkinsfile | 3 ++- .../Phabricator-windows-pipeline/Jenkinsfile | 3 ++- scripts/phabtalk/phabtalk.py | 17 +++++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Jenkins/Phabricator-pipeline/Jenkinsfile b/Jenkins/Phabricator-pipeline/Jenkinsfile index efb4694..42c3a14 100644 --- a/Jenkins/Phabricator-pipeline/Jenkinsfile +++ b/Jenkins/Phabricator-pipeline/Jenkinsfile @@ -174,7 +174,8 @@ pipeline { --clang-tidy-ignore "${SCRIPT_DIR}/clang-tidy-comments.ignore" \ --results-dir "${TARGET_DIR}" \ --results-url "${RESULT_URL}" \ - --failures "${failure_message}" + --failures "${failure_message}" \ + --name "linux" """ } } diff --git a/Jenkins/Phabricator-windows-pipeline/Jenkinsfile b/Jenkins/Phabricator-windows-pipeline/Jenkinsfile index 0d79d67..89437df 100644 --- a/Jenkins/Phabricator-windows-pipeline/Jenkinsfile +++ b/Jenkins/Phabricator-windows-pipeline/Jenkinsfile @@ -162,7 +162,8 @@ pipeline { --results-dir "${RESULT_DIR}" ^ --results-url "${RESULT_URL}" ^ --failures "${failure_message}" ^ - --buildresult ${currentBuild.result} + --buildresult ${currentBuild.result} ^ + --name "windows" """ dir("${RESULT_DIR}") { // upload results to diff --git a/scripts/phabtalk/phabtalk.py b/scripts/phabtalk/phabtalk.py index 08b8910..80cbb6a 100755 --- a/scripts/phabtalk/phabtalk.py +++ b/scripts/phabtalk/phabtalk.py @@ -124,9 +124,9 @@ class PhabTalk: lint=lint_messages)) 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), + artifactKey = str(uuid.uuid4()) + artifactType = 'uri' + artifactData = {'uri': '{}/{}'.format(results_url, file), 'ui.external': True, 'name': name} if self.dryrun: @@ -203,9 +203,11 @@ class BuildReport: self.results_url = args.results_url # type: str self.workspace = args.workspace # type: str self.failure_messages = args.failures # type: str + self.name = args.name # type: str self.api = PhabTalk(args.conduit_token, args.host, args.dryrun) + self.revision_id = self.api.get_revision_id(self.diff_id) self.comments = [] self.success = True self.working = False @@ -263,12 +265,14 @@ class BuildReport: '.failure {color:red;}\n' '.success {color:green;}\n' '') + f.write('

Build result for diff {0} {1} at {2}

'.format( + self.revision_id, self.diff_id, self.name)) if self.failure_messages and len(self.failure_messages) > 0: for s in self.failure_messages.split('\n'): f.write('

{}

'.format(s)) f.write('

' + '

'.join(self.comments) + '

') f.write('') - self.api.add_artifact(self.ph_id, 'summary.html', 'summary', self.results_url) + self.api.add_artifact(self.ph_id, 'summary.html', 'summary ' + self.name, self.results_url) def add_clang_format(self): """Populates results from diff produced by clang format.""" @@ -282,7 +286,7 @@ class BuildReport: return p = os.path.join(self.results_dir, self.clang_format_patch) if os.stat(p).st_size != 0: - self.api.add_artifact(self.ph_id, self.clang_format_patch, "clang-format", self.results_url) + self.api.add_artifact(self.ph_id, self.clang_format_patch, 'clang-format ' + self.name, self.results_url) diffs = _parse_patch(open(p, 'r')) success = len(diffs) == 0 for d in diffs: @@ -324,7 +328,7 @@ class BuildReport: return p = os.path.join(self.results_dir, self.clang_tidy_result) if os.stat(p).st_size > 0: - self.api.add_artifact(self.ph_id, self.clang_tidy_result, "clang-tidy", self.results_url) + self.api.add_artifact(self.ph_id, self.clang_tidy_result, 'clang-tidy ' + self.name, self.results_url) ignore = pathspec.PathSpec.from_lines(pathspec.patterns.GitWildMatchPattern, open(self.clang_tidy_ignore, 'r').readlines()) for line in open(p, 'r'): @@ -491,6 +495,7 @@ def main(): help="public URL to access results directory") parser.add_argument('--workspace', type=str, required=True, help="path to workspace") parser.add_argument('--failures', type=str, default=None, help="optional failure messages separated by newline") + parser.add_argument('--name', type=str, default='', help="optional name of the build bot") args = parser.parse_args() reporter = BuildReport(args)