From fc802da68cfd8be0bf67e5060b16ed1a7d956278 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Thu, 25 Jan 2024 12:45:13 +0100 Subject: [PATCH] ci: lint yaml files that use `dependencies: []` together with `needs` --- .gitlab/ci/build.yml | 2 + .gitlab/ci/pre_check.yml | 2 +- tools/ci/generate_rules.py | 12 ++++-- tools/ci/gitlab_yaml_linter.py | 43 +++++++++++++------ tools/ci/idf_ci_utils.py | 77 +++++++++++++++++++++++++++------- 5 files changed, 102 insertions(+), 34 deletions(-) diff --git a/.gitlab/ci/build.yml b/.gitlab/ci/build.yml index a62517c6ba..43d650c7fe 100644 --- a/.gitlab/ci/build.yml +++ b/.gitlab/ci/build.yml @@ -172,6 +172,7 @@ build_clang_test_apps_esp32c6: extends: - .build_template - .rules:build:check + dependencies: # set dependencies to null to avoid missing artifacts issue needs: - job: fast_template_app artifacts: false @@ -272,6 +273,7 @@ build_template_app: - .build_template_app_template - .rules:build stage: host_test + dependencies: # set dependencies to null to avoid missing artifacts issue needs: - job: fast_template_app artifacts: false diff --git a/.gitlab/ci/pre_check.yml b/.gitlab/ci/pre_check.yml index ead57430a7..7cfa437be9 100644 --- a/.gitlab/ci/pre_check.yml +++ b/.gitlab/ci/pre_check.yml @@ -3,7 +3,7 @@ image: $ESP_ENV_IMAGE tags: - host_test - dependencies: [] + dependencies: # set dependencies to null to avoid missing artifacts issue check_pre_commit: extends: diff --git a/tools/ci/generate_rules.py b/tools/ci/generate_rules.py index 072c22f974..4dc9534039 100755 --- a/tools/ci/generate_rules.py +++ b/tools/ci/generate_rules.py @@ -1,8 +1,7 @@ #!/usr/bin/env python # -# SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 - import argparse import inspect import os @@ -11,7 +10,8 @@ from collections import defaultdict from itertools import product import yaml -from idf_ci_utils import IDF_PATH, GitlabYmlConfig +from idf_ci_utils import GitlabYmlConfig +from idf_ci_utils import IDF_PATH try: import pygraphviz as pgv @@ -201,9 +201,13 @@ class RulesWriter: def new_rules_str(self): # type: () -> str res = [] for k, v in sorted(self.rules.items()): - if '.rules:' + k not in self.yml_config.used_rules: + if k.startswith('pattern'): + continue + + if '.rules:' + k not in self.yml_config.used_templates: print(f'WARNING: unused rule: {k}, skipping...') continue + res.append(self.RULES_TEMPLATE.format(k, self._format_rule(k, v))) return '\n\n'.join(res) diff --git a/tools/ci/gitlab_yaml_linter.py b/tools/ci/gitlab_yaml_linter.py index 0f9750f0d5..e68cfd83da 100755 --- a/tools/ci/gitlab_yaml_linter.py +++ b/tools/ci/gitlab_yaml_linter.py @@ -1,18 +1,17 @@ #!/usr/bin/env python - -# SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 - """ Check gitlab ci yaml files """ - import argparse import os import typing as t from functools import cached_property -from idf_ci_utils import IDF_PATH, GitlabYmlConfig, get_submodule_dirs +from idf_ci_utils import get_submodule_dirs +from idf_ci_utils import GitlabYmlConfig +from idf_ci_utils import IDF_PATH class YmlLinter: @@ -50,9 +49,10 @@ class YmlLinter: if ( k not in self.yml_config.global_keys and k not in self.yml_config.anchors + and k not in self.yml_config.templates and k not in self.yml_config.jobs ): - raise SystemExit(f'Parser incorrect. Key {k} not in global keys, rules or jobs') + raise SystemExit(f'Parser incorrect. Key {k} not in global keys, anchors, templates, or jobs') def _lint_default_values_artifacts(self) -> None: defaults_artifacts = self.yml_config.default.get('artifacts', {}) @@ -79,13 +79,30 @@ class YmlLinter: for item in undefined_patterns: self._errors.append(f'undefined pattern {item}. Please add {item} to .patterns-submodule') - def _lint_gitlab_yml_rules(self) -> None: - unused_rules = self.yml_config.rules - self.yml_config.used_rules - for item in unused_rules: - self._errors.append(f'Unused rule: {item}, please remove it') - undefined_rules = self.yml_config.used_rules - self.yml_config.rules - for item in undefined_rules: - self._errors.append(f'Undefined rule: {item}') + def _lint_gitlab_yml_templates(self) -> None: + unused_templates = self.yml_config.templates.keys() - self.yml_config.used_templates + for item in unused_templates: + # known unused ones + if item not in [ + '.before_script:fetch:target_test', # used in dynamic pipeline + ]: + self._errors.append(f'Unused template: {item}, please remove it') + + undefined_templates = self.yml_config.used_templates - self.yml_config.templates.keys() + for item in undefined_templates: + self._errors.append(f'Undefined template: {item}') + + def _lint_dependencies_and_needs(self) -> None: + """ + Use `dependencies: []` together with `needs: []` could cause missing artifacts issue. + """ + for job_name, d in self.yml_config.jobs.items(): + if 'dependencies' in d and 'needs' in d: + if d['dependencies'] is not None and d['needs']: + self._errors.append( + f'job {job_name} has both `dependencies` and `needs` defined. ' + f'Please set `dependencies:` (to null) explicitly to avoid missing artifacts issue' + ) if __name__ == '__main__': diff --git a/tools/ci/idf_ci_utils.py b/tools/ci/idf_ci_utils.py index 70d0675f44..ecabd22ada 100644 --- a/tools/ci/idf_ci_utils.py +++ b/tools/ci/idf_ci_utils.py @@ -125,7 +125,9 @@ class GitlabYmlConfig: all_config = dict() root_yml = yaml.load(open(root_yml_filepath), Loader=yaml.FullLoader) - for item in root_yml['include']: + + # expanding "include" + for item in root_yml.pop('include', []) or []: all_config.update(yaml.load(open(os.path.join(IDF_PATH, item)), Loader=yaml.FullLoader)) if 'default' in all_config: @@ -133,6 +135,16 @@ class GitlabYmlConfig: self._config = all_config + # anchor is the string that will be reused in templates + self._anchor_keys: t.Set[str] = set() + # template is a dict that will be extended + self._template_keys: t.Set[str] = set() + self._used_template_keys: t.Set[str] = set() # tracing the used templates + # job is a dict that will be executed + self._job_keys: t.Set[str] = set() + + self.expand_extends() + @property def default(self) -> t.Dict[str, t.Any]: return self._defaults @@ -147,33 +159,66 @@ class GitlabYmlConfig: @cached_property def anchors(self) -> t.Dict[str, t.Any]: - return {k: v for k, v in self.config.items() if k.startswith('.')} + return {k: v for k, v in self.config.items() if k in self._anchor_keys} @cached_property def jobs(self) -> t.Dict[str, t.Any]: - return {k: v for k, v in self.config.items() if not k.startswith('.') and k not in self.global_keys} + return {k: v for k, v in self.config.items() if k in self._job_keys} @cached_property - def rules(self) -> t.Set[str]: - return {k for k, _ in self.anchors.items() if self._is_rule_key(k)} + def templates(self) -> t.Dict[str, t.Any]: + return {k: v for k, v in self.config.items() if k in self._template_keys} @cached_property - def used_rules(self) -> t.Set[str]: - res = set() + def used_templates(self) -> t.Set[str]: + return self._used_template_keys - for v in self.config.values(): - if not isinstance(v, dict): + def expand_extends(self) -> None: + """ + expand the `extends` key in-place. + """ + for k, v in self.config.items(): + if k in self.global_keys: continue - for item in to_list(v.get('extends')): - if self._is_rule_key(item): - res.add(item) + if isinstance(v, (str, list)): + self._anchor_keys.add(k) + elif k.startswith('.if-'): + self._anchor_keys.add(k) + elif k.startswith('.'): + self._template_keys.add(k) + elif isinstance(v, dict): + self._job_keys.add(k) + else: + raise ValueError(f'Unknown type for key {k} with value {v}') - return res + # no need to expand anchor - @staticmethod - def _is_rule_key(key: str) -> bool: - return key.startswith('.rules:') or key.endswith('template') + # expand template first + for k in self._template_keys: + self._expand_extends(k) + + # expand job + for k in self._job_keys: + self._expand_extends(k) + + def _expand_extends(self, name: str) -> t.Dict[str, t.Any]: + extends = to_list(self.config[name].pop('extends', None)) + original_d = self.config[name].copy() + if not extends: + return self.config[name] # type: ignore + + d = {} + while extends: + self._used_template_keys.update(extends) + + for i in extends: + d.update(self._expand_extends(i)) + + extends = to_list(self.config[name].pop('extends', None)) + + self.config[name] = {**d, **original_d} + return self.config[name] # type: ignore def get_all_manifest_files() -> t.List[str]: