Composite GHA action with caching#10026
Conversation
LinuxJedi
force-pushed
the
actions-composite-caching
branch
from
499bd8a to
3eb4eb5
Compare
LinuxJedi
force-pushed
the
actions-composite-caching
branch
from
3eb4eb5 to
b2d28ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uses: actions/checkout@v4 | ||
| with: | ||
| sparse-checkout: .github/actions | ||
| depth: 1 |
There was a problem hiding this comment.
actions/checkout@v4 does not support an input named depth; the correct input is fetch-depth. As written, this will likely perform the default (non-shallow) checkout, which is slower and undermines the intent of adding a lightweight checkout for .github/actions. Replace depth: 1 with fetch-depth: 1 (and apply the same change to the other workflows that introduced depth).
| depth: 1 | |
| fetch-depth: 1 |
| for i in 1 2 3; do | ||
| sudo apt-get update && sudo apt-get install -y linux-headers-$(uname -r) && break | ||
| echo "::warning::apt-get failed (attempt $i/3), retrying..." | ||
| sleep $((5 * i)) | ||
| done || $(exit 2) |
There was a problem hiding this comment.
|| $(exit 2) is invalid/unsafe shell usage (command substitution) and will not reliably terminate the step as intended. Use || exit 2 (or check success explicitly after the loop) so the workflow fails correctly when all retries are exhausted.
| uses: ./wolfssl/.github/actions/install-apt-deps | ||
| with: | ||
| packages: libpcap0.8 libpcap-dev curl libcurl4-openssl-dev libnl-3-dev binutils-dev libssl-dev libiberty-dev libnl-genl-3-dev libnl-route-3-dev libdbus-1-dev bridge-utils tshark python3-pycryptodome |
There was a problem hiding this comment.
This introduces a hard dependency on the local path ./wolfssl/.github/actions/install-apt-deps existing at runtime. In this job, the workflow previously did not require any repo checkout prior to installing deps, so this path may not exist and will cause the job to fail with “action not found”. Prefer checking out .github/actions into the workspace (as other workflows do) and referencing the action as ./.github/actions/install-apt-deps, or otherwise ensure ./wolfssl is present before this step.
| packages: build-essential autoconf automake libtool jq psmisc | ||
|
|
There was a problem hiding this comment.
The previous implementation intentionally fell back to installing without psmisc if that package install failed. With the composite action, the step will now fail hard if psmisc is unavailable or temporarily uninstallable on the runner image. If that fallback is still needed, consider (a) restoring the conditional fallback logic here, or (b) extending install-apt-deps to support optional packages (e.g., an optional-packages input) so the workflow keeps its prior resilience.
| packages: build-essential autoconf automake libtool jq psmisc | |
| packages: build-essential autoconf automake libtool jq | |
| - name: Install optional psmisc | |
| continue-on-error: true | |
| run: | | |
| sudo apt-get update | |
| sudo apt-get install -y psmisc |
| SORTED_PKGS=$(echo "${{ inputs.packages }}" | tr ' ' '\n' | sort | tr '\n' ' ') | ||
| PKG_HASH=$(echo "$SORTED_PKGS" | sha256sum | cut -d' ' -f1 | head -c 16) |
There was a problem hiding this comment.
The action expands ${{ inputs.packages }} unquoted into a shell command, which is fragile and can become a shell-injection vector if the input is ever derived from untrusted data (or even just contains unexpected whitespace/newlines). It also reduces cache hit-rate when callers include duplicate packages because the cache key is based on raw sorted tokens. Consider parsing packages into a bash array safely (e.g., read into an array and pass as \"${pkgs[@]}\"), and normalize the cache-key input with sort -u to dedupe packages so semantically-identical lists share a cache.
| fi | ||
| for i in $(seq 1 $RETRIES); do | ||
| if sudo apt-get update -q && \ | ||
| sudo apt-get install -y $NO_REC ${{ inputs.packages }}; then |
There was a problem hiding this comment.
The action expands ${{ inputs.packages }} unquoted into a shell command, which is fragile and can become a shell-injection vector if the input is ever derived from untrusted data (or even just contains unexpected whitespace/newlines). It also reduces cache hit-rate when callers include duplicate packages because the cache key is based on raw sorted tokens. Consider parsing packages into a bash array safely (e.g., read into an array and pass as \"${pkgs[@]}\"), and normalize the cache-key input with sort -u to dedupe packages so semantically-identical lists share a cache.
This adds caching for apt and should make things a bit more stable and faster.
LinuxJedi
force-pushed
the
actions-composite-caching
branch
from
b2d28ab to
410cad2
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10026
No scan targets match the changed files in this PR. Review skipped.
This adds caching for apt and should make things a bit more stable and faster.