Skip to content

Commit 6baa132

Browse files
authored
Conditional execution of steps with if: (#520)
This extends the schema of the batch spec `steps` to add a new field: - `if: <string that can use templating to evaluate to "true" or "false">` (the corresponding schema changes `sourcegraph/sourcegraph` are here: https://github.com/sourcegraph/sourcegraph/pull/20399) `if:` is evaluated in a best-effort partial-evaluation manner before the execution of the `steps`. If we can statically evaluate it, we use the result of that evaluation to build a per-repository/per-workspace list of tasks, which increases cache utilisation. See the comments in `partial_eval.go` for more details. It also extends the `StepContext` in the templates to have the `steps.{added,modified,deleted}_files` and `steps.path` fields.
1 parent 22b2158 commit 6baa132

18 files changed

+1590
-374
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ All notable changes to `src-cli` are documented in this file.
1313

1414
### Added
1515

16+
- Starting with Sourcegraph 3.28.0 batch spec `steps` can contain an `if: <template string>` attribute that determines whether the given step will be executed. [#520](https://github.com/sourcegraph/src-cli/pull/520)
17+
1618
### Changed
1719

1820
### Fixed

cmd/src/batch_common.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ type executeBatchSpecOpts struct {
201201
// Sourcegraph, including execution as needed and applying the resulting batch
202202
// spec if specified.
203203
func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) error {
204+
batches.DebugOut = opts.out
204205
svc := service.New(&service.Opts{
205206
AllowUnsupported: opts.flags.allowUnsupported,
206207
AllowIgnored: opts.flags.allowIgnored,
@@ -285,11 +286,15 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) error {
285286
}
286287

287288
pending = batchCreatePending(opts.out, "Determining workspaces")
288-
tasks, err := svc.BuildTasks(ctx, repos, batchSpec)
289+
tb, err := executor.NewTaskBuilder(batchSpec, svc)
289290
if err != nil {
290-
return errors.Wrap(err, "Calculating execution plan")
291+
return errors.Wrap(err, "Parsing batch spec to determine workspaces")
291292
}
292-
batchCompletePending(pending, fmt.Sprintf("Found %d workspaces", len(tasks)))
293+
tasks, err := tb.BuildAll(ctx, repos)
294+
if err != nil {
295+
return err
296+
}
297+
batchCompletePending(pending, fmt.Sprintf("Found %d workspaces with steps to execute", len(tasks)))
293298

294299
execOpts := executor.Opts{
295300
CacheDir: opts.flags.cacheDir,

internal/batches/batch_spec.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,25 @@ type Step struct {
9494
Files map[string]string `json:"files,omitempty" yaml:"files,omitempty"`
9595
Outputs Outputs `json:"outputs,omitempty" yaml:"outputs,omitempty"`
9696

97+
If interface{} `json:"if,omitempty" yaml:"if,omitempty"`
98+
9799
image docker.Image
98100
}
99101

102+
func (s *Step) IfCondition() string {
103+
switch v := s.If.(type) {
104+
case bool:
105+
if v {
106+
return "true"
107+
}
108+
return "false"
109+
case string:
110+
return v
111+
default:
112+
return ""
113+
}
114+
}
115+
100116
func (s *Step) SetImage(image docker.Image) {
101117
s.image = image
102118
}
@@ -178,6 +194,17 @@ func ParseBatchSpec(data []byte, features FeatureFlags) (*BatchSpec, error) {
178194
errs = multierror.Append(errs, errors.New("batch spec includes workspaces, which is not supported in this Sourcegraph version"))
179195
}
180196

197+
if !features.AllowConditionalExec {
198+
for i, step := range spec.Steps {
199+
if step.IfCondition() != "" {
200+
errs = multierror.Append(errs, fmt.Errorf(
201+
"step %d in batch spec uses the 'if' attribute for conditional execution, which is not supported in this Sourcegraph version",
202+
i+1,
203+
))
204+
}
205+
}
206+
}
207+
181208
return &spec, errs.ErrorOrNil()
182209
}
183210

internal/batches/batch_spec_test.go

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package batches
22

3-
import "testing"
3+
import (
4+
"fmt"
5+
"testing"
6+
)
47

58
func TestParseBatchSpec(t *testing.T) {
69
t.Run("valid", func(t *testing.T) {
@@ -87,4 +90,84 @@ changesetTemplate:
8790
t.Fatalf("wrong error. want=%q, have=%q", wantErr, haveErr)
8891
}
8992
})
93+
94+
t.Run("uses unsupported conditional exec", func(t *testing.T) {
95+
const spec = `
96+
name: hello-world
97+
description: Add Hello World to READMEs
98+
on:
99+
- repositoriesMatchingQuery: file:README.md
100+
steps:
101+
- run: echo Hello World | tee -a $(find -name README.md)
102+
if: "false"
103+
container: alpine:3
104+
105+
changesetTemplate:
106+
title: Hello World
107+
body: My first batch change!
108+
branch: hello-world
109+
commit:
110+
message: Append Hello World to all README.md files
111+
published: false
112+
`
113+
114+
_, err := ParseBatchSpec([]byte(spec), FeatureFlags{})
115+
if err == nil {
116+
t.Fatal("no error returned")
117+
}
118+
119+
wantErr := `1 error occurred:
120+
* step 1 in batch spec uses the 'if' attribute for conditional execution, which is not supported in this Sourcegraph version
121+
122+
`
123+
haveErr := err.Error()
124+
if haveErr != wantErr {
125+
t.Fatalf("wrong error. want=%q, have=%q", wantErr, haveErr)
126+
}
127+
})
128+
129+
t.Run("parsing if attribute", func(t *testing.T) {
130+
const specTemplate = `
131+
name: hello-world
132+
description: Add Hello World to READMEs
133+
on:
134+
- repositoriesMatchingQuery: file:README.md
135+
steps:
136+
- run: echo Hello World | tee -a $(find -name README.md)
137+
if: %s
138+
container: alpine:3
139+
140+
changesetTemplate:
141+
title: Hello World
142+
body: My first batch change!
143+
branch: hello-world
144+
commit:
145+
message: Append Hello World to all README.md files
146+
published: false
147+
`
148+
149+
for _, tt := range []struct {
150+
raw string
151+
want string
152+
}{
153+
{raw: `"true"`, want: "true"},
154+
{raw: `"false"`, want: "false"},
155+
{raw: `true`, want: "true"},
156+
{raw: `false`, want: "false"},
157+
{raw: `"${{ foobar }}"`, want: "${{ foobar }}"},
158+
{raw: `${{ foobar }}`, want: "${{ foobar }}"},
159+
{raw: `foobar`, want: "foobar"},
160+
} {
161+
spec := fmt.Sprintf(specTemplate, tt.raw)
162+
batchSpec, err := ParseBatchSpec([]byte(spec), FeatureFlags{AllowConditionalExec: true})
163+
if err != nil {
164+
t.Fatal(err)
165+
}
166+
167+
fmt.Printf("len(batchSpec.Steps)=%d", len(batchSpec.Steps))
168+
if batchSpec.Steps[0].IfCondition() != tt.want {
169+
t.Fatalf("wrong IfCondition. want=%q, got=%q", tt.want, batchSpec.Steps[0].IfCondition())
170+
}
171+
}
172+
})
90173
}

internal/batches/debug.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package batches
22

3-
import "github.com/sourcegraph/sourcegraph/lib/output"
3+
import (
4+
"github.com/sourcegraph/sourcegraph/lib/output"
5+
)
46

57
// DebugOut can be used to print debug messages in development to the TUI.
68
// For that it needs to be set to an actual *output.Output.

internal/batches/executor/executor_test.go

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,6 @@ output4=integration-test-batch-change`,
305305
wantAuthorName: "myOutputName1=main.go",
306306
wantAuthorEmail: "myOutputName1=main.go",
307307
},
308-
309308
{
310309
name: "workspaces",
311310
archives: []mock.RepoArchive{
@@ -374,6 +373,40 @@ output4=integration-test-batch-change`,
374373
},
375374
},
376375
},
376+
{
377+
name: "step condition",
378+
archives: []mock.RepoArchive{
379+
{Repo: srcCLIRepo, Files: map[string]string{
380+
"README.md": "# Welcome to the README\n",
381+
}},
382+
{Repo: sourcegraphRepo, Files: map[string]string{
383+
"README.md": "# Sourcegraph README\n",
384+
}},
385+
},
386+
steps: []batches.Step{
387+
{Run: `echo -e "foobar\n" >> README.md`},
388+
{
389+
Run: `echo "foobar" >> hello.txt`,
390+
If: `${{ matches repository.name "github.com/sourcegraph/sourcegra*" }}`,
391+
},
392+
{
393+
Run: `echo "foobar" >> in-path.txt`,
394+
If: `${{ matches steps.path "sub/directory/of/repo" }}`,
395+
},
396+
},
397+
tasks: []*Task{
398+
{Repository: srcCLIRepo},
399+
{Repository: sourcegraphRepo, Path: "sub/directory/of/repo"},
400+
},
401+
wantFilesChanged: filesByRepository{
402+
srcCLIRepo.ID: filesByBranch{
403+
changesetTemplateBranch: []string{"README.md"},
404+
},
405+
sourcegraphRepo.ID: {
406+
changesetTemplateBranch: []string{"README.md", "hello.txt", "in-path.txt"},
407+
},
408+
},
409+
},
377410
}
378411

379412
for _, tc := range tests {
@@ -949,5 +982,6 @@ func featuresAllEnabled() batches.FeatureFlags {
949982
UseGzipCompression: true,
950983
AllowTransformChanges: true,
951984
AllowWorkspaces: true,
985+
AllowConditionalExec: true,
952986
}
953987
}

0 commit comments

Comments
 (0)