Skip to content

CI: centralize bounded APT installation - #1892

Merged
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
doraemonmj:fix/ci-packaging-apt-hang
Aug 20, 2026
Merged

CI: centralize bounded APT installation#1892
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
doraemonmj:fix/ci-packaging-apt-hang

Conversation

@doraemonmj

@doraemonmj doraemonmj commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Problem

packaging-matrix could remain in Set up C++ compiler until the job's
60-minute limit when an Ubuntu package mirror stalled. The mirror instability
predates this change; #1805 did not create it, but its packaging-tool setup
introduced unbounded apt-get update and apt-get install calls that exposed
the job to it.

Of the OS packages installed there, only the host compiler is required.
ccache is an optimization and the packaging matrix already supports running
without it. ninja can be installed with the Python build dependencies instead
of requiring another system-package install.

Changes

  • Add a shared apt-install composite action for required and optional
    packages. Every update, install, and dpkg recovery operation has a wall-clock
    bound; interrupted state is cleaned up before retrying.
  • Reuse the runner's cached package indexes by default in packaging,
    pre-commit, and no-hardware UT. If a required install fails, refresh the
    system indexes once and retry the install.
  • Refresh only the Ubuntu Toolchain PPA source in setup-gcc-15, preserving
    the runner's existing Ubuntu indexes. The PPA update and required install
    each receive one retry.
  • Treat ccache as optional in packaging: an APT or Homebrew failure emits a
    warning and the matrix continues without compiler caching.
  • Install ninja>=1.11 with the venv and update the packaging documentation and
    verification hint accordingly.
  • Add regression coverage for the shared action and workflow wiring, including
    Bash 3.2 compatibility used by macOS runners.

Bounds

Operation Policy
APT connection 20-second HTTP/HTTPS timeout, no internal APT retry
Regular required install 240 seconds; on failure, one 120-second system-index refresh and one final install
GCC PPA refresh Up to two 60-second attempts, limited to the PPA .sources file
GCC required install Up to two 240-second attempts
dpkg recovery 60 seconds, then a 15-second kill grace period

These are per-operation bounds. No job-level timeout-minutes value is changed.

Testing

  • All pre-commit hooks passed locally and in CI.
  • Focused APT-action regression tests passed on Ubuntu and macOS.
  • Packaging and no-hardware UT passed on both Ubuntu and macOS in
    CI run 32325726639.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b25e04c-9376-44b7-a99b-d7937e02e3cc

📥 Commits

Reviewing files that changed from the base of the PR and between 7449523 and ee6c5d4.

📒 Files selected for processing (10)
  • .github/actions/apt-install/action.yml
  • .github/actions/setup-gcc-15/action.yml
  • .github/workflows/_packaging.yml
  • .github/workflows/_pre-commit.yml
  • .github/workflows/_ut-no-hardware.yml
  • docs/ci.md
  • docs/getting-started.md
  • docs/python-packaging.md
  • tests/ut/py/test_gcc_setup_action.py
  • tools/verify_packaging.sh

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

This change adds a reusable APT installation action with validation, retries, recovery, source isolation, and required or optional package handling. CI workflows use the action for Linux setup. Documentation and packaging checks now require ninja>=1.11.

Changes

APT installation and CI setup

Layer / File(s) Summary
Reusable APT action and validation
.github/actions/apt-install/action.yml, tests/ut/py/test_gcc_setup_action.py
The new action validates inputs, isolates .sources updates, retries bounded APT operations, recovers interrupted installs, and distinguishes required from optional package failures. Tests cover these behaviors and YAML validity.
Workflow package installation
.github/actions/setup-gcc-15/action.yml, .github/workflows/_packaging.yml, .github/workflows/_pre-commit.yml, .github/workflows/_ut-no-hardware.yml, tests/ut/py/test_gcc_setup_action.py
Compiler and Linux CI setup now detect packages separately from installation and invoke the shared action. Optional ccache failures produce warnings. Linux GoogleTest installation uses the action.
Ninja environment requirements
.github/workflows/_packaging.yml, docs/ci.md, docs/getting-started.md, docs/python-packaging.md, tools/verify_packaging.sh
Packaging environments, development instructions, verification instructions, and dependency remediation commands now include ninja>=1.11.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to ee6c5

The PR bounds CI package-manager operations, makes optional tooling non-blocking, and standardizes Ninja installation without any supplied evidence of an actionable merge-blocking risk.

Possibly related PRs

Poem

A rabbit found packages in a row,
With bounded hops through APT flow.
Required ones must safely land,
Optional ones may wave a paw.
Ninja joins the build-time band—
CI now follows one clear plan.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description clearly explains the bounded APT installation changes, optional package handling, Ninja migration, retry policies, and testing.
Title check ✅ Passed The title concisely and accurately summarizes the main change: centralizing bounded APT installation for CI.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ChaoZheng109 ChaoZheng109 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two Should-fix items from a full review of this PR. The diagnosis and the overall approach look right to me — putting the bound on the apt call rather than the workflow step is the judgement that makes the required/optional split possible, the "every apt invocation" claim holds under an exhaustive sweep of .github/ at HEAD, and both packaging jobs are green so the PyPI-ninja path is exercised end to end. These two are the parts I'd want addressed or answered before merge.

Comment thread .github/actions/setup-gcc-15/action.yml Outdated
Comment thread .github/workflows/_packaging.yml
@doraemonmj
doraemonmj force-pushed the fix/ci-packaging-apt-hang branch from 9c03323 to e153ed8 Compare August 19, 2026 08:31
@doraemonmj

doraemonmj commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

已将 APT 稳定性改动更新为最终单提交(8eb4423):

  • 新增共享 apt-install composite action,统一 setup-gcc-15、packaging、pre-commit 和 no-hardware UT;sim、sanitizers、profiling 通过 setup-gcc-15 间接覆盖。
  • 默认直接使用 runner 缓存索引;必需包安装失败时,才执行一次有上限的 system update 并重试。
  • GCC PPA 只刷新隔离的 deb822 source-parts,并设置 APT::Get::List-Cleanup=0,不会删除 runner 已有的 Ubuntu indexes;PPA refresh 和 GCC 安装各允许一次重试。
  • 所有 update、install 和 dpkg recovery 都有 wall-clock timeout;APT 内部重试关闭,HTTP/HTTPS 单连接超时为 20 秒。
  • packaging 只把编译器作为必需系统包;ccache 安装失败仅告警,ninja>=1.11 改由 venv 中的 PyPI 包提供。
  • 修复 macOS 自带 Bash 3.2 在 set -u 下展开空数组导致的 UT 失败,并补充相关回归测试。

验证结果:

  • 本地全部 pre-commit hooks 通过。
  • 相关 APT action UT 在 Ubuntu 本地和 macOS runner 上通过。
  • PR 的 pre-commit、Ubuntu/macOS packaging、Ubuntu/macOS no-hardware UT 均已通过:CI run 32325726639

@doraemonmj
doraemonmj force-pushed the fix/ci-packaging-apt-hang branch 4 times, most recently from 374900b to 8eb4423 Compare August 20, 2026 02:44
- Add a shared action for timed APT retries and recovery.
- Reuse it across packaging, lint, unit-test, and GCC setup jobs.
- Keep update commands compatible with Bash 3.2 nounset handling.
- Document the behavior and cover workflow changes with unit tests.
@doraemonmj doraemonmj changed the title CI: bound every apt invocation and stop packaging blocking on ccache CI: centralize bounded APT installation Aug 20, 2026
@ChaoZheng109
ChaoZheng109 merged commit 4936924 into hw-native-sys:main Aug 20, 2026
19 checks passed
@doraemonmj
doraemonmj deleted the fix/ci-packaging-apt-hang branch August 25, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants