From 7a0ff7d715ff5ffc402902d65e448a43cf6bdcbd Mon Sep 17 00:00:00 2001 From: Silke Hofstra Date: Sun, 31 Mar 2024 18:23:06 +0200 Subject: [PATCH 1/3] ci/package_checks: Add `--results-only` and `--fail-on-warnings` flags **Summary** Add flags for: - Only showing the results, no summaries about what is checked. - Exiting with an error code when warnings are found. --- common/CI/package_checks.py | 39 ++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/common/CI/package_checks.py b/common/CI/package_checks.py index 900d02ec334..33f5e083df8 100755 --- a/common/CI/package_checks.py +++ b/common/CI/package_checks.py @@ -507,7 +507,7 @@ class PackageVersion(PullRequestCheck): def run(self) -> List[Result]: return [Result(message=self._error, level=self._level, - file=path, line=self.file_line(path, r'^version\s*:'),) + file=path, line=self.file_line(path, r'^version\s*:'), ) for path in self.package_files if not self._check_version(path)] @@ -730,7 +730,10 @@ class Checker: UnwantedFiles, ] - def __init__(self, base: Optional[str], head: str, path: str, modified: bool, untracked: bool, files: List[str]): + def __init__(self, base: Optional[str], head: str, path: str, files: List[str], + modified: bool, untracked: bool, results_only: bool, exit_warn: bool): + self.results_only = results_only + self.exit_warn = exit_warn self.base = base self.head = head self.git = Git(path) @@ -750,22 +753,25 @@ def __init__(self, base: Optional[str], head: str, path: str, modified: bool, un self.files += self.git.untracked_files() def run(self) -> bool: - print(f'Checking files: {", ".join(self.files)}') - if self.commits: - print(f'Checking commits: {", ".join(self.commits)}') + if not self.results_only: + print(f'Checking files: {", ".join(self.files)}') + if self.commits: + print(f'Checking commits: {", ".join(self.commits)}') results = [result for check in self.checks for result in check(self.git, self.files, self.commits, self.base).run()] errors = [r for r in results if r.level == Level.ERROR] + warnings = [r for r in results if r.level == Level.WARNING] - print(f"Found {len(results)} result(s), {len(errors)} error(s)") + if not self.results_only: + print(f"Found {len(results)} result(s), {len(warnings)} warnings and {len(errors)} error(s)") for result in results: result.log() self.write_summary() - return len(errors) > 0 + return len(errors) > 0 or self.exit_warn and len(warnings) > 0 def write_summary(self) -> None: if self.summary_file is None: @@ -793,13 +799,20 @@ def _base_to_remote(base: str) -> List[str]: help='Include modified files') parser.add_argument('--untracked', action='store_true', help='Include untracked files') + parser.add_argument('--fail-on-warnings', action='store_true', + help='Exit with an error if warnings are encountered') + parser.add_argument('--results-only', action='store_true', + help='Only show results, nothing else') parser.add_argument('filename', type=str, nargs="*", help='Additional files to check') + cli_args = parser.parse_args() - checker = Checker(cli_args.base, - cli_args.head, - cli_args.root, - cli_args.modified, - cli_args.untracked, - cli_args.filename) + checker = Checker(base=cli_args.base, + head=cli_args.head, + path=cli_args.root, + modified=cli_args.modified, + untracked=cli_args.untracked, + files=cli_args.filename, + results_only=cli_args.results_only, + exit_warn=cli_args.fail_on_warnings) exit(checker.run()) From afce3a9b5283a41a2629c78bee9654214aa75b94 Mon Sep 17 00:00:00 2001 From: Silke Hofstra Date: Sun, 31 Mar 2024 18:25:15 +0200 Subject: [PATCH 2/3] common: Rework `package-publish-safety-catches.sh` to use the CI checks **Summary** Run the CI checks in the safety catches instead of using a separate implementation. --- .github/workflows/script_lint.yml | 1 - .../Scripts/package-publish-safety-catches.sh | 57 +++++-------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/.github/workflows/script_lint.yml b/.github/workflows/script_lint.yml index 3f7742ad8bd..d809807d3df 100644 --- a/.github/workflows/script_lint.yml +++ b/.github/workflows/script_lint.yml @@ -40,7 +40,6 @@ jobs: check_together: 'yes' ignore_paths: >- common/Legacy/** - common/Scripts/package-publish-safety-catches.sh common/Scripts/helpers.zsh common/Scripts/sync_licenses.sh common/Scripts/new-package.sh diff --git a/common/Scripts/package-publish-safety-catches.sh b/common/Scripts/package-publish-safety-catches.sh index 6b627570682..9d85d8aecbd 100755 --- a/common/Scripts/package-publish-safety-catches.sh +++ b/common/Scripts/package-publish-safety-catches.sh @@ -1,51 +1,20 @@ #!/usr/bin/env bash +set -euo pipefail -# Safety catches when publishing packages. Not foolproof and there are edge cases where we don't want it which allows for force-pushing. -# Currently: - # - Checks that the release has been bumped. - # - Warns if there are additions to abi_used_libs for system.{base,devel} packages. +check_script="$(dirname "$0")/../CI/package_checks.py" +check_args=(--base=origin/main --fail-on-warnings --results-only) -# FIXME: Check for rundeps changes as well for system.{base,devel} packages. -# FIXME: For the initial inclusion check that the release == 1 - -LAST_COMMIT=$(git rev-list -1 HEAD .) - -LAST_COMMIT_DIFF=$(git log -u -1 ${LAST_COMMIT}) - -PKG_BUMP=$(git log -u -1 ${LAST_COMMIT} package.yml | grep -w +release) - -# Check the release has been bumped -if [[ $PKG_BUMP == "" ]]; then - echo "Warning: Cannot determine that the release has been bumped" - read -p "Press y to force-through. If unsure press any other key to abort." prompt - if [[ $prompt = "y" ]]; then - exit 0 - else - exit 1 - fi +if python3 "${check_script}" "${check_args[@]}" +then + exit 0 fi -# Checks for additions to abi_used_libs for system.{base,devel} packages. - -if [[ `git grep -E 'system.base|system.devel' pspec_x86_64.xml` ]]; then - SYSTEM_BASE_DEVEL_PKG=1 -fi - -if [[ `grep -E abi_used_libs <<< $LAST_COMMIT_DIFF` ]]; then - # Only if the change is an addition - ABI_ADDITION=`git log -u -1 ${LAST_COMMIT} -U0 --word-diff abi_used_libs | grep {+` - if [[ $ABI_ADDITION != "" ]]; then - CHANGED_ABI_USED_LIBS=1 - fi -fi +echo "Package checks failed. Press 'y' to continue, or any other key to abort." +read -rp "Continue anyway? [yN] " prompt -if [[ ! -z "${SYSTEM_BASE_DEVEL_PKG}" && ! -z "${CHANGED_ABI_USED_LIBS}" ]]; then - echo "Found a system.base/system.devel pkg where" $ABI_ADDITION "has been added to abi_used_libs." - echo "Please ensure that the package containing" $ABI_ADDITION "is in system.base/system.devel BEFORE continuing." - read -p "Press y to continue. If unsure press any other key to abort." prompt - if [[ $prompt = "y" ]]; then - exit 0 - else - exit 1 - fi +if [[ $prompt = "y" ]] +then + exit 0 +else + exit 1 fi From 94d54fc6c842ad287d98860fb64e5344e8df625f Mon Sep 17 00:00:00 2001 From: Silke Hofstra Date: Sun, 31 Mar 2024 18:40:51 +0200 Subject: [PATCH 3/3] Taskfile: improve descriptions of `publish`, `republish` and `run-safety-catches` tasks. --- Taskfile.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 2551ac598a2..be4f9fc54f2 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -162,7 +162,9 @@ tasks: # For staff and packagers with push access publish: - desc: Tag and publish a release + desc: >- + Publish the last commit for the current package to the repository and the build server. + The `run-safety-catches` task is used to check for common issues before pushing. dir: '{{ .USER_WORKING_DIR }}' preconditions: - sh: test $(git symbolic-ref HEAD 2>/dev/null) = "refs/heads/main" @@ -175,7 +177,9 @@ tasks: - task: push republish: - desc: Rebuild existing tag + desc: >- + Retry the last commit for the current package on the build server. + The `run-safety-catches` task is used to check for common issues before pushing. dir: '{{ .USER_WORKING_DIR }}' preconditions: - sh: test $(git symbolic-ref HEAD 2>/dev/null) = "refs/heads/main" @@ -185,7 +189,7 @@ tasks: - task: push run-safety-catches: - desc: Run safety catches script + desc: Check for issues in the currently staged commits using the CI package checks. dir: '{{ .USER_WORKING_DIR }}' cmds: - "{{ .TASKFILE_DIR }}/common/Scripts/package-publish-safety-catches.sh"