From a96a9a13db69d83d408285f45adb306aa9c790bc Mon Sep 17 00:00:00 2001 From: Alexey Alter-Pesotskiy Date: Thu, 10 Jul 2025 14:05:36 +0100 Subject: [PATCH] [CI] Improve linters and git hooks --- .github/workflows/cron-checks.yml | 2 +- .github/workflows/smoke-checks.yml | 17 +- Gemfile | 1 + Gemfile.lock | 2 + Scripts/bootstrap.sh | 14 +- fastlane/Fastfile | 21 +- fastlane/Sonarfile | 2 +- hooks/git-format-staged | 249 ---------------------- hooks/pre-commit.sh | 16 -- hooks/regenerage-spm-package-if-needed.sh | 6 - lefthook.yml | 9 + 11 files changed, 36 insertions(+), 303 deletions(-) delete mode 100755 hooks/git-format-staged delete mode 100755 hooks/pre-commit.sh delete mode 100755 hooks/regenerage-spm-package-if-needed.sh create mode 100644 lefthook.yml diff --git a/.github/workflows/cron-checks.yml b/.github/workflows/cron-checks.yml index 2a986ede0..725d42542 100644 --- a/.github/workflows/cron-checks.yml +++ b/.github/workflows/cron-checks.yml @@ -234,7 +234,7 @@ jobs: - uses: actions/checkout@v4.1.1 - uses: ./.github/actions/bootstrap - run: bundle exec fastlane rubocop - - run: bundle exec fastlane run_swift_format lint:true + - run: bundle exec fastlane run_swift_format strict:true - run: bundle exec fastlane pod_lint slack: diff --git a/.github/workflows/smoke-checks.yml b/.github/workflows/smoke-checks.yml index 899df915c..12b52fb6a 100644 --- a/.github/workflows/smoke-checks.yml +++ b/.github/workflows/smoke-checks.yml @@ -137,20 +137,13 @@ jobs: if: ${{ github.event.inputs.record_snapshots_swiftui != 'true' && github.event.inputs.record_snapshots_uikit != 'true' }} steps: - uses: actions/checkout@v4.1.1 - with: - fetch-depth: 0 - uses: ./.github/actions/bootstrap - - name: Public Interface Validation - run: bundle exec fastlane validate_public_interface - - name: Run Danger - run: bundle exec fastlane lint_pr - - name: Run Fastlane Linting - run: bundle exec fastlane rubocop - - name: Run SwiftFormat Linting - run: bundle exec fastlane run_swift_format lint:true - - name: Run Podspec Linting + - run: bundle exec fastlane validate_public_interface + - run: bundle exec fastlane lint_pr + - run: bundle exec fastlane rubocop + - run: bundle exec fastlane run_swift_format strict:true + - run: bundle exec fastlane pod_lint if: startsWith(github.event.pull_request.head.ref, 'release/') - run: bundle exec fastlane pod_lint build-xcode15: name: Build SDKs (Xcode 15) diff --git a/Gemfile b/Gemfile index 1b1a67aad..d07d02886 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,7 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } gem 'danger', group: :danger_dependencies gem 'fastlane', group: :fastlane_dependencies gem 'json' +gem 'lefthook' gem 'rubocop', '1.38', group: :rubocop_dependencies gem 'sinatra', group: :sinatra_dependencies gem 'slather' diff --git a/Gemfile.lock b/Gemfile.lock index 537da841a..c72dafb08 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -273,6 +273,7 @@ GEM rexml (>= 3.3.9) kramdown-parser-gfm (1.1.0) kramdown (~> 2.0) + lefthook (1.11.16) logger (1.7.0) method_source (1.1.0) mini_magick (4.13.2) @@ -427,6 +428,7 @@ DEPENDENCIES fastlane-plugin-stream_actions (= 0.3.84) fastlane-plugin-versioning json + lefthook plist puma rackup diff --git a/Scripts/bootstrap.sh b/Scripts/bootstrap.sh index 04f1ebfc7..70ed376ea 100755 --- a/Scripts/bootstrap.sh +++ b/Scripts/bootstrap.sh @@ -20,15 +20,11 @@ trap "echo ; echo ❌ The Bootstrap script failed to finish without error. See t source ./Githubfile -puts "Create git/hooks folder if needed" -mkdir -p .git/hooks - -# Symlink hooks folder to .git/hooks folder -puts "Create symlink for pre-commit hooks" -# Symlink needs to be ../../hooks and not ./hooks because git evaluates them in .git/hooks -ln -sf ../../hooks/pre-commit.sh .git/hooks/pre-commit -chmod +x .git/hooks/pre-commit -chmod +x ./hooks/git-format-staged +if [ "${GITHUB_ACTIONS:-}" != "true" ]; then + puts "Set up git hooks" + bundle install + bundle exec lefthook install +fi if [ "${SKIP_MINT_BOOTSTRAP:-}" != true ]; then puts "Bootstrap Mint dependencies" diff --git a/fastlane/Fastfile b/fastlane/Fastfile index d0e85f44d..f66788a9f 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -20,8 +20,6 @@ reversed_gci = gci.split('.').reverse.join('.') is_localhost = !is_ci @force_check = false swift_environment_path = File.absolute_path('../Sources/StreamVideo/Generated/SystemEnvironment+Version.swift') -swiftformat_excluded_paths = ["**/Generated", "**/generated", "**/protobuf", "**/OpenApi"] -swiftformat_source_paths = ["Sources", "DemoApp", "DemoAppUIKit", "StreamVideoTests", "StreamVideoSwiftUITests", "StreamVideoUIKitTests"] before_all do |lane| if is_ci @@ -227,7 +225,7 @@ end desc 'Runs LLC tests' lane :test do |options| - next unless is_check_required(sources: sources_matrix[:unit], force_check: @force_check) + next unless is_check_required(sources: sources_matrix[:llc], force_check: @force_check) update_testplan_on_ci(path: "StreamVideoTests/StreamVideo.xctestplan") @@ -673,10 +671,13 @@ end desc 'Run source code formatting/linting' lane :run_swift_format do |options| Dir.chdir('..') do - action = options[:lint] ? "--lint" : "" - swiftformat_source_paths.each do |path| - sh("mint run swiftformat #{action} --config .swiftformat --exclude #{swiftformat_excluded_paths.join(',')} #{path}") - sh("mint run swiftlint lint --config .swiftlint.yml --progress --quiet --reporter json #{path}") + strict = options[:strict] ? '--lint' : nil + sources_matrix[:swiftformat_include].each do |path| + sh("mint run swiftformat #{strict} --config .swiftformat --exclude #{sources_matrix[:swiftformat_exclude].join(',')} #{path}") + next if path.include?('Tests') + + sh("mint run swiftlint lint --config .swiftlint.yml --fix --progress --quiet --reporter json #{path}") unless strict + sh("mint run swiftlint lint --config .swiftlint.yml --strict --progress --quiet --reporter json #{path}") end end end @@ -700,7 +701,7 @@ end lane :sources_matrix do { e2e: ['Sources', 'DemoApp', 'SwiftUIDemoAppUITests', xcode_project], - unit: ['Sources', 'StreamVideoTests', xcode_project], + llc: ['Sources/StreamVideo', 'StreamVideoTests', xcode_project], swiftui: ['Sources', 'StreamVideoSwiftUITests', xcode_project], uikit: ['Sources', 'StreamVideoUIKitTests', xcode_project], swiftui_sample_apps: ['Sources', 'DemoApp', xcode_project], @@ -708,7 +709,9 @@ lane :sources_matrix do documentation_tests: ['Sources', 'DocumentationTests', xcode_project], size: ['Sources', xcode_project], public_interface: ['Sources'], - ruby: ['fastlane'] + ruby: ['fastlane'], + swiftformat_include: ["Sources", "DemoApp", "DemoAppUIKit", "StreamVideoTests", "StreamVideoSwiftUITests", "StreamVideoUIKitTests"], + swiftformat_exclude: ["**/Generated", "**/generated", "**/protobuf", "**/OpenApi"] } end diff --git a/fastlane/Sonarfile b/fastlane/Sonarfile index d8c2a55de..d88008f98 100755 --- a/fastlane/Sonarfile +++ b/fastlane/Sonarfile @@ -2,7 +2,7 @@ desc 'Get code coverage report and run complexity analysis for Sonar' lane :sonar_upload do - next unless is_check_required(sources: sources_matrix[:unit], force_check: @force_check) + next unless is_check_required(sources: sources_matrix[:llc], force_check: @force_check) version_number = get_version_number( xcodeproj: 'StreamVideo.xcodeproj', diff --git a/hooks/git-format-staged b/hooks/git-format-staged deleted file mode 100755 index 43afabd60..000000000 --- a/hooks/git-format-staged +++ /dev/null @@ -1,249 +0,0 @@ -#!/usr/bin/env python -# -# Git command to transform staged files according to a command that accepts file -# content on stdin and produces output on stdout. This command is useful in -# combination with `git add -p` which allows you to stage specific changes in -# a file. This command runs a formatter on the file with staged changes while -# ignoring unstaged changes. -# -# Usage: git-format-staged [OPTION]... [FILE]... -# Example: git-format-staged --formatter 'prettier --stdin' '*.js' -# -# Tested with Python 3.6 and Python 2.7. -# -# Original author: Jesse Hallett - -from __future__ import print_function -import argparse -from fnmatch import fnmatch -from gettext import gettext as _ -import os -import re -import subprocess -import sys - -# The string $VERSION is replaced during the publish process. -VERSION = '$VERSION' -PROG = sys.argv[0] - -def info(msg): - print(msg, file=sys.stderr) - -def warn(msg): - print('{}: warning: {}'.format(PROG, msg), file=sys.stderr) - -def fatal(msg): - print('{}: error: {}'.format(PROG, msg), file=sys.stderr) - exit(1) - -def format_staged_files(file_patterns, formatter, git_root, update_working_tree=True, write=True): - try: - output = subprocess.check_output([ - 'git', 'diff-index', - '--cached', - '--diff-filter=AM', # select only file additions and modifications - '--no-renames', - 'HEAD' - ]) - for line in output.splitlines(): - entry = parse_diff(line.decode('utf-8')) - entry_path = normalize_path(entry['src_path'], relative_to=git_root) - if not (matches_some_path(file_patterns, entry_path)): - continue - if format_file_in_index(formatter, entry, update_working_tree=update_working_tree, write=write): - info('Reformatted {} with {}'.format(entry['src_path'], formatter)) - except Exception as err: - fatal(str(err)) - -# Run formatter on file in the git index. Creates a new git object with the -# result, and replaces the content of the file in the index with that object. -# Returns hash of the new object if formatting produced any changes. -def format_file_in_index(formatter, diff_entry, update_working_tree=True, write=True): - orig_hash = diff_entry['dst_hash'] - new_hash = format_object(formatter, orig_hash, diff_entry['src_path']) - - # If the new hash is the same then the formatter did not make any changes. - if not write or new_hash == orig_hash: - return None - - replace_file_in_index(diff_entry, new_hash) - - if update_working_tree: - try: - patch_working_file(diff_entry['src_path'], orig_hash, new_hash) - except Exception as err: - # Errors patching working tree files are not fatal - warn(str(err)) - - return new_hash - -file_path_placeholder = re.compile('\{\}') - -# Run formatter on a git blob identified by its hash. Writes output to a new git -# blob, and returns the hash of the new blob. -def format_object(formatter, object_hash, file_path): - get_content = subprocess.Popen( - ['git', 'cat-file', '-p', object_hash], - stdout=subprocess.PIPE - ) - format_content = subprocess.Popen( - re.sub(file_path_placeholder, file_path, formatter), - shell=True, - stdin=get_content.stdout, - stdout=subprocess.PIPE - ) - write_object = subprocess.Popen( - ['git', 'hash-object', '-w', '--stdin'], - stdin=format_content.stdout, - stdout=subprocess.PIPE - ) - - get_content.stdout.close() - format_content.stdout.close() - - if get_content.wait() != 0: - raise ValueError('unable to read file content from object database: ' + object_hash) - - if format_content.wait() != 0: - raise Exception('formatter exited with non-zero status') # TODO: capture stderr from format command - - new_hash, err = write_object.communicate() - - if write_object.returncode != 0: - raise Exception('unable to write formatted content to object database') - - return new_hash.decode('utf-8').rstrip() - -def replace_file_in_index(diff_entry, new_object_hash): - subprocess.check_call(['git', 'update-index', - '--cacheinfo', '{},{},{}'.format( - diff_entry['dst_mode'], - new_object_hash, - diff_entry['src_path'] - )]) - -def patch_working_file(path, orig_object_hash, new_object_hash): - patch = subprocess.check_output( - ['git', 'diff', orig_object_hash, new_object_hash] - ) - - # Substitute object hashes in patch header with path to working tree file - patch_b = patch.replace(orig_object_hash.encode(), path.encode()).replace(new_object_hash.encode(), path.encode()) - - apply_patch = subprocess.Popen( - ['git', 'apply', '-'], - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE - ) - - output, err = apply_patch.communicate(input=patch_b) - - if apply_patch.returncode != 0: - raise Exception('could not apply formatting changes to working tree file {}'.format(path)) - -# Format: src_mode dst_mode src_hash dst_hash status/score? src_path dst_path? -diff_pat = re.compile('^:(\d+) (\d+) ([a-f0-9]+) ([a-f0-9]+) ([A-Z])(\d+)?\t([^\t]+)(?:\t([^\t]+))?$') - -# Parse output from `git diff-index` -def parse_diff(diff): - m = diff_pat.match(diff) - if not m: - raise ValueError('Failed to parse diff-index line: ' + diff) - return { - 'src_mode': unless_zeroed(m.group(1)), - 'dst_mode': unless_zeroed(m.group(2)), - 'src_hash': unless_zeroed(m.group(3)), - 'dst_hash': unless_zeroed(m.group(4)), - 'status': m.group(5), - 'score': int(m.group(6)) if m.group(6) else None, - 'src_path': m.group(7), - 'dst_path': m.group(8) - } - -zeroed_pat = re.compile('^0+$') - -# Returns the argument unless the argument is a string of zeroes, in which case -# returns `None` -def unless_zeroed(s): - return s if not zeroed_pat.match(s) else None - -def get_git_root(): - return subprocess.check_output( - ['git', 'rev-parse', '--show-toplevel'] - ).decode('utf-8').rstrip() - -def normalize_path(p, relative_to=None): - return os.path.abspath( - os.path.join(relative_to, p) if relative_to else p - ) - -def matches_some_path(patterns, target): - is_match = False - for signed_pattern in patterns: - (is_pattern_positive, pattern) = from_signed_pattern(signed_pattern) - if fnmatch(target, normalize_path(pattern)): - is_match = is_pattern_positive - return is_match - -# Checks for a '!' as the first character of a pattern, returns the rest of the -# pattern in a tuple. The tuple takes the form (is_pattern_positive, pattern). -# For example: -# from_signed_pattern('!pat') == (False, 'pat') -# from_signed_pattern('pat') == (True, 'pat') -def from_signed_pattern(pattern): - if pattern[0] == '!': - return (False, pattern[1:]) - else: - return (True, pattern) - -class CustomArgumentParser(argparse.ArgumentParser): - def parse_args(self, args=None, namespace=None): - args, argv = self.parse_known_args(args, namespace) - if argv: - msg = argparse._( - 'unrecognized arguments: %s. Do you need to quote your formatter command?' - ) - self.error(msg % ' '.join(argv)) - return args - -if __name__ == '__main__': - parser = CustomArgumentParser( - description='Transform staged files using a formatting command that accepts content via stdin and produces a result via stdout.', - epilog='Example: %(prog)s --formatter "prettier --stdin" "src/*.js" "test/*.js"' - ) - parser.add_argument( - '--formatter', '-f', - required=True, - help='Shell command to format files, will run once per file. Occurrences of the placeholder `{}` will be replaced with a path to the file being formatted. (Example: "prettier --stdin --stdin-filepath \'{}\'")' - ) - parser.add_argument( - '--no-update-working-tree', - action='store_true', - help='By default formatting changes made to staged file content will also be applied to working tree files via a patch. This option disables that behavior, leaving working tree files untouched.' - ) - parser.add_argument( - '--no-write', - action='store_true', - help='Prevents %(prog)s from modifying staged or working tree files. You can use this option to check staged changes with a linter instead of formatting. With this option stdout from the formatter command is ignored. Example: %(prog)s --no-write -f "eslint --stdin --stdin-filename \'{}\' >&2" "*.js"' - ) - parser.add_argument( - '--version', - action='version', - version='%(prog)s version {}'.format(VERSION), - help='Display version of %(prog)s' - ) - parser.add_argument( - 'files', - nargs='+', - help='Patterns that specify files to format. The formatter will only transform staged files that are given here. Patterns may be literal file paths, or globs which will be tested against staged file paths using Python\'s fnmatch function. For example "src/*.js" will match all files with a .js extension in src/ and its subdirectories. Patterns may be negated to exclude files using a "!" character. Patterns are evaluated left-to-right. (Example: "main.js" "src/*.js" "test/*.js" "!test/todo/*")' - ) - args = parser.parse_args() - files = vars(args)['files'] - format_staged_files( - file_patterns=files, - formatter=vars(args)['formatter'], - git_root=get_git_root(), - update_working_tree=not vars(args)['no_update_working_tree'], - write=not vars(args)['no_write'] - ) diff --git a/hooks/pre-commit.sh b/hooks/pre-commit.sh deleted file mode 100755 index f702aedba..000000000 --- a/hooks/pre-commit.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/bash - -# Exit immediately if a command exits with a non-zero status. -set -e - -if [ "${GITHUB_ACTIONS}" != "true" ]; then - # Define an array of paths - paths=("Sources" "DemoApp" "DemoAppUIKit" "StreamVideoTests" "StreamVideoUIKitTests") - - # Loop through each path - for path in "${paths[@]}"; do - mint run swiftformat --lint --config .swiftformat \ - --exclude "**/Generated","**/generated","**/protobuf","**/OpenApi" \ - "$path" - done -fi diff --git a/hooks/regenerage-spm-package-if-needed.sh b/hooks/regenerage-spm-package-if-needed.sh deleted file mode 100755 index ac1ef3e34..000000000 --- a/hooks/regenerage-spm-package-if-needed.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash -# Regenerate list of files in Package.swift -arch -x86_64 ./generateSPMFileLists - -# Add changes to the current commit -git add Package.swift \ No newline at end of file diff --git a/lefthook.yml b/lefthook.yml new file mode 100644 index 000000000..30b72f3b5 --- /dev/null +++ b/lefthook.yml @@ -0,0 +1,9 @@ +pre-commit: + parallel: true + jobs: + - run: bundle exec fastlane run_swift_format + glob: "*.{swift}" + stage_fixed: true + skip: + - merge + - rebase