alibaba / alibaba/zvec

CI reliability hardening: pin tooling, improve clang-tidy cache keys, and preserve failure diagnostics

Open
#549 0 comments 1 reaction 1 assignee Assigned to @Cuiyus View on GitHub
Dominant language
C++
Stars
15.9k
Forks
998
Avg merge
4d 7h
Merged PRs (30d)
34

Description

## Summary

While reviewing `.github/workflows/01-ci-pipeline.yml` and its reusable workflows, I found several CI reliability risks that can cause flaky or hard-to-diagnose failures even when the source change is valid.

This is a proposed hardening pass rather than a claim that every item is currently failing.

## Findings

### 1. Unpinned Python build/test tools

`03-macos-linux-build.yml` pins some tools but leaves these floating:

```text
pytest
pytest-xdist
scikit-build-core
setuptools_scm
```

A new upstream release can change behavior without a repository change.

**Suggested fix:** pin tested ranges or install from a lock/constraints file shared by all runners.

### 2. Brittle exact Ubuntu package version

`clang_tidy.yml` installs:

```text
clang-tidy=1:18.0-59~exp2
```

This exact archive version can disappear or differ across runner-image refreshes.

**Suggested fix:** install `clang-tidy-18`, verify `clang-tidy --version`, or use an LLVM setup action with an explicitly supported major version.

### 3. Generated-header cache key is missing toolchain/config inputs

The cache key includes third-party CMake/proto hashes and submodule commits, but not all inputs that can affect generated artifacts, such as:

- compiler identity/version;
- CMake version;
- relevant root CMake configuration;
- workflow/schema version.

A cache hit can therefore restore incompatible generated state.

**Suggested fix:** add compiler/CMake fingerprints and a manual cache-schema version to the key, or cache only immutable downloaded dependencies rather than generated build output.

### 4. Changed-file handling is unsafe for unusual filenames

The clang-tidy workflow converts changed files through whitespace-separated shell strings and `xargs`. Paths containing spaces, quotes, or newlines can be split incorrectly.

**Suggested fix:** use NUL-delimited paths throughout (`-z`, `xargs -0`) or pass a JSON array from the changed-files action into Python and launch subprocesses directly.

### 5. Failure diagnostics are lost after the job ends

clang-tidy logs are printed in groups, but build/test workflows do not upload structured logs, `compile_commands.json`, CMake logs, or test reports on failure.

**Suggested fix:** add `if: failure()` artifact uploads for:

- `build/compile_commands.json`;
- CMake configure logs;
- clang-tidy logs;
- pytest JUnit XML;
- CTest/JUnit output;
- runner/toolchain version summary.

### 6. No explicit timeouts

Long CMake builds, package installs, tests, or hung subprocesses can consume the full default job limit.

**Suggested fix:** add job- or step-level `timeout-minutes` and terminate child processes cleanly.

### 7. Repeated `apt-get update`

Linux build jobs run `apt-get update` separately for Clang and AIO installation.

**Suggested fix:** consolidate Linux package installation into one step to reduce network flakiness and runtime.

### 8. Main-branch coverage is intentionally narrow but should be explicit

On pushes to `main`, most platform jobs are skipped and only Linux x64 runs. If this is a cost-control decision, it would help to pair it with a scheduled full matrix and document that policy in the workflow.

## Proposed validation

After the hardening patch:

1. run a PR that changes C++, Python, CMake, and docs separately;
2. test a cache hit and cache miss;
3. intentionally trigger clang-tidy, compile, and pytest failures;
4. verify diagnostic artifacts are available;
5. run the full matrix on `workflow_dispatch` and schedule;
6. confirm main pushes still follow the intended cost policy.

I can help translate this into a patch if maintainers confirm the preferred pinning and full-matrix policy.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.