Skip to content

Validate the release build on pull requests#10

Merged
kenvandine merged 2 commits into
lemonadefrom
ci_validate_release_on_pr
May 31, 2026
Merged

Validate the release build on pull requests#10
kenvandine merged 2 commits into
lemonadefrom
ci_validate_release_on_pr

Conversation

@kenvandine

Copy link
Copy Markdown
Member

Motivation

The last several fixes (#6#9) were all packaging issues — a missing OpenMP DLL, a build-path RPATH, a PowerShell quoting bug, a missing CUDA sub-package, a wrong DLL match pattern — that only showed up when the scheduled/dispatched Release workflow ran. Each one cost a full release run to discover. This PR runs the same build/pack jobs on pull requests so these surface before merging.

How it works

  • Adds a pull_request trigger to the Release workflow. The build jobs (windows-cpu, windows-rocm, windows-cuda, ubuntu-22-rocm, ubuntu-22-cuda) run and exercise their Pack artifacts / validation steps.
  • Nothing is published. The release job is already gated on github.event_name == 'schedule' || github.event.inputs.create_release == 'true', so it is skipped on pull_request — no tag, no release, no asset upload.

Keeping PR runs affordable

  • Path-scoped to .github/workflows/release.yml and .github/actions/**. The build jobs clone llama.cpp source from upstream (ggml-org/llama.cpp), so the only PR-controllable inputs to this pipeline are the workflow and its composite action — no point running the full build on unrelated PRs.

  • One CUDA arch on PRs. Both CUDA matrices build a single representative arch (sm_89) on pull_request, since the packaging logic is identical across all sm_*. The full 7-arch matrix still runs on schedule/dispatch:

    sm: ${{ github.event_name == 'pull_request' && fromJSON('["sm_89"]') || fromJSON('["sm_75", "sm_80", "sm_86", "sm_89", "sm_90", "sm_100", "sm_120"]') }}

Note

This PR's own CI run is a live test of the feature — it touches .github/workflows/release.yml, so the path filter matches and the validation jobs should run on this PR.

🤖 Generated with Claude Code

Run the release build/pack jobs on PRs so packaging issues (missing
runtime DLLs, RPATH problems, PowerShell quoting bugs, etc.) surface
before merging instead of only on the scheduled/dispatched release runs.

The existing release job is gated on schedule/create_release, so it is
skipped on pull_request and nothing is published.

To keep PR runs affordable:
- Scope the pull_request trigger to .github/** paths. The build jobs
  clone llama.cpp source from upstream, so only changes to the release
  pipeline itself can affect the outcome.
- Build a single representative CUDA arch (sm_89) on PRs; the packaging
  logic is identical across all sm_*. The full matrix still runs on
  schedule/dispatch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@fl0rianr fl0rianr left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good work, this looks like the right direction for this repo, especially since the recent failures were all in the build/pack jobs that this now exercises.

One small scope mismatch I noticed:.github/actions/**is included in the PR path filter, but get-tag-name is still referenced via lemonade-sdk/llama.cpp/.github/actions/get-tag-name@lemonade, so changes to that composite action would not be tested from the PR head. If the intent is to validate action changes too, this should use the PR-local action checkout; otherwise maybe considering to drop .github/actions/** from the trigger to avoid implying that coverage.
You might want to have a second look on that before merging, that's all. Thanks!

The release workflow consumes the get-tag-name composite action via a
pinned remote ref (lemonade-sdk/llama.cpp/.github/actions/get-tag-name@lemonade),
not a PR-local ./ path. A PR run therefore always uses the action from
the lemonade branch, so triggering on .github/actions/** changes would
run the old action version and falsely imply the change was validated.

Scope the PR trigger to the workflow file itself, which is the only
PR-controllable input that actually affects the outcome.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kenvandine

Copy link
Copy Markdown
Member Author

Good catch, thanks @fl0rianr — you're right, that's a false-coverage trap. All four get-tag-name references use the pinned remote ref @lemonade, not a PR-local ./.github/actions/... path, so a PR run always pulls the action from the base branch regardless of what the PR head changes. Including .github/actions/** in the filter would trigger a (full, ~15 min) release build on action-only PRs while still testing the old action — coverage in name only.

Actually validating the PR-head action would require checking out the lemonade repo (these jobs intentionally check out upstream ggml-org/llama.cpp source) and rewiring all four uses: references to ./..., which is beyond the scope of "validate the release build."

So I went with your fallback and dropped .github/actions/** from the trigger in 8cb5a00 — the PR filter is now just .github/workflows/release.yml, the only PR-controllable input that actually affects a PR run's outcome. Updated the comment to explain why.

@kenvandine kenvandine requested a review from fl0rianr May 31, 2026 18:47

@fl0rianr fl0rianr left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, that addresses my concern. The workflow scope now matches what is actually exercised by the PR run. It's up to you if you want to update the PR description.

@kenvandine kenvandine merged commit 300709a into lemonade May 31, 2026
6 checks passed
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