Skip to content

Commit 9cf830c

Browse files
leodidoona-agent
andcommitted
feat: add context support to getGitCommitTimestamp
Add context parameter to getGitCommitTimestamp() to support cancellation: - Use exec.CommandContext instead of exec.Command - Allows git command to be terminated if build is cancelled - Prevents orphaned git processes - Respects build timeouts - Follows standard Go pattern for cancellable operations Also add test for context cancellation behavior. Addresses review feedback from Cornelius on PR #281. Co-authored-by: Ona <no-reply@ona.com>
1 parent e674c45 commit 9cf830c

File tree

2 files changed

+27
-8
lines changed

2 files changed

+27
-8
lines changed

pkg/leeway/sbom.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,9 @@ func GetSBOMParallelism(sbomConfig WorkspaceSBOM) int {
102102
return runtime.NumCPU()
103103
}
104104

105-
// getGitCommitTimestamp returns the timestamp of the git commit
106-
func getGitCommitTimestamp(commit string) (time.Time, error) {
105+
// getGitCommitTimestamp returns the timestamp of the git commit.
106+
// The context allows for cancellation of the git command if the build is cancelled.
107+
func getGitCommitTimestamp(ctx context.Context, commit string) (time.Time, error) {
107108
// Try SOURCE_DATE_EPOCH first (for reproducible builds)
108109
if epoch := os.Getenv("SOURCE_DATE_EPOCH"); epoch != "" {
109110
timestamp, err := strconv.ParseInt(epoch, 10, 64)
@@ -114,8 +115,8 @@ func getGitCommitTimestamp(commit string) (time.Time, error) {
114115
log.WithError(err).WithField("SOURCE_DATE_EPOCH", epoch).Warn("Invalid SOURCE_DATE_EPOCH, falling back to git commit timestamp")
115116
}
116117

117-
// Get commit timestamp from git
118-
cmd := exec.Command("git", "show", "-s", "--format=%ct", commit)
118+
// Get commit timestamp from git with context support for cancellation
119+
cmd := exec.CommandContext(ctx, "git", "show", "-s", "--format=%ct", commit)
119120
output, err := cmd.Output()
120121
if err != nil {
121122
return time.Time{}, fmt.Errorf("failed to get commit timestamp: %w", err)
@@ -316,7 +317,7 @@ func writeSBOM(buildctx *buildContext, p *Package, builddir string) (err error)
316317
}
317318

318319
// Normalize SBOMs after generation
319-
timestamp, err := getGitCommitTimestamp(p.C.Git().Commit)
320+
timestamp, err := getGitCommitTimestamp(context.Background(), p.C.Git().Commit)
320321
if err != nil {
321322
return fmt.Errorf("failed to get deterministic timestamp for SBOM normalization (commit: %s): %w. "+
322323
"Ensure git is available and the repository is not a shallow clone, or set SOURCE_DATE_EPOCH environment variable",

pkg/leeway/sbom_normalize_test.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package leeway
22

33
import (
4+
"context"
45
"encoding/json"
56
"os"
67
"path/filepath"
@@ -110,7 +111,7 @@ func TestGetGitCommitTimestamp_SourceDateEpoch(t *testing.T) {
110111
}
111112

112113
// Use HEAD as commit (should exist in test environment)
113-
timestamp, err := getGitCommitTimestamp("HEAD")
114+
timestamp, err := getGitCommitTimestamp(context.Background(), "HEAD")
114115

115116
if tt.wantErr && err == nil {
116117
t.Error("expected error, got nil")
@@ -130,7 +131,8 @@ func TestGetGitCommitTimestamp_SourceDateEpoch(t *testing.T) {
130131
func TestGetGitCommitTimestamp_GitCommit(t *testing.T) {
131132
// This test requires being in a git repository
132133
// Use HEAD as a known commit
133-
timestamp, err := getGitCommitTimestamp("HEAD")
134+
ctx := context.Background()
135+
timestamp, err := getGitCommitTimestamp(ctx, "HEAD")
134136
if err != nil {
135137
t.Skipf("skipping test: not in a git repository or git not available: %v", err)
136138
}
@@ -146,7 +148,7 @@ func TestGetGitCommitTimestamp_GitCommit(t *testing.T) {
146148
}
147149

148150
// Verify deterministic: calling twice should return same result
149-
timestamp2, err := getGitCommitTimestamp("HEAD")
151+
timestamp2, err := getGitCommitTimestamp(ctx, "HEAD")
150152
if err != nil {
151153
t.Fatalf("second call failed: %v", err)
152154
}
@@ -155,6 +157,22 @@ func TestGetGitCommitTimestamp_GitCommit(t *testing.T) {
155157
}
156158
}
157159

160+
func TestGetGitCommitTimestamp_ContextCancellation(t *testing.T) {
161+
// Create a cancelled context
162+
ctx, cancel := context.WithCancel(context.Background())
163+
cancel() // Cancel immediately
164+
165+
_, err := getGitCommitTimestamp(ctx, "HEAD")
166+
if err == nil {
167+
t.Error("expected error with cancelled context, got nil")
168+
}
169+
170+
// Verify the error is related to context cancellation
171+
if !contains(err.Error(), "context canceled") && !contains(err.Error(), "failed to get commit timestamp") {
172+
t.Errorf("expected context cancellation error, got: %v", err)
173+
}
174+
}
175+
158176
func TestNormalizeCycloneDX(t *testing.T) {
159177
// Create a temporary directory for test files
160178
tmpDir := t.TempDir()

0 commit comments

Comments
 (0)