Skip to content

ci: better depends caching #7029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 4 commits into from
Dec 8, 2025
Merged

ci: better depends caching #7029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5c10d85
ci: improve depends caching to be more compact, and bypass quickly if…
PastaPastaPasta Dec 1, 2025
36c143b
fix: include ci-slim.Dockerfile
PastaPastaPasta Dec 3, 2025
fd070cc
fix: add depends/Makefile to PACKAGES hash
PastaPastaPasta Dec 5, 2025
6f7f799
fix: include depends/Makefile in sparse checkout
PastaPastaPasta Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 61 additions & 33 deletions .github/workflows/build-depends.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,80 +14,108 @@ on:
outputs:
key:
description: "Key needed for restoring depends cache"
value: ${{ jobs.build-depends.outputs.key }}
value: ${{ jobs.check-cache.outputs.cache-key }}
host:
description: "Host triplet for this build target"
value: ${{ jobs.check-cache.outputs.host }}
dep-opts:
description: "DEP_OPTS used to build depends"
value: ${{ jobs.check-cache.outputs.dep-opts }}

jobs:
build-depends:
name: Build depends
runs-on: ubuntu-24.04
check-cache:
name: Check cache
runs-on: ubuntu-latest
outputs:
key: ${{ steps.restore.outputs.cache-primary-key }}
container:
image: ${{ inputs.container-path }}
options: --user root
cache-hit: ${{ steps.cache-check.outputs.cache-hit }}
cache-key: ${{ steps.setup.outputs.cache-key }}
host: ${{ steps.setup.outputs.HOST }}
dep-opts: ${{ steps.setup.outputs.DEP_OPTS }}
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
sparse-checkout: |
ci/dash
ci/test
depends/Makefile
depends/packages
depends/hosts
contrib/containers/ci/ci.Dockerfile
contrib/containers/ci/ci-slim.Dockerfile

- name: Initial setup
- name: Compute cache key
id: setup
run: |
BUILD_TARGET="${{ inputs.build-target }}"
source ./ci/dash/matrix.sh
echo "DEP_OPTS=${DEP_OPTS}" >> "${GITHUB_OUTPUT}"
echo "HOST=${HOST}" >> "${GITHUB_OUTPUT}"
DEP_HASH="$(echo -n "${BUILD_TARGET}" "${DEP_OPTS}" "${HOST}" | sha256sum | head -c 64)"
Copy link

@coderabbitai coderabbitai bot Dec 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential hash collision risk from space-separated concatenation.

The echo -n command at line 55 concatenates BUILD_TARGET, DEP_OPTS, and HOST with spaces between them, which could produce identical strings from different inputs. For example, if BUILD_TARGET contains a space, the boundary between variables becomes ambiguous:

  • BUILD_TARGET="a b" DEP_OPTS="c" → "a b c"
  • BUILD_TARGET="a" DEP_OPTS="b c" → "a b c" (collision)

To ensure deterministic and collision-free hashing, use explicit separators between variables.

Consider using explicit delimiters:

- DEP_HASH="$(echo -n "${BUILD_TARGET}" "${DEP_OPTS}" "${HOST}" | sha256sum | head -c 64)"
+ DEP_HASH="$(echo -n "${BUILD_TARGET}:${DEP_OPTS}:${HOST}" | sha256sum | head -c 64)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
DEP_HASH="$(echo -n "${BUILD_TARGET}" "${DEP_OPTS}" "${HOST}" | sha256sum | head -c 64)"
DEP_HASH="$(echo -n "${BUILD_TARGET}:${DEP_OPTS}:${HOST}" | sha256sum | head -c 64)"
🤖 Prompt for AI Agents 
.github/workflows/build-depends.yml around line 55: the current echo -n
concatenation with spaces can produce ambiguous inputs and hash collisions when
variables contain spaces; change the concatenation to use an unambiguous
separator or fixed-length encoding (for example, use printf to output each
variable with a NUL or other explicit delimiter between them, or prefix each
value with its length) before piping to sha256sum so the resulting hash is
deterministic and collision-free.

Copy link

@UdjinM6 UdjinM6 Dec 6, 2025

Choose a reason for hiding this comment

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

This shouldn't happen because BUILD_TARGET, DEP_OPTS and HOST (in that specific order) can have no common parts I think.

Copy link

@coderabbitai coderabbitai bot Dec 6, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Examine the ci/dash/matrix.sh file to understand possible values for BUILD_TARGET, DEP_OPTS, and HOST
cat ci/dash/matrix.sh

Length of output: 2040

🏁 Script executed:

#!/bin/bash
# Check a few setup files to see what HOST and DEP_OPTS values look like
echo "=== arm-linux setup ==="
grep -E '(HOST=|DEP_OPTS=)' ci/test/00_setup_env_arm.sh | head -5

echo -e "\n=== linux64 setup ==="
grep -E '(HOST=|DEP_OPTS=)' ci/test/00_setup_env_native_qt5.sh | head -5

echo -e "\n=== mac setup ==="
grep -E '(HOST=|DEP_OPTS=)' ci/test/00_setup_env_mac.sh | head -5

echo -e "\n=== win64 setup ==="
grep -E '(HOST=|DEP_OPTS=)' ci/test/00_setup_env_win64.sh | head -5

echo -e "\n=== Check if DEP_OPTS is ever set with spaces in values ==="
rg 'DEP_OPTS=' ci/test/ -A 1

Length of output: 1657

@UdjinM6, you're absolutely right. After examining the actual values from ci/dash/matrix.sh and the setup files:

  • BUILD_TARGET: Fixed set of identifiers like linux64, mac, win64, arm-linux (no spaces)
  • HOST: Standard triplets like x86_64-pc-linux-gnu, x86_64-apple-darwin
  • DEP_OPTS: Build options like NO_UPNP=1 DEBUG=1 or CC=clang-19 CXX='clang++-19 -stdlib=libc++'

These three variables have completely distinct formats that don't overlap. A BUILD_TARGET will never contain values that look like DEP_OPTS, and DEP_OPTS won't look like a HOST triplet. Given these domain constraints, the collision scenario I described isn't practically possible here.

My concern was theoretically valid for arbitrary strings, but your understanding of the actual values in this codebase is correct—no collision can occur. Thanks for the clarification!

🧠 Learnings used 
Learnt from: kwvg
Repo: dashpay/dash PR: 6421
File: .github/workflows/build-depends.yml:80-85
Timestamp: 2025-02-17T10:21:55.551Z
Learning: In GitHub Actions workflow files, when using the `env` command with `DEP_OPTS` or similar variables containing space-delimited key-value pairs for environment variables, word splitting is desired behavior and the variables should not be quoted.

echo "\"${BUILD_TARGET}\" has HOST=\"${HOST}\" and DEP_OPTS=\"${DEP_OPTS}\" with hash \"${DEP_HASH}\""
echo "DEP_HASH=${DEP_HASH}" >> "${GITHUB_OUTPUT}"

DOCKERFILE_HASH="${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/containers/ci/ci-slim.Dockerfile') }}"
PACKAGES_HASH="${{ hashFiles('depends/packages/*', 'depends/Makefile') }}"
CACHE_KEY="depends-${DOCKERFILE_HASH}-${{ inputs.build-target }}-${DEP_HASH}-${PACKAGES_HASH}"
echo "cache-key=${CACHE_KEY}" >> "${GITHUB_OUTPUT}"
echo "Cache key: ${CACHE_KEY}"
shell: bash

- name: Check for cached depends
id: cache-check
uses: actions/cache@v4
with:
path: depends/built/${{ steps.setup.outputs.HOST }}
key: ${{ steps.setup.outputs.cache-key }}
lookup-only: true

build:
name: Build depends
needs: [check-cache]
if: needs.check-cache.outputs.cache-hit != 'true'
runs-on: ubuntu-24.04
container:
image: ${{ inputs.container-path }}
options: --user root
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Restore depends sources
uses: actions/cache/restore@v4
with:
path: |
depends/sources
path: depends/sources
key: depends-sources-${{ hashFiles('depends/packages/*') }}
restore-keys: |
depends-sources-
restore-keys: depends-sources-

- name: Cache SDKs
uses: actions/cache@v4
if: inputs.build-target == 'mac'
with:
path: |
depends/SDKs
path: depends/SDKs
key: depends-sdks-${{ hashFiles('depends/hosts/darwin.mk') }}
restore-keys: |
depends-sdks-
restore-keys: depends-sdks-

- name: Restore cached depends
uses: actions/cache/restore@v4
id: restore
with:
path: |
depends/built
depends/${{ steps.setup.outputs.HOST }}
key: depends-${{ hashFiles('contrib/containers/ci/ci.Dockerfile') }}-${{ inputs.build-target }}-${{ steps.setup.outputs.DEP_HASH }}-${{ hashFiles('depends/packages/*') }}
path: depends/built/${{ needs.check-cache.outputs.host }}
key: ${{ needs.check-cache.outputs.cache-key }}
restore-keys: |
depends-${{ hashFiles('contrib/containers/ci/ci.Dockerfile') }}-${{ inputs.build-target }}-${{ steps.setup.outputs.DEP_HASH }}-
depends-${{ hashFiles('contrib/containers/ci/ci.Dockerfile') }}-${{ inputs.build-target }}-
depends-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/containers/ci/ci-slim.Dockerfile') }}-${{ inputs.build-target }}-

- name: Build depends
run: |
export HOST="${{ steps.setup.outputs.HOST }}"
export HOST="${{ needs.check-cache.outputs.host }}"
if [ "${HOST}" = "x86_64-apple-darwin" ]; then
./contrib/containers/guix/scripts/setup-sdk
fi
env ${{ steps.setup.outputs.DEP_OPTS }} make -j$(nproc) -C depends
env ${{ needs.check-cache.outputs.dep-opts }} make -j$(nproc) -C depends

- name: Save depends cache
uses: actions/cache/save@v4
if: steps.restore.outputs.cache-hit != 'true'
with:
path: |
depends/built
depends/${{ steps.setup.outputs.HOST }}
key: ${{ steps.restore.outputs.cache-primary-key }}
path: depends/built/${{ needs.check-cache.outputs.host }}
key: ${{ needs.check-cache.outputs.cache-key }}
20 changes: 17 additions & 3 deletions .github/workflows/build-src.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ on:
description: "Key needed to access cached depends"
required: true
type: string
depends-host:
description: "Host triplet from depends build"
required: true
type: string
depends-dep-opts:
description: "DEP_OPTS used to build depends"
required: false
type: string
default: ""
outputs:
key:
description: "Key needed for restoring artifacts bundle"
Expand Down Expand Up @@ -59,12 +68,17 @@ jobs:
- name: Restore depends cache
uses: actions/cache/restore@v4
with:
path: |
depends/built
depends/${{ steps.setup.outputs.HOST }}
path: depends/built/${{ inputs.depends-host }}
key: ${{ inputs.depends-key }}
fail-on-cache-miss: true

- name: Rebuild depends prefix
run: |
# Use the HOST and DEP_OPTS from the depends build, not this build-target
# This ensures the build_id matches the cached packages
make -j$(nproc) -C depends HOST="${{ inputs.depends-host }}" ${{ inputs.depends-dep-opts }}
shell: bash

- name: Manage ccache
uses: actions/cache@v4
with:
Expand Down
20 changes: 20 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ jobs:
build-target: arm-linux
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-arm-linux.outputs.key }}
depends-host: ${{ needs.depends-arm-linux.outputs.host }}
depends-dep-opts: ${{ needs.depends-arm-linux.outputs.dep-opts }}

src-linux64:
name: linux64-build
Expand All @@ -148,6 +150,8 @@ jobs:
build-target: linux64
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}

src-linux64_fuzz:
name: linux64_fuzz-build
Expand All @@ -158,6 +162,8 @@ jobs:
build-target: linux64_fuzz
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}

src-linux64_multiprocess:
name: linux64_multiprocess-build
Expand All @@ -168,6 +174,8 @@ jobs:
build-target: linux64_multiprocess
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64_multiprocess.outputs.key }}
depends-host: ${{ needs.depends-linux64_multiprocess.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64_multiprocess.outputs.dep-opts }}

src-linux64_nowallet:
name: linux64_nowallet-build
Expand All @@ -177,6 +185,8 @@ jobs:
build-target: linux64_nowallet
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64_nowallet.outputs.key }}
depends-host: ${{ needs.depends-linux64_nowallet.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64_nowallet.outputs.dep-opts }}

src-linux64_sqlite:
name: linux64_sqlite-build
Expand All @@ -187,6 +197,8 @@ jobs:
build-target: linux64_sqlite
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}

src-linux64_tsan:
name: linux64_tsan-build
Expand All @@ -197,6 +209,8 @@ jobs:
build-target: linux64_tsan
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64_multiprocess.outputs.key }}
depends-host: ${{ needs.depends-linux64_multiprocess.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64_multiprocess.outputs.dep-opts }}

src-linux64_ubsan:
name: linux64_ubsan-build
Expand All @@ -207,6 +221,8 @@ jobs:
build-target: linux64_ubsan
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}

src-mac:
name: mac-build
Expand All @@ -216,6 +232,8 @@ jobs:
build-target: mac
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-mac.outputs.key }}
depends-host: ${{ needs.depends-mac.outputs.host }}
depends-dep-opts: ${{ needs.depends-mac.outputs.dep-opts }}

src-win64:
name: win64-build
Expand All @@ -225,6 +243,8 @@ jobs:
build-target: win64
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-win64.outputs.key }}
depends-host: ${{ needs.depends-win64.outputs.host }}
depends-dep-opts: ${{ needs.depends-win64.outputs.dep-opts }}

test-linux64:
name: linux64-test
Expand Down
Loading