Skip to content

Conversation

@rjarry
Copy link
Collaborator

@rjarry rjarry commented Nov 5, 2025

The previous commit was a bit aggressive. It completely prevents cache from being saved.

Cache hit occurred on the primary key ccache-x86_64-clang-18-refs/heads/main, not saving cache.

Use two separate steps:

  • actions/cache/restore@v4: always enabled, on the main branch, and in pull requests.
  • actions/cache/save@v4: only enabled on the main branch.

Incidentally, it makes the workflow easier to understand.

Fixes: 75ba8ab ("github: prevent pull requests from saving ccache")
Link: https://github.com/DPDK/grout/actions/runs/19098927620/job/54565817905#step:23:2
Link: https://github.com/actions/cache/tree/main/restore
Link: https://github.com/actions/cache/tree/main/save

Summary by CodeRabbit

Release Notes

No user-visible changes

These updates are internal infrastructure modifications to build and caching workflows with no direct impact on product functionality or user experience.

The previous commit was a bit aggressive. It completely prevents cache
from being saved.

> Cache hit occurred on the primary key
> ccache-x86_64-clang-18-refs/heads/main, not saving cache.

Use two separate steps:

- actions/cache/restore@v4: always enabled, on the main branch, and in
  pull requests.
- actions/cache/save@v4: only enabled on the main branch.

Incidentally, it makes the workflow easier to understand.

Fixes: 75ba8ab ("github: prevent pull requests from saving ccache")
Link: https://github.com/DPDK/grout/actions/runs/19098927620/job/54565817905#step:23:2
Link: https://github.com/actions/cache/tree/main/restore
Link: https://github.com/actions/cache/tree/main/save
Signed-off-by: Robin Jarry <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

📝 Walkthrough

Walkthrough

The pull request refactors ccache handling in GitHub Actions workflows. In both .github/workflows/check.yml and .github/workflows/package.yml, the cache strategy is unified by introducing explicit cache steps with id: cache to replace per-job unconditional actions/cache@v4 usage. Cache keys are simplified by removing per-compiler and per-ref components. Corresponding actions/cache/save@v4 steps are added post-build, conditioned on non-PR events. This pattern is applied across build-cross-aarch64, deb, and rpm jobs, standardizing cache restoration and persistence while eliminating the previous unconditional approach.

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: refactoring GitHub Actions cache handling to fix the cache save procedure by splitting restore and save steps.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75ba8ab and cfb1c5e.

📒 Files selected for processing (2)
  • .github/workflows/check.yml (2 hunks)
  • .github/workflows/package.yml (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
  • GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
  • GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
  • GitHub Check: rpm
  • GitHub Check: deb
🔇 Additional comments (4)
.github/workflows/check.yml (1)

108-112: Cache save pattern is correct, but relies on missing permission.

The use of steps.cache.outputs.cache-primary-key is the correct way to maintain consistency between restore and save steps. However, this step will fail without the actions: write permission (see previous comment).

.github/workflows/package.yml (3)

42-46: Cache restore pattern is well-formed.

The explicit cache restore steps with id: cache and simplified keys correctly avoid the per-compiler complexity. The cache-primary-key output is properly referenced in the save steps.

Also applies to: 96-100


58-62: Cache save condition is appropriate.

The condition ${{ ! github.event.pull_request }} correctly skips cache saves in PR contexts (where github.event.pull_request is an object/truthy) and allows saves on workflow_call triggers (where it's null/falsy). The required actions: write permissions are declared at the job level.

Also applies to: 112-116


18-19: Permissions are correctly configured.

Both jobs explicitly declare permissions: { actions: write }, which is required for actions/cache/save@v4 to function.

Also applies to: 71-72


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 and usage tips.

@christophefontaine christophefontaine merged commit 10c0a2b into DPDK:main Nov 5, 2025
12 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