Organize scripts

Move all .py that are supposed to be run as __main__ under ./scripts
so there is no need to manipulate sys.path to import modules.

Runby cleanup

.gitignore

@@ -6,4 +6,4 @@ __pycache__/
 containers/workspace
 **/.DS_Store
 **/.ipynb_checkpoints
-scripts/buildkite/cache.json
+scripts/metrics/cache.json

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.
 

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:

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)
+

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
 

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
 
 

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):

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}))

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/**/*'],

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:

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')

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')