Spoorcc/issue820 (Fix caching during build)#860
Conversation
WalkthroughThe build workflow is updated to implement platform-specific caching strategies. Ccache operations now exclude Windows runners, while a new clcache caching step is added for Windows. Environment variables are adjusted to use workspace-based cache directories instead of home-based paths across both platforms. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (push)
participant GH as GitHub Actions
participant Runner as Runner (linux/mac/windows)
rect rgba(135,206,235,0.12)
note over GH,Runner: Build workflow starts
end
Dev->>GH: push
GH->>Runner: select runner (os)
alt Non-Windows runner
Runner->>Runner: Setup ccache\n(actions/cache restore)
Runner->>Runner: Export NUITKA_CACHE_DIR_CCACHE -> workspace/.ccache
Runner->>Runner: Run build with ccache
else Windows runner
Runner->>Runner: Setup clcache\n(actions/cache restore .clcache)
Runner->>Runner: Export NUITKA_CACHE_DIR_CLCACHE -> workspace/.clcache
Runner->>Runner: Skip ccache step
Runner->>Runner: Run build with clcache
end
Runner->>GH: upload artifacts/logs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
03e71a5 to
4942f6d
Compare
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
f2cd46b to
6c5805c
Compare
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
6c5805c to
2fce055
Compare
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
2fce055 to
bb025cf
Compare
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
bb025cf to
235a833
Compare
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
235a833 to
3922588
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
54-58: Inconsistent path separators across platform-specific cache env vars.Lines 56 and 57 use different path separators: line 56 uses forward slash (ccache for Unix), while line 57 uses backslash (clcache for Windows). While each may work in their respective contexts, standardizing to forward slashes is more portable and consistent, as forward slashes work across platforms in most modern tools and GitHub Actions contexts.
CCACHE_BASEDIR: ${{ github.workspace }} CCACHE_NOHASHDIR: true NUITKA_CACHE_DIR_CCACHE: ${{ github.workspace }}/.ccache - NUITKA_CACHE_DIR_CLCACHE: ${{ github.workspace }}\.clcache + NUITKA_CACHE_DIR_CLCACHE: ${{ github.workspace }}/.clcacheAlso update line 49 for consistency:
with: - path: ${{ github.workspace }}\.clcache + path: ${{ github.workspace }}/.clcache key: ${{ github.job }}-${{ matrix.platform }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/build.yml
47-47: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (13)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test (ubuntu-latest, 3.10)
- GitHub Check: test (windows-latest, 3.13)
- GitHub Check: test (windows-latest, 3.11)
- GitHub Check: test (windows-latest, 3.9)
- GitHub Check: test (macos-latest, 3.12)
- GitHub Check: test (macos-latest, 3.9)
- GitHub Check: test (macos-latest, 3.11)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: test-cygwin
- GitHub Check: build (ubuntu-latest)
- GitHub Check: DevContainer Build & Test
🔇 Additional comments (1)
.github/workflows/build.yml (1)
37-43: LGTM — ccache step correctly gated to non-Windows platforms.The platform-specific condition makes sense, and
create-symlink: trueis appropriate here since the step is already Windows-excluded.
spoorcc
force-pushed
the
spoorcc/issue820
branch
from
3922588 to
05f81ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
56-57: Standardize path separators for consistency.Line 56 uses forward slashes (
/.ccache) while line 57 uses backslashes (\.clcache). Although both will functionally work (modern tools support forward slashes even on Windows), using forward slashes consistently across all paths improves portability and clarity.NUITKA_CACHE_DIR_CCACHE: ${{ github.workspace }}/.ccache - NUITKA_CACHE_DIR_CLCACHE: ${{ github.workspace }}\.clcache + NUITKA_CACHE_DIR_CLCACHE: ${{ github.workspace }}/.clcache
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml(1 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). (18)
- GitHub Check: Publish Python distribution 📦 to TestPyPI
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test (windows-latest, 3.14)
- GitHub Check: test (windows-latest, 3.13)
- GitHub Check: test (macos-latest, 3.13)
- GitHub Check: test (windows-latest, 3.11)
- GitHub Check: test (windows-latest, 3.9)
- GitHub Check: test (windows-latest, 3.12)
- GitHub Check: test (macos-latest, 3.9)
- GitHub Check: test (macos-latest, 3.12)
- GitHub Check: test (windows-latest, 3.10)
- GitHub Check: test-cygwin
- GitHub Check: test (macos-latest, 3.10)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: test
- GitHub Check: DevContainer Build & Test
🔇 Additional comments (2)
.github/workflows/build.yml (2)
37-50: Previous review feedback has been addressed: actions/cache updated to v4.3.0.The outdated
actions/cache@v3has been replaced withactions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0, which is the latest stable version. The conditional gating for ccache (non-Windows) and new clcache setup (Windows-only) is logically sound.
53-61: No issues found—configuration is correct.The environment variables
NUITKA_CACHE_DIR_CCACHEandNUITKA_CACHE_DIR_CLCACHEare the correct Nuitka configuration variable names, and this configuration pattern is recommended for CI environments where home directories aren't persistent. The workspace-relative paths are properly expanded by GitHub Actions at runtime, and setting both variables simultaneously is standard practice in CI contexts—each variable is used by Nuitka depending on the compiler toolchain in use (ccache for gcc/Linux or clcache for MSVC/Windows).
1 participant
Investigation of #820
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.