Skip to content

Conversation

@PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

  • Introduced a new job cache-sources in the GitHub Actions workflow to cache dependency sources, improving build efficiency.
  • Updated existing jobs to depend on cache-sources to utilize cached data when available.
  • Created a new file cache-depends-sources.yml to define the caching logic, including steps for checking and downloading sources.

This is more important because in DashCoreAutoGuix, because things are less often merged into develop (they are merged into their own feature branch) the shared caches coming from develop become stale and get evicted. If we run it daily, it will ensure the shared cache (in develop) can be used by the runs in the feature branches. It may also be useful here, because currently each run tries to generate it's own cache, and they may race, and the sources used by the different runs are slightly different.

This is normally masked, because the final depends are cached well in this repo.

This change enhances the CI process by reducing redundant downloads and speeding up builds.

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

- Introduced a new job `cache-sources` in the GitHub Actions workflow to cache dependency sources, improving build efficiency.
- Updated existing jobs to depend on `cache-sources` to utilize cached data when available.
- Created a new file `cache-depends-sources.yml` to define the caching logic, including steps for checking and downloading sources.

This change enhances the CI process by reducing redundant downloads and speeding up builds.
@PastaPastaPasta PastaPastaPasta added this to the 23.1 milestone Dec 1, 2025
@github-actions
Copy link

github-actions bot commented Dec 1, 2025

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

Please coordinate with the authors of these PRs to avoid merge conflicts.

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

A new cache-depends-sources workflow was added to restore dependency sources (lookup-only) and run make -C depends download on a cache miss. The primary build workflow (.github/workflows/build.yml) now includes a cache-sources job that runs only when skip is false; several build jobs were changed to depend on both container and cache-sources. build-depends.yml replaces a previous cache step with actions/cache/restore@v4 for restoration and some build jobs pass container-path: ${{ needs.container.outputs.path }} into build-depends.yml.

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions (build.yml)
    participant CS as cache-sources Job
    participant Cache as GitHub Cache
    participant Repo as Repository (depends/)
    participant Build as Build Jobs

    GHA->>CS: start cache-sources (if skip == false)
    activate CS
    CS->>Repo: checkout repo (ref = PR/head)
    CS->>Cache: restore lookup-only (key: hashFiles('depends/packages/*'))
    activate Cache
    alt cache hit
        Cache-->>CS: cache hit (sources available)
    else cache miss
        Cache-->>CS: cache miss
        CS->>Repo: make -C depends download
        Repo-->>CS: sources downloaded
        %% Note: lookup-only means no automatic save in this job
    end
    deactivate Cache
    CS-->>GHA: complete
    deactivate CS

    GHA->>Build: require needs: [container, cache-sources]
    Build->>GHA: run builds (may call build-depends.yml with container-path)
    GHA-->>Build: continue build steps
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify cache-depends-sources.yml triggers (workflow_call, cron) and runner selection.
  • Confirm actions/cache lookup-only vs restore usage across workflows matches intended save/restore behavior.
  • Check cache key/restore-key patterns (hashFiles('depends/packages/*')) for correct invalidation surface.
  • Review updated needs in .github/workflows/build.yml for potential dependency cycles and correct skip gating.
  • Validate container-path: ${{ needs.container.outputs.path }} usage and that needs.container.outputs.path is produced by the container job.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding dependency source caching to the CI workflow.
Description check ✅ Passed The description is clearly related to the changeset, explaining the motivation, implementation details, and expected benefits of the caching improvements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b4b623 and 72eb33d.

📒 Files selected for processing (2)
  • .github/workflows/build-depends.yml (1 hunks)
  • .github/workflows/cache-depends-sources.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/build-depends.yml
  • .github/workflows/cache-depends-sources.yml

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.

UdjinM6
UdjinM6 previously approved these changes Dec 2, 2025
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

@PastaPastaPasta PastaPastaPasta mentioned this pull request Dec 2, 2025
5 tasks
@PastaPastaPasta PastaPastaPasta requested review from knst and kwvg December 2, 2025 14:45
@PastaPastaPasta
Copy link
Member Author

latest working as expected: https://github.com/PastaPastaPasta/dash/actions/runs/19863026549

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 72eb33d

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

utACK 72eb33d

@PastaPastaPasta PastaPastaPasta merged commit b12e32d into dashpay:develop Dec 2, 2025
30 of 34 checks passed
PastaPastaPasta added a commit that referenced this pull request Dec 8, 2025
6f7f799 fix: include depends/Makefile in sparse checkout (pasta)
fd070cc fix: add depends/Makefile to PACKAGES hash (pasta)
36c143b fix: include ci-slim.Dockerfile (pasta)
5c10d85 ci: improve depends caching to be more compact, and bypass quickly if we have a cache hit (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  depends on [another](#7027)

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 6f7f799

Tree-SHA512: b8018aa17d43dc426cba4403ef63800c3af49f2ced0fb0b5be89b0dfb153c337af93a00b55dd1be091dd40efa61513649b15fc2a7b506e0857456273dc4f3708
@PastaPastaPasta PastaPastaPasta deleted the ci/keep-depends-sources-warm branch December 9, 2025 02:51
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.

3 participants