tangled.org
Signed-off-by: Evan Jarrett evan@evanjarrett.com
+1
-1
spindle/engines/nixery/engine.go
+1
-1
spindle/engines/nixery/engine.go
···
109
109
setup := &setupSteps{}
110
110
111
111
setup.addStep(nixConfStep())
112
-
setup.addStep(models.BuildCloneStep(twf, *tpl.TriggerMetadata, workspaceDir, e.cfg.Server.Dev))
112
+
setup.addStep(models.BuildCloneStep(twf, *tpl.TriggerMetadata, e.cfg.Server.Dev))
113
113
// this step could be empty
114
114
if s := dependencyStep(dwf.Dependencies); s != nil {
115
115
setup.addStep(*s)
spindle/engines/nixery/setup_steps.go
spindle/engines/nixery/setup_steps.go
This file has not been changed.
+18
-24
spindle/models/clone.go
+18
-24
spindle/models/clone.go
···
9
9
)
10
10
11
11
type CloneStep struct {
12
-
name string
13
-
kind StepKind
14
-
commands []string
12
+
name string
13
+
kind StepKind
14
+
commands []string
15
15
}
16
16
17
17
func (s CloneStep) Name() string {
···
31
31
}
32
32
33
33
// BuildCloneStep generates git clone commands.
34
-
// The shared builder handles:
34
+
// The caller must ensure the current working directory is set to the desired
35
+
// workspace directory before executing these commands.
36
+
//
37
+
// The generated commands are:
35
38
// - git init
36
39
// - git remote add origin <url>
37
40
// - git fetch --depth=<d> --recurse-submodules=<yes|no> <sha>
38
41
// - git checkout FETCH_HEAD
39
-
// And supports all trigger types (push, PR, manual) and clone options.
40
-
func BuildCloneStep(twf tangled.Pipeline_Workflow, tr tangled.Pipeline_TriggerMetadata, workspaceDir string, dev bool) CloneStep {
42
+
//
43
+
// Supports all trigger types (push, PR, manual) and clone options.
44
+
func BuildCloneStep(twf tangled.Pipeline_Workflow, tr tangled.Pipeline_TriggerMetadata, dev bool) CloneStep {
41
45
if twf.Clone != nil && twf.Clone.Skip {
42
46
return CloneStep{}
43
47
}
44
48
45
49
commitSHA, err := extractCommitSHA(tr)
46
50
if err != nil {
47
-
return CloneStep{
48
-
kind: StepKindSystem,
49
-
name: "Clone repository into workspace (error)",
51
+
return CloneStep{
52
+
kind: StepKindSystem,
53
+
name: "Clone repository into workspace (error)",
50
54
commands: []string{fmt.Sprintf("echo 'Failed to get clone info: %s' && exit 1", err.Error())},
51
55
}
52
56
}
53
57
54
58
repoURL := buildRepoURL(tr, dev)
55
59
56
-
if workspaceDir == "" {
57
-
workspaceDir = "/tangled/workspace"
58
-
}
59
-
60
-
initCmd := fmt.Sprintf("git init %s", workspaceDir)
61
-
remoteCmd := fmt.Sprintf("git remote add origin %s", repoURL)
62
-
63
60
var cloneOpts tangled.Pipeline_CloneOpts
64
61
if twf.Clone != nil {
65
62
cloneOpts = *twf.Clone
66
63
}
67
64
fetchArgs := buildFetchArgs(cloneOpts, commitSHA)
68
-
fetchCmd := fmt.Sprintf("git fetch %s", strings.Join(fetchArgs, " "))
69
-
checkoutCmd := "git checkout FETCH_HEAD"
70
-
65
+
71
66
return CloneStep{
72
67
kind: StepKindSystem,
73
68
name: "Clone repository into workspace",
74
69
commands: []string{
75
-
initCmd,
76
-
fmt.Sprintf("cd %s", workspaceDir),
77
-
remoteCmd,
78
-
fetchCmd,
79
-
checkoutCmd,
70
+
"git init",
71
+
fmt.Sprintf("git remote add origin %s", repoURL),
72
+
fmt.Sprintf("git fetch %s", strings.Join(fetchArgs, " ")),
73
+
"git checkout FETCH_HEAD",
80
74
},
81
75
}
82
76
}
+34
-27
spindle/models/clone_test.go
+34
-27
spindle/models/clone_test.go
···
30
30
},
31
31
}
32
32
33
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
33
+
step := BuildCloneStep(twf, tr, false)
34
34
35
35
if step.Kind() != StepKindSystem {
36
36
t.Errorf("Expected StepKindSystem, got %v", step.Kind())
···
41
41
}
42
42
43
43
commands := step.Commands()
44
-
if len(commands) != 5 {
45
-
t.Errorf("Expected 5 commands, got %d", len(commands))
44
+
if len(commands) != 4 {
45
+
t.Errorf("Expected 4 commands, got %d", len(commands))
46
46
}
47
47
48
48
// Verify commands contain expected git operations
···
89
89
},
90
90
}
91
91
92
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
92
+
step := BuildCloneStep(twf, tr, false)
93
93
94
94
allCmds := strings.Join(step.Commands(), " ")
95
95
if !strings.Contains(allCmds, "pr-sha-789") {
···
116
116
},
117
117
}
118
118
119
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
119
+
step := BuildCloneStep(twf, tr, false)
120
120
121
121
// Manual triggers don't have a SHA yet (TODO), so git fetch won't include a SHA
122
122
allCmds := strings.Join(step.Commands(), " ")
···
147
147
},
148
148
}
149
149
150
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
150
+
step := BuildCloneStep(twf, tr, false)
151
151
152
152
// Empty step when skip is true
153
153
if step.Name() != "" {
···
177
177
},
178
178
}
179
179
180
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", true)
180
+
step := BuildCloneStep(twf, tr, true)
181
181
182
182
// In dev mode, should use http:// and replace localhost with host.docker.internal
183
183
allCmds := strings.Join(step.Commands(), " ")
···
207
207
},
208
208
}
209
209
210
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
210
+
step := BuildCloneStep(twf, tr, false)
211
211
212
212
allCmds := strings.Join(step.Commands(), " ")
213
213
if !strings.Contains(allCmds, "--depth=10") {
···
238
238
},
239
239
}
240
240
241
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
241
+
step := BuildCloneStep(twf, tr, false)
242
242
243
243
allCmds := strings.Join(step.Commands(), " ")
244
244
if !strings.Contains(allCmds, "--depth=1") {
···
263
263
},
264
264
}
265
265
266
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
266
+
step := BuildCloneStep(twf, tr, false)
267
267
268
268
// Should return an error step
269
269
if !strings.Contains(step.Name(), "error") {
···
296
296
},
297
297
}
298
298
299
-
step := BuildCloneStep(twf, tr, "/tangled/workspace", false)
299
+
step := BuildCloneStep(twf, tr, false)
300
300
301
301
// Should return an error step
302
302
if !strings.Contains(step.Name(), "error") {
···
309
309
}
310
310
}
311
311
312
-
func TestBuildCloneStep_CustomWorkspace(t *testing.T) {
312
+
func TestBuildCloneStep_UnknownTriggerKind(t *testing.T) {
313
313
twf := tangled.Pipeline_Workflow{
314
314
Clone: &tangled.Pipeline_CloneOpts{
315
315
Depth: 1,
···
317
317
},
318
318
}
319
319
tr := tangled.Pipeline_TriggerMetadata{
320
-
Kind: string(workflow.TriggerKindPush),
321
-
Push: &tangled.Pipeline_PushTriggerData{
322
-
NewSha: "abc123",
323
-
},
320
+
Kind: "unknown_trigger",
324
321
Repo: &tangled.Pipeline_TriggerRepo{
325
322
Knot: "example.com",
326
323
Did: "did:plc:user123",
···
328
325
},
329
326
}
330
327
331
-
step := BuildCloneStep(twf, tr, "/custom/path", false)
328
+
step := BuildCloneStep(twf, tr, false)
332
329
330
+
// Should return an error step
331
+
if !strings.Contains(step.Name(), "error") {
332
+
t.Error("Expected error in step name for unknown trigger kind")
333
+
}
334
+
333
335
allCmds := strings.Join(step.Commands(), " ")
334
-
if !strings.Contains(allCmds, "/custom/path") {
335
-
t.Error("Commands should use custom workspace directory")
336
+
if !strings.Contains(allCmds, "unknown trigger kind") {
337
+
t.Error("Commands should contain error message about unknown trigger kind")
336
338
}
337
339
}
338
340
339
-
func TestBuildCloneStep_DefaultWorkspace(t *testing.T) {
341
+
func TestBuildCloneStep_NilCloneOpts(t *testing.T) {
340
342
twf := tangled.Pipeline_Workflow{
341
-
Clone: &tangled.Pipeline_CloneOpts{
342
-
Depth: 1,
343
-
Skip: false,
344
-
},
343
+
Clone: nil, // Nil clone options should use defaults
345
344
}
346
345
tr := tangled.Pipeline_TriggerMetadata{
347
346
Kind: string(workflow.TriggerKindPush),
···
355
354
},
356
355
}
357
356
358
-
step := BuildCloneStep(twf, tr, "", false) // Empty should default to /tangled/workspace
357
+
step := BuildCloneStep(twf, tr, false)
359
358
359
+
// Should still work with default options
360
+
if step.Kind() != StepKindSystem {
361
+
t.Errorf("Expected StepKindSystem, got %v", step.Kind())
362
+
}
363
+
360
364
allCmds := strings.Join(step.Commands(), " ")
361
-
if !strings.Contains(allCmds, "/tangled/workspace") {
362
-
t.Error("Commands should default to /tangled/workspace")
365
+
if !strings.Contains(allCmds, "--depth=1") {
366
+
t.Error("Commands should default to '--depth=1' when Clone is nil")
367
+
}
368
+
if !strings.Contains(allCmds, "git init") {
369
+
t.Error("Commands should contain 'git init'")
363
370
}
364
371
}
History
4 rounds
5 comments
evan.jarrett.net
submitted
#3
1 commit
expand
collapse
spindle: implement shared clone step outside of nixery engine.
Signed-off-by: Evan Jarrett <evan@evanjarrett.com>
expand 1 comment
pull request successfully merged
evan.jarrett.net
submitted
#2
2 commits
expand
collapse
spindle: move the clone step out of nixery into a shared function for all spindle engines
Signed-off-by: Evan Jarrett <evan@evanjarrett.com>
Create a BuildCloneStep function that returns a step struct that inherits from models.Step
Signed-off-by: Evan Jarrett <evan@evanjarrett.com>
expand 2 comments
Not entirely sure why this says there are merge conflicts when i rebased master This is an attempt to go off of your design with returning a whole step struct. I think it turns out essentially the same code... so maybe i'm missing something obvious.
that is a bit strange, i don't see anything off with the logs on the knotserver itself. that aside, some comments on the code itself:
- don't think we need to
cd workspaceDir, we already have this upon contrainer creation (the WorkingDir is set):
resp, err := e.docker.ContainerCreate(ctx, &container.Config{
Image: addl.image,
Cmd: []string{"cat"},
OpenStdin: true, // so cat stays alive :3
Tty: false,
Hostname: "spindle",
WorkingDir: workspaceDir,
- the rest of the clone code itself looks good to me!
- could you squash the two commits into one? the commit message could be
spindle: introduce common clone stepor similar
evan.jarrett.net
submitted
#1
2 commits
expand
collapse
spindle: move the clone step out of nixery into a shared function for all spindle engines
Signed-off-by: Evan Jarrett <evan@evanjarrett.com>
Create a BuildCloneStep function that returns a step struct that inherits from models.Step
Signed-off-by: Evan Jarrett <evan@evanjarrett.com>
expand 0 comments
evan.jarrett.net
submitted
#0
1 commit
expand
collapse
spindle: move the clone step out of nixery into a shared function for all spindle engines
Signed-off-by: Evan Jarrett <evan@evanjarrett.com>
expand 2 comments
thanks for the contribution! couple of nits:
- we already have a top level
workflowpackage that also definesCloneOpts(to unmarshal the workflow yaml file however), it is a bit confusing to introduce anotherworkflowpackage insidespindlewith a similar structCloneOptions(it also shares some of the same fields) - IMO a simpler route here would be to have a generic
CloneStepmethod that is calculated fromtangled.Pipeline_Workflowand returns a struct that follows themodels.Stepinterface
@oppi.li What package would you want this generic cloneStep in?
I think your solution is better, there was no precedent for a custom models.Step outside of nixery, so I was originally trying to avoid that.
I would still eventually want RepoURL and CommitSHA exposed for my usecase. but I might have to just submit a different PR for system-level env vars.
Gave it a once-over; looks good! And thanks for the tests.