ROX-33792: optimize git checkout with shallow clones#19615
Open
ROX-33792: optimize git checkout with shallow clones#19615
Conversation
Pre-compute build tag and short commit in define-job-matrix (with full history), then pass to downstream jobs via BUILD_TAG and SHORTCOMMIT environment variables. This allows downstream jobs to use shallow clones while maintaining correct version information. Changes: - define-job-matrix: Add fetch-depth: 0, compute build-tag and short-commit outputs - Updated jobs to use BUILD_TAG and SHORTCOMMIT env vars: - pre-build-go-binaries - build-and-push-main - push-main-manifests - build-and-push-operator - push-operator-manifests - scan-images-with-roxctl - Removed fetch-depth: 0 from all downstream jobs (now use default shallow clone) Benefits: - Only define-job-matrix needs full history (1× fetch) - All other jobs use shallow clones (default fetch-depth: 1) - Estimated savings: ~12s × 4-8 jobs = 48-96s per workflow run Technical details: - Makefiles respect BUILD_TAG and SHORTCOMMIT env vars (see make/env.mk) - status.sh now gets version info from environment instead of git - Prerelease/rcd jobs still override BUILD_TAG as needed - Backwards compatible: local builds still work (fallback to git) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since BUILD_TAG and SHORTCOMMIT are now computed once and exported via job outputs, consider centralizing their propagation (e.g., via a reusable job or a shared
envat the workflow level where possible) to avoid repetitive per-jobenvblocks and reduce the risk of inconsistencies in future edits. - In the vulnerability scan step you set BUILD_TAG/SHORTCOMMIT from the matrix outputs but still recompute
release_tagviamake tag; if the intent is to rely on the precomputed value, you could use$BUILD_TAGdirectly and avoid an extramake/git call there.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since BUILD_TAG and SHORTCOMMIT are now computed once and exported via job outputs, consider centralizing their propagation (e.g., via a reusable job or a shared `env` at the workflow level where possible) to avoid repetitive per-job `env` blocks and reduce the risk of inconsistencies in future edits.
- In the vulnerability scan step you set BUILD_TAG/SHORTCOMMIT from the matrix outputs but still recompute `release_tag` via `make tag`; if the intent is to rely on the precomputed value, you could use `$BUILD_TAG` directly and avoid an extra `make`/git call there.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Images are ready for the commit at 46cb0e3. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19615 +/- ##
==========================================
- Coverage 49.28% 49.28% -0.01%
==========================================
Files 2735 2735
Lines 206215 206215
==========================================
- Hits 101632 101625 -7
- Misses 97041 97047 +6
- Partials 7542 7543 +1
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:
|
Fixes test failures where development builds were incorrectly compiled as release builds. Root cause: - make/env.mk checks if BUILD_TAG is set in CI to determine release builds - Our shallow clone optimization now sets BUILD_TAG for ALL builds - This caused ALL builds to be compiled with GOTAGS=release Solution: - Check if BUILD_TAG contains '.x' (development tag marker) - Only set GOTAGS=release for clean version tags without '.x' Examples: - 4.7.x-36-gc2253ef650 → development build (GOTAGS empty) - 4.6.0 → release build (GOTAGS=release) - 4.11.x-nightly-20260325 → development build (GOTAGS empty) This fixes E2E test failures: - TestMetadataIsSetCorrectly expecting buildFlavor=development - TestFeatureFlagSettings expecting development defaults AI-assisted debugging and fix.
Contributor
|
/retest |
davdhacs
approved these changes
Mar 25, 2026
Contributor
davdhacs
left a comment
There was a problem hiding this comment.
+1 with or without SHORTCOMMIT :)
| - name: Compute short commit | ||
| id: short-commit | ||
| run: | | ||
| SHORTCOMMIT=$(make --quiet --no-print-directory shortcommit) |
Contributor
There was a problem hiding this comment.
It looks like SHORTCOMMIT is only used by main-build-dockerized. Can it be removed from this workflow?
| # Use a release go -tag when CI is targeting a release tag (not development tags with .x) | ||
| ifdef CI | ||
| ifneq ($(BUILD_TAG),) | ||
| ifeq ($(findstring .x,$(BUILD_TAG)),) |
Contributor
There was a problem hiding this comment.
Is there another way to avoid having this in the release execution path? It looks good, but it feels like there may be a way to separate it fully like if there is another variable in addition to BUILD_TAG that can signify a non-release CI build (or we could add one?)
Contributor
There was a problem hiding this comment.
I tried the following, but I think your ".x" check reads better:
Suggested change
| ifeq ($(findstring .x,$(BUILD_TAG)),) | |
| # Preserve existing GOTAGS and append release tags | |
| GOTAGS := $(if $(GOTAGS),$(GOTAGS)$(comma))$(RELEASE_GOTAGS) | |
| else ifneq ($(DEV_BUILD_TAG),) | |
| BUILD_TAG := $(DEV_BUILD_TAG) |
Contributor
Author
There was a problem hiding this comment.
So there is TAG in line 47 but it's clear on purpose to prevent injection from env.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Pre-compute build tag and short commit in define-job-matrix (with full history), then pass to downstream jobs via
BUILD_TAGandSHORTCOMMITenvironment variables. This allows downstream jobs to use shallow clones while maintaining correct version information.Benefits:
Technical details:
User-facing documentation
Testing and quality
Automated testing
How I validated my change
CI