fix project resolutions

and some of the typing issues found by mypy

.gitignore

@@ -7,3 +7,4 @@ containers/workspace
 **/.DS_Store
 **/.ipynb_checkpoints
 scripts/metrics/cache.json
+Pipfile.lock

Pipfile

@@ -0,0 +1,21 @@
+[[source]]
+url = "https://pypi.org/simple"
+verify_ssl = true
+name = "pypi"
+
+[packages]
+backoff = "==1.10.0"
+lxml = "==4.6.3"
+pathspec = "==0.8.1"
+phabricator = "==0.8.1"
+pyaml = "==20.4.0"
+requests = "==2.25.1"
+unidiff = "==0.6.0"
+python-benedict = "==0.23.2"
+GitPython = "==3.1.14"
+types-pyyaml = "*"
+
+[dev-packages]
+
+[requires]
+python_version = "3.9"

scripts/.gitignore

@@ -1 +1 @@
-benchmark/
+benchmark/

scripts/choose_projects.py

@@ -13,10 +13,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Compute the LLVM_ENABLE_PROJECTS for Cmake from diff.
+"""Compute the LLVM_ENABLE_PROJECTS for cmake from diff.
 
-This script will compute which projects are affected by the diff proviaded via STDIN.
-It gets the modified files in the patch, assings them to projects and based on a
+This script will compute which projects are affected by the diff provided via STDIN.
+It gets the modified files in the patch, assigns them to projects and based on a
 project dependency graph it will get the transitively affected projects.
 """
 
@@ -25,56 +25,67 @@ import logging
 import os
 import platform
 import sys
-from typing import Dict, List, Set, Tuple, Optional
-
-from unidiff import PatchSet
+from typing import Any, Dict, List, Set, TextIO, Tuple, Optional, Union
+from unidiff import PatchSet  # type: ignore
 import yaml
 
-# TODO: We could also try to avoid running tests for llvm for projects that 
-#   only need various cmake scripts and don't actually depend on llvm (e.g.
-#   libcxx does not need to run llvm tests, but may still need to include llvm).
-
-
 class ChooseProjects:
-    
     # file where dependencies are defined
     SCRIPT_DIR = os.path.dirname(__file__)
     DEPENDENCIES_FILE = os.path.join(SCRIPT_DIR, 'llvm-dependencies.yaml')
-    # projects used if anything goes wrong
-    FALLBACK_PROJECTS = ['all']
 
     def __init__(self, llvm_dir: Optional[str]):
-        self.llvm_dir = llvm_dir  # type: Optional[str]
-        self.defaultProjects = dict()  # type: Dict[str, Dict[str, str]]
-        self.dependencies = dict()  # type: Dict[str,List[str]]
-        self.usages = dict()   # type: Dict[str,List[str]]
-        self.all_projects = []  # type: List[str]
-        self.config = {}
+        self.llvm_dir = llvm_dir
+        self.defaultProjects: Dict[str, Dict[str, str]] = {}
+         # List of projects this project depends on, transitive closure.
+         # E.g. compiler-rt -> [llvm, clang].
+        self.dependencies: Dict[str,Set[str]] = {}
+        # List of projects that depends on this project. It's a full closure.
+        # E.g. llvm -> [clang, libcxx, ...]
+        self.usages: Dict[str, Set[str]] = dict()
+        self.all_projects: List[str] = ['all']
+        self.config: Dict[str, Any] = {}
         self._load_config()
 
     def _load_config(self):
         logging.info('loading project config from {}'.format(self.DEPENDENCIES_FILE))
         with open(self.DEPENDENCIES_FILE) as dependencies_file:
             self.config = yaml.load(dependencies_file, Loader=yaml.SafeLoader)
-        self.dependencies = self.config['dependencies']
-        for user, used_list in self.dependencies.items():
-            for used in used_list:
-                self.usages.setdefault(used, []).append(user)
+        for k, v in self.config['dependencies'].items():
+            self.dependencies[k] = set(v)
+        # Closure of dependencies.
+        while True:
+            updated = False
+            for s in self.dependencies.values():
+                n = len(s)
+                extend = set()
+                for d in s:
+                    extend.update(self.dependencies.get(d, set()))
+                s.update(extend)
+                if len(s) > n:
+                    updated = True
+            if not updated:
+                break
+        # Usages don't need to be closed as dependencies already are.
+        for project, deps in self.dependencies.items():
+            for d in deps:
+                self.usages.setdefault(d, set()).add(project)
+        logging.info(f'computed dependencies: {self.dependencies}')
+        logging.info(f'computed usages: {self.usages}')
         self.all_projects = self.config['allprojects'].keys()
 
-    def get_excluded(self, target: str) -> Set[str]:
-        excluded = self.config['excludedProjects'][target]
-        return set(excluded if excluded is not None else [])
+    def get_excluded(self, os: str) -> Set[str]:
+        """Returns transitive closure for excluded projects"""
+        return self.get_affected_projects(set(self.config['excludedProjects'].get(os, [])))
 
-    def get_check_targets(self, affected_projects: Set[str]) -> Set[str]:
+    def get_check_targets(self, projects: Set[str]) -> Set[str]:
         """Return the `check-xxx` targets to pass to ninja for the given list of projects"""
+        if 'all' in projects:
+            return set(["check-all"])
         targets = set()
         all_projects = self.config['allprojects']
-        for project in affected_projects:
-            if project == "all":
-                targets = set(["check-all"])
-                return targets
-            targets.update(set(all_projects[project]))
+        for project in projects:
+            targets.update(set(all_projects.get(project, [])))
         return targets
 
     @staticmethod
@@ -84,59 +95,50 @@ class ChooseProjects:
             return 'windows'
         return 'linux'
 
-    def choose_projects(self, patch: str = None, current_os: str = None) -> List[str]:
+    def choose_projects(self, patch: str = None, os_name: Optional[str] = None) -> List[str]:
         """List all touched project with all projects that they depend on and also
         all projects that depend on them"""
         if self.llvm_dir is None:
             raise ValueError('path to llvm folder must be set in ChooseProject.')
-
         llvm_dir = os.path.abspath(os.path.expanduser(self.llvm_dir))
         logging.info('Scanning LLVM in {}'.format(llvm_dir))
         if not self.match_projects_dirs():
-            return self.FALLBACK_PROJECTS, set()
+            logging.warning(f'{llvm_dir} does not look like a llvm-project directory')
+            return self.get_all_enabled_projects(os_name)
         changed_files = self.get_changed_files(patch)
         changed_projects, unmapped_changes = self.get_changed_projects(changed_files)
-
         if unmapped_changes:
             logging.warning('There were changes that could not be mapped to a project.'
                             'Building all projects instead!')
-            return self.FALLBACK_PROJECTS, set()
-        return self.extend_projects(changed_projects, current_os)
+            return self.get_all_enabled_projects(os_name)
+        return self.extend_projects(changed_projects, os_name)
 
-    def extend_projects(self, projects: Set[str], current_os : str = None) -> List[str]:
+    def extend_projects(self, projects: Set[str], os_name : Optional[str] = None) -> List[str]:
+        """Given a set of projects returns a set of projects to be tested taking
+        in account exclusions from llvm-dependencies.yaml.
+        """
         logging.info(f'projects: {projects}')
-        if not current_os:
-            current_os = self._detect_os()
-        excluded_projects = self.get_excluded(current_os)
+        if not os_name:
+            os_name = self._detect_os()
+        # Find all affected by current set.
         affected_projects = self.get_affected_projects(projects)
-        logging.info(f'with affected projects: {affected_projects}')
-        to_exclude = affected_projects.intersection(excluded_projects)
-        if len(to_exclude):
-            logging.warning(f'{to_exclude} projects are excluded on {current_os}')
-        affected_projects = affected_projects - to_exclude
-        all_dependencies = set()
-        for project in affected_projects:
-            dependencies = self.get_dependencies(affected_projects)
-            logging.debug(f'> project {project} with dependencies: {dependencies}')
-            to_exclude = dependencies.intersection(excluded_projects)
-            if len(to_exclude) != 0:
-                logging.warning(f'Excluding project {project} because of excluded dependencies {to_exclude}')
-                affected_projects = affected_projects - project
-            else:
-                all_dependencies.update(dependencies)
-        logging.info(f'full dependencies: {all_dependencies}')
-        return sorted(affected_projects), sorted(all_dependencies)
+        logging.info(f'all affected projects(*) {affected_projects}')
+        # Exclude everything that is affected by excluded.
+        excluded_projects = self.get_excluded(os_name)
+        logging.info(f'all excluded projects(*) {excluded_projects}')
+        affected_projects = affected_projects - excluded_projects
+        logging.info(f'effective projects list {affected_projects}')
+        return sorted(affected_projects)
 
     def run(self):
-        affected_projects, dependencies = self.choose_projects()
+        affected_projects = self.choose_projects()
         print("Affected:", ';'.join(affected_projects))
-        print("Dependencies:", ';'.join(dependencies))
+        print("Dependencies:", ';'.join(self.get_dependencies(affected_projects)))
         print("Check targets:", ';'.join(self.get_check_targets(affected_projects)))
         return 0
 
     def match_projects_dirs(self) -> bool:
         """Make sure that all projects are folders in the LLVM dir.
-        Otherwise we can't create the regex...
         """
         subdirs = os.listdir(self.llvm_dir)
         for project in self.all_projects:
@@ -146,7 +148,7 @@ class ChooseProjects:
         return True
 
     @staticmethod
-    def get_changed_files(patch_str: str = None) -> Set[str]:
+    def get_changed_files(patch_str: Union[str, TextIO, None] = None) -> Set[str]:
         """get list of changed files from the patch or from STDIN.
         e.g. ['compiler-rt/lib/tsan/CMakeLists.txt']"""
         if patch_str is None:
@@ -179,49 +181,35 @@ class ChooseProjects:
 
     def get_affected_projects(self, changed_projects: Set[str]) -> Set[str]:
         """Compute transitive closure of affected projects based on the
-        dependencies between the projects."""
-        affected_projects = set(changed_projects)
-        last_len = -1
-        while len(affected_projects) != last_len:
-            last_len = len(affected_projects)
-            changes = set()
-            for project in affected_projects:
-                if project in self.usages:
-                    changes.update(self.usages[project])
-            affected_projects.update(changes)
-
-        logging.info(f'added {affected_projects - changed_projects} projects as they are affected')
-        return affected_projects
+        dependencies between the projects (including initially passed)."""
+        affected: Set[str] = set(changed_projects)
+        for p in changed_projects:
+            affected.update(self.usages.get(p, set()))
+        logging.info(f'added {affected - changed_projects} projects as they are affected')
+        return affected
 
     def get_dependencies(self, projects: Set[str]) -> Set[str]:
-        """Return transitive dependencies for a given project.
+        """Return transitive dependencies for a given projects (including the projects themself).
 
         These are the required dependencies for given `projects` so that they can be built.
         """
-        all_dependencies = set()
-        # Recursive call to add a project and all its dependencies to `all_dependencies`.
-        def add_dependencies_for_project(project: str):
-            if project in all_dependencies:
-                return
-            if project in self.dependencies:
-                for dependent in self.dependencies[project]:
-                    if dependent not in projects:
-                        all_dependencies.add(dependent)
-                        add_dependencies_for_project(dependent)
-        for project in projects:
-            add_dependencies_for_project(project)
-        return all_dependencies
+        affected: Set[str] = set(projects)
+        for p in projects:
+            affected.update(self.dependencies.get(p, set()))
+        return affected
 
-    def get_all_enabled_projects(self) -> List[str]:
+    def get_all_enabled_projects(self, os_name: Optional[str] = None) -> List[str]:
         """Get list of all not-excluded projects for current platform."""
-        return self.extend_projects(set(self.all_projects))
+        return self.extend_projects(set(self.all_projects), os_name)
 
 
 if __name__ == "__main__":
-    logging.basicConfig(filename='choose_projects.log', level=logging.INFO)
     parser = argparse.ArgumentParser(
         description='Compute the projects affected by a change. A patch file is expected on stdin.')
-    parser.add_argument('llvmdir', default='.')
+    parser.add_argument('--llvmdir', type=str, default='.')
+    parser.add_argument('--log-level', type=str, default='INFO')
     args = parser.parse_args()
+    logging.basicConfig(level=args.log_level, format='%(levelname)-7s %(message)s')
+    logging.info(f'checking changes in {args.llvmdir}')
     chooser = ChooseProjects(args.llvmdir)
     sys.exit(chooser.run())

scripts/llvm-dependencies.yaml

@@ -20,8 +20,8 @@ dependencies:
     - mlir
     - clang
   libc:
-  - clang
-  - clang-tools-extra
+    - clang
+    - clang-tools-extra
   # FIXME: not sure about 'libcxx' and 'libcxxabi'
   libcxx: []
   libcxxabi: []
@@ -46,8 +46,7 @@ dependencies:
 
 # List of all projects in the LLVM monorepository. This list is taken from
 # llvm/CMakeLists.txt in "set(LLVM_ALL_PROJECTS ..."
-# The value for all project is the list of targets to tests when this project
-# is affected by a patch.
+# The value for all project is the list of ninja targets to run.
 allprojects:
   clang: ["check-clang"]
   clang-tools-extra: ["check-clang-tools"]

scripts/pipeline_main.py

@@ -16,11 +16,12 @@
 # Script runs in checked out llvm-project directory.
 
 import os
-
+from typing import Dict
 from steps import generic_linux, generic_windows, from_shell_output, extend_steps_env, bazel
 from sync_fork import sync_fork
 import git
 import yaml
+from choose_projects import ChooseProjects
 
 steps_generators = [
     '${BUILDKITE_BUILD_CHECKOUT_PATH}/libcxx/utils/ci/buildkite-pipeline-snapshot.sh',
@@ -31,33 +32,30 @@ if __name__ == '__main__':
     no_cache = os.getenv('ph_no_cache') is not None
     log_level = os.getenv('ph_log_level', 'WARNING')
     notify_emails = list(filter(None, os.getenv('ph_notify_emails', '').split(',')))
-    # Syncing LLVM fork so any pipelines started from upstream llvm-project#
+    # Syncing LLVM fork so any pipelines started from upstream llvm-project
     # but then triggered a build on fork will observe the commit.
-    sync_fork(os.path.join(os.getenv('BUILDKITE_BUILD_PATH'), 'llvm-project-fork'), [os.getenv('BUILDKITE_BRANCH'), 'main'])
+    sync_fork(os.path.join(os.getenv('BUILDKITE_BUILD_PATH', ''), 'llvm-project-fork'), [os.getenv('BUILDKITE_BRANCH'), 'main'])
     steps = []
 
-    env = {}
+    env: Dict[str, str] = {}
     for e in os.environ:
         if e.startswith('ph_'):
-            env[e] = os.getenv(e)
+            env[e] = os.getenv(e, '')
     repo = git.Repo('.')
-    steps.extend(generic_linux(
-        os.getenv('ph_projects', 'llvm;clang;clang-tools-extra;libc;libcxx;libcxxabi;lld;libunwind;mlir;openmp;polly;flang'),
-        False))
-    # FIXME: openmp is removed as it constantly fails.
 
-    # TODO: Make this project list be evaluated through "choose_projects"(? as now we define "all" and exclusions in
-    #  two placess).
-    steps.extend(generic_windows(
-        os.getenv('ph_projects', 'llvm;clang;clang-tools-extra;libc;lld;mlir;polly;flang')))
+    cp = ChooseProjects(None)
+    linux_projects, _ = cp.get_all_enabled_projects('linux')
+    steps.extend(generic_linux(os.getenv('ph_projects', ';'.join(linux_projects)), check_diff=False))
+    windows_projects, _ = cp.get_all_enabled_projects('windows')
+    steps.extend(generic_windows(os.getenv('ph_projects', ';'.join(windows_projects))))
     steps.extend(bazel([], force=True))
     if os.getenv('ph_skip_generated') is None:
-        e = os.environ.copy()
+        env = os.environ.copy()
         # BUILDKITE_COMMIT might be an alias, e.g. "HEAD". Resolve it to make the build hermetic.
         if ('BUILDKITE_COMMIT' not in env) or (env['BUILDKITE_COMMIT'] == "HEAD"):
             env['BUILDKITE_COMMIT'] = repo.head.commit.hexsha
         for gen in steps_generators:
-            steps.extend(from_shell_output(gen, env=e))
+            steps.extend(from_shell_output(gen, env=env))
 
     notify = []
     for e in notify_emails:

scripts/pipeline_premerge.py

@@ -17,6 +17,7 @@
 
 import logging
 import os
+from typing import Dict
 
 from buildkite_utils import annotate, feedback_url, set_metadata
 from choose_projects import ChooseProjects
@@ -45,33 +46,33 @@ if __name__ == '__main__':
     set_metadata('ph_buildable_revision', os.getenv('ph_buildable_revision'))
     set_metadata('ph_build_id', os.getenv("ph_build_id"))
 
-    env = {}
+    env: Dict[str, str] = {}
     for e in os.environ:
         if e.startswith('ph_'):
-            env[e] = os.getenv(e)
+            env[e] = os.getenv(e, '')
     repo = git.Repo('.')
     steps = []
     # List all affected projects.
     patch = repo.git.diff("HEAD~1")
     cp = ChooseProjects('.')
 
-    linux_projects, dependencies = cp.choose_projects(patch = patch, current_os = "linux")
-    logging.info(f'linux_projects: {linux_projects} (dependencies: {dependencies}')
+    linux_projects = cp.choose_projects(patch = patch, os_name = "linux")
+    logging.info(f'linux_projects: {linux_projects}')
     if len(linux_projects) > 0:
-        steps.extend(generic_linux(';'.join(sorted(linux_projects)), True))
+        steps.extend(generic_linux(';'.join(linux_projects), check_diff=True))
 
-    windows_projects, dependencies = cp.choose_projects(patch = patch, current_os = "windows")
-    logging.info(f'windows_projects: {windows_projects} (dependencies: {dependencies}')
+    windows_projects = cp.choose_projects(patch = patch, os_name = "windows")
+    logging.info(f'windows_projects: {windows_projects}')
     if len(windows_projects) > 0:
-        steps.extend(generic_windows(';'.join(sorted(windows_projects))))
+        steps.extend(generic_windows(';'.join(windows_projects)))
 
     # Add custom checks.
     if os.getenv('ph_skip_generated') is None:
-        e = os.environ.copy()
+        env = os.environ.copy()
         # BUILDKITE_COMMIT might be an alias, e.g. "HEAD". Resolve it to make the build hermetic.
-        e["BUILDKITE_COMMIT"] = repo.head.commit.hexsha
+        env["BUILDKITE_COMMIT"] = repo.head.commit.hexsha
         for gen in steps_generators:
-            steps.extend(from_shell_output(gen, env=e))
+            steps.extend(from_shell_output(gen, env=env))
     modified_files = cp.get_changed_files(patch)
     steps.extend(bazel(modified_files))
 

scripts/premerge_checks.py

@@ -23,9 +23,7 @@ import re
 import shutil
 import sys
 import time
-from functools import partial
-from typing import Callable
-
+from typing import Callable, List, Type
 import clang_format_report
 import clang_tidy_report
 import run_cmake
@@ -146,7 +144,7 @@ if __name__ == '__main__':
         else:
             checks = " ".join(cp.get_check_targets(projects))
             logging.info(f"Running checks: {checks}")
-            report_lambda: Callable[Step, Report] = lambda s, r: ninja_check_projects_report(s, r, checks)
+            report_lambda: Callable[[Step, Report], None] = lambda s, r: ninja_check_projects_report(s, r, checks)
             run_step(f"ninja {checks}", report, report_lambda)
         if args.check_clang_tidy:
             if commands_in_build: