Skip to content

ci: pin GitHub Actions runners to explicit OS versions#508

Merged
qin-ctx merged 2 commits intomainfrom
codex-ci-macos14-support
Mar 10, 2026
Merged

ci: pin GitHub Actions runners to explicit OS versions#508
qin-ctx merged 2 commits intomainfrom
codex-ci-macos14-support

Conversation

@zhoujh01
Copy link
Collaborator

Description

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

macOS Wheel Config

Verify that the explicit macOS version/architecture mapping (macos-14 → arm64, macos-15-intel → x86_64) aligns with the actual runner labels and wheel tagging requirements.

- name: Configure macOS wheel architecture tag
  if: runner.os == 'macOS'
  shell: bash
  run: |
    if [[ "${{ matrix.os }}" == "macos-14" ]]; then
      TARGET_ARCH="arm64"
      MACOS_VERSION="14.0"
    elif [[ "${{ matrix.os }}" == "macos-15-intel" ]]; then
      TARGET_ARCH="x86_64"
      MACOS_VERSION="15.0"
    else
      echo "Unsupported macOS runner for release wheels: ${{ matrix.os }}"
      exit 1
    fi
Runner Label Validity

Confirm that 'macos-15-intel' and 'macos-14' are valid GitHub Actions runner labels (either GitHub-hosted or self-hosted) for the project.

- os: macos-15-intel
  target: x86_64-apple-darwin
  artifact_name: ov-macos-x86_64
- os: macos-14

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Include user site-packages in extension check

Add site.USER_SITE to the list of directories checked for the native extension. This
ensures the smoke test works even if the wheel was installed in the user
site-packages directory instead of the system site-packages.

.github/workflows/_build.yml [461-463]

 candidates = []
-for base in site.getsitepackages():
+bases = site.getsitepackages() + [site.USER_SITE]
+for base in bases:
     candidates.extend(pathlib.Path(base).glob("openviking/storage/vectordb/engine*.so"))
Suggestion importance[1-10]: 5

__

Why: The suggestion improves the reliability of the macOS wheel verification smoke test by also checking the user site-packages directory for the native extension, which addresses a potential edge case where the wheel is installed there instead of system site-packages.

Low

(
contains(inputs.os_json, 'linux') ||
contains(inputs.os_json, 'ubuntu-latest') ||
contains(inputs.os_json, 'ubuntu-24.04') ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] contains(inputs.os_json, 'ubuntu-24.04') 会与 ubuntu-24.04-arm 产生子串误匹配,导致构建矩阵错误。

问题详述: contains() 是纯字符串子串匹配,不是 JSON 数组元素精确匹配。当用户手动触发工作流仅传入 os_json = '["ubuntu-24.04-arm"]'(只想构建 ARM 架构)时:

  • 旧代码:contains('["ubuntu-24.04-arm"]', 'ubuntu-latest')false ✅ 不会误匹配
  • 新代码:contains('["ubuntu-24.04-arm"]', 'ubuntu-24.04')true ❌ 子串误匹配!

这会导致:

  1. build-linux job 的 if 条件提前为 true(L122)
  2. runs-on fallback 解析为 ubuntu-24.04(L127),产生非预期的 x86_64 构建
  3. arch 矩阵表达式(L136)中 contains(..., 'ubuntu-24.04') && contains(..., 'ubuntu-24.04-arm') 均为 true,错误地构建 x86_64 + aarch64,而用户只想构建 aarch64

修复建议: 利用 JSON 数组中的双引号做精确匹配,将所有 contains(inputs.os_json, 'ubuntu-24.04') 替换为 contains(inputs.os_json, '"ubuntu-24.04"')

# 修复前
contains(inputs.os_json, 'ubuntu-24.04')

# 修复后
contains(inputs.os_json, '"ubuntu-24.04"')

contains('["ubuntu-24.04-arm"]', '"ubuntu-24.04"')false,因为 JSON 中 "ubuntu-24.04-arm" 不包含子串 "ubuntu-24.04"(后面紧跟的是 -arm" 而非 ")。

此问题同样影响本文件 L127 和 L136。

# Run only when non-Linux runners are explicitly requested
if: inputs.build_wheels && (contains(inputs.os_json, 'macos') || contains(inputs.os_json, 'windows'))
runs-on: ${{ matrix.os || 'ubuntu-latest' }}
runs-on: ${{ matrix.os || 'ubuntu-24.04' }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] build-other job 的 fallback runs-on: ${{ matrix.os || 'ubuntu-24.04' }} 中的 ubuntu-24.04 在实际使用中不会被触发(因为 exclude 已排除所有 ubuntu 变体,matrix.os 始终有值),但语义上有误导性。建议移除 fallback 或添加注释说明。

@zhoujh01 zhoujh01 force-pushed the codex-ci-macos14-support branch from 445d5ad to 3cdbefd Compare March 10, 2026 15:20
@qin-ctx qin-ctx merged commit 61d8c53 into main Mar 10, 2026
31 checks passed
@qin-ctx qin-ctx deleted the codex-ci-macos14-support branch March 10, 2026 15:25
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants