Use ifneq instead of ifndef for SKIP_DEBUG_BINARIES check#8039
Conversation
Add SKIP_DEBUG_BINARIES environment variable that can be set to skip building debug versions of binaries. Set this variable only for pull_request events in the Build Binaries workflow to save ~4+ minutes per architecture. Debug builds still run on main branch, merge groups, and during release workflows. Co-authored-by: yurishkuro <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce CI time by skipping the “debug” variant of binary builds (built with -gcflags="all=-N -l") during pull_request workflows, while keeping debug builds enabled for main branch and release contexts.
Changes:
- Add a Makefile guard to skip invoking the debug-binary build when
SKIP_DEBUG_BINARIESis set. - Set
SKIP_DEBUG_BINARIESin theci-build-binaries.ymlworkflow for PR-triggered runs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/makefiles/BuildBinaries.mk | Adds a conditional around the recursive debug-binary build invocation. |
| .github/workflows/ci-build-binaries.yml | Exports SKIP_DEBUG_BINARIES during the binary build step based on the event type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8039 +/- ##
=======================================
Coverage 95.47% 95.47%
=======================================
Files 316 316
Lines 16756 16756
=======================================
Hits 15997 15997
Misses 593 593
Partials 166 166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Change from `ifndef SKIP_DEBUG_BINARIES` to `ifneq ($(SKIP_DEBUG_BINARIES),1)` to make the check more explicit. This clearly shows we're checking if the variable equals "1" rather than relying on the subtle behavior of ifndef with empty strings. Both work the same way, but ifneq is more readable. Co-authored-by: yurishkuro <[email protected]>
| env: | ||
| # Skip debug binaries on PRs to save CI time (4+ min per arch) | ||
| # Debug binaries are still built on main branch and during releases | ||
| SKIP_DEBUG_BINARIES: ${{ github.event_name == 'pull_request' && '1' || '' }} |
There was a problem hiding this comment.
let's set to 0 explicitly instead of blank
| SKIP_DEBUG_BINARIES: ${{ github.event_name == 'pull_request' && '1' || '' }} | |
| SKIP_DEBUG_BINARIES: ${{ github.event_name == 'pull_request' && '1' || '0' }} |
There was a problem hiding this comment.
Done in 5bdce54. Changed to explicitly use '0' instead of empty string for non-PR events.
| # invoke make recursively such that DEBUG_BINARY=1 can take effect | ||
| # skip debug builds if SKIP_DEBUG_BINARIES is set to 1 (e.g., during PRs to save CI time) | ||
| ifneq ($(SKIP_DEBUG_BINARIES),1) | ||
| $(MAKE) _build-platform-binaries-debug GOOS=$(GOOS) GOARCH=$(GOARCH) DEBUG_BINARY=1 |
There was a problem hiding this comment.
shouldn't DEBUG_BINARY=1 flag be set on L161 instead, e.g.
_build-platform-binaries-debug: DEBUG_BINARY=1
_build-platform-binaries-debug: \
build-jaeger \
build-remote-storage
? Otherwise if someone manually runs make _build-platform-binaries-debug they will not actually get debug builds.
There was a problem hiding this comment.
Done in 5bdce54. Moved DEBUG_BINARY=1 to the target definition line. Now manual runs of make _build-platform-binaries-debug will also get debug builds.
…arget 1. Set SKIP_DEBUG_BINARIES to '0' instead of '' for non-PR events 2. Move DEBUG_BINARY=1 to target definition instead of invocation This ensures manual runs of `make _build-platform-binaries-debug` also get debug builds, and makes the workflow logic more explicit. Co-authored-by: yurishkuro <[email protected]>
…ing#8039) ## Changes Addressed review feedback: 1. **Set SKIP_DEBUG_BINARIES to '0' explicitly** (instead of empty string) - Changed: `${{ github.event_name == 'pull_request' && '1' || '' }}` - To: `${{ github.event_name == 'pull_request' && '1' || '0' }}` 2. **Move DEBUG_BINARY=1 to target definition** - Added `_build-platform-binaries-debug: DEBUG_BINARY=1` on the target line - Removed `DEBUG_BINARY=1` from the make invocation - This ensures manual runs of `make _build-platform-binaries-debug` also get debug builds ## Behavior All scenarios tested and working: - ✅ SKIP_DEBUG_BINARIES not set → debug builds run with DEBUG_BINARY=1 - ✅ SKIP_DEBUG_BINARIES=0 → debug builds run with DEBUG_BINARY=1 - ✅ SKIP_DEBUG_BINARIES=1 → debug builds skipped - ✅ Manual `make _build-platform-binaries-debug` → debug builds run with DEBUG_BINARY=1 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: yurishkuro <[email protected]>
Changes
Addressed review feedback:
Set SKIP_DEBUG_BINARIES to '0' explicitly (instead of empty string)
${{ github.event_name == 'pull_request' && '1' || '' }}${{ github.event_name == 'pull_request' && '1' || '0' }}Move DEBUG_BINARY=1 to target definition
_build-platform-binaries-debug: DEBUG_BINARY=1on the target lineDEBUG_BINARY=1from the make invocationmake _build-platform-binaries-debugalso get debug buildsBehavior
All scenarios tested and working:
make _build-platform-binaries-debug→ debug builds run with DEBUG_BINARY=1💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.