Skip to content

build: skip sscache action on non-main branches#61790

Open
joyeecheung wants to merge 1 commit intonodejs:mainfrom
joyeecheung:skip-non-main
Open

build: skip sscache action on non-main branches#61790
joyeecheung wants to merge 1 commit intonodejs:mainfrom
joyeecheung:skip-non-main

Conversation

@joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Feb 12, 2026

To reduce cache thrashing.

From a look at the action results, PRs targeting non-main generally don't have a good cache hit rate anyway, so it seems better to just keep it to main where the majority of the volume is:

https://github.com/nodejs/node/actions/runs/21866462915
https://github.com/nodejs/node/actions/runs/21866461713

Refs: #61436

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Feb 12, 2026
@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.87%. Comparing base (4a13a62) to head (3698f29).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61790      +/-   ##
==========================================
- Coverage   89.76%   88.87%   -0.89%     
==========================================
  Files         675      675              
  Lines      204674   204674              
  Branches    39330    39245      -85     
==========================================
- Hits       183716   181901    -1815     
- Misses      13235    14994    +1759     
- Partials     7723     7779      +56     

see 99 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@targos targos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 12, 2026
Comment on lines +105 to +106
CC: ${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'sccache clang-19' || 'clang-19' }}
CXX: ${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'sccache clang++-19' || 'clang++-19' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid the repetition?

Suggested change
CC: ${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'sccache clang-19' || 'clang-19' }}
CXX: ${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'sccache clang++-19' || 'clang++-19' }}
CC: ${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'sccache' || '' }} clang-19
CXX: ${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'sccache' || '' }} clang++-19

${{ (github.base_ref == 'main' || github.ref_name == 'main') && '--keep SCCACHE_GHA_VERSION --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN' || '' }} \
--arg loadJSBuiltinsDynamically false \
--arg ccache '(import <nixpkgs> {}).sccache' \
${{ (github.base_ref == 'main' || github.ref_name == 'main') && '--arg ccache ''(import <nixpkgs> {}).sccache''' || '' }} \
Copy link
Contributor

@aduh95 aduh95 Feb 13, 2026

Choose a reason for hiding this comment

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

We should set it to null otherwise it defaults to ccache (which would be pure overhead on a ephemeral runner)

Suggested change
${{ (github.base_ref == 'main' || github.ref_name == 'main') && '--arg ccache ''(import <nixpkgs> {}).sccache''' || '' }} \
--arg ccache '${{ (github.base_ref == 'main' || github.ref_name == 'main') && 'import <nixpkgs> {}).sccache' || 'null' }}' \

Alternatively, we could define it in the Configure sccache step above, and fallback to null if it was skipped

Suggested change
${{ (github.base_ref == 'main' || github.ref_name == 'main') && '--arg ccache ''(import <nixpkgs> {}).sccache''' || '' }} \
--arg ccache "${NIX_SCCACHE:-null}"

-I nixpkgs=./tools/nix/pkgs.nix \
--pure --keep TAR_DIR --keep FLAKY_TESTS \
--keep SCCACHE_GHA_VERSION --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN \
${{ (github.base_ref == 'main' || github.ref_name == 'main') && '--keep SCCACHE_GHA_VERSION --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN' || '' }} \
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would simplify this, it's no big deal if we pass not-defined variables

Suggested change
${{ (github.base_ref == 'main' || github.ref_name == 'main') && '--keep SCCACHE_GHA_VERSION --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN' || '' }} \
--keep SCCACHE_GHA_VERSION --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN \

@aduh95 aduh95 added lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x meta Issues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants