Make steps resolution part of workspaces detection

Author: eseliger

internal/batches/service/build_tasks.go

@@ -8,7 +8,6 @@ import (
 	"github.com/sourcegraph/sourcegraph/lib/batches/template"
 	"github.com/sourcegraph/src-cli/internal/batches/executor"
 	"github.com/sourcegraph/src-cli/internal/batches/graphql"
-	"github.com/sourcegraph/src-cli/internal/batches/util"
 )
 
 // buildTasks returns tasks for all the workspaces determined for the given spec.
@@ -24,56 +23,24 @@ func buildTasks(ctx context.Context, spec *batcheslib.BatchSpec, repos []*graphq
 		if !ok {
 			return nil, errors.New("invalid state, didn't find repo for workspace definition")
 		}
-		t, ok, err := buildTask(spec, repo, ws.Path, ws.OnlyFetchWorkspace)
+
+		t, err := buildTask(spec, repo, ws.Path, ws.OnlyFetchWorkspace, ws.Steps)
 		if err != nil {
 			return nil, err
 		}
 
-		if ok {
-			tasks = append(tasks, t)
-		}
+		tasks = append(tasks, t)
 	}
 
 	return tasks, nil
 }
 
-func buildTask(spec *batcheslib.BatchSpec, r *graphql.Repository, path string, onlyWorkspace bool) (*executor.Task, bool, error) {
+func buildTask(spec *batcheslib.BatchSpec, r *graphql.Repository, path string, onlyWorkspace bool, steps []batcheslib.Step) (*executor.Task, error) {
 	batchChange := template.BatchChangeAttributes{
 		Name:        spec.Name,
 		Description: spec.Description,
 	}
 
-	taskSteps := []batcheslib.Step{}
-	for _, step := range spec.Steps {
-		// If no if condition is given, just go ahead and add the step to the list.
-		if step.IfCondition() == "" {
-			taskSteps = append(taskSteps, step)
-			continue
-		}
-
-		stepCtx := &template.StepContext{
-			Repository:  util.GraphQLRepoToTemplatingRepo(r),
-			BatchChange: batchChange,
-		}
-		static, boolVal, err := template.IsStaticBool(step.IfCondition(), stepCtx)
-		if err != nil {
-			return nil, false, err
-		}
-
-		// If we could evaluate the condition statically and the resulting
-		// boolean is false, we don't add that step.
-		if !static {
-			taskSteps = append(taskSteps, step)
-		} else if boolVal {
-			taskSteps = append(taskSteps, step)
-		}
-	}
-
-	// If the task doesn't have any steps we don't need to execute it
-	if len(taskSteps) == 0 {
-		return nil, false, nil
-	}
-
 	// "." means the path is root, but in the executor we use "" to signify root
 	if path == "." {
 		path = ""
@@ -82,11 +49,11 @@ func buildTask(spec *batcheslib.BatchSpec, r *graphql.Repository, path string, o
 	return &executor.Task{
 		Repository:         r,
 		Path:               path,
-		Steps:              taskSteps,
+		Steps:              steps,
 		OnlyFetchWorkspace: onlyWorkspace,
 
 		TransformChanges:      spec.TransformChanges,
 		Template:              spec.ChangesetTemplate,
 		BatchChangeAttributes: &batchChange,
-	}, true, nil
+	}, nil
 }

internal/batches/service/build_tasks_test.go

@@ -3,16 +3,21 @@ package service
 import (
 	"context"
 	"fmt"
+	"sort"
 
 	"github.com/gobwas/glob"
+	"github.com/pkg/errors"
 	batcheslib "github.com/sourcegraph/sourcegraph/lib/batches"
+	"github.com/sourcegraph/sourcegraph/lib/batches/template"
 	"github.com/sourcegraph/src-cli/internal/batches"
 	"github.com/sourcegraph/src-cli/internal/batches/graphql"
+	"github.com/sourcegraph/src-cli/internal/batches/util"
 )
 
 type RepoWorkspace struct {
 	RepoID             string
 	Path               string
+	Steps              []batcheslib.Step
 	OnlyFetchWorkspace bool
 }
 
@@ -31,6 +36,11 @@ func findWorkspaces(
 	finder directoryFinder,
 	repos []*graphql.Repository,
 ) ([]RepoWorkspace, error) {
+	repoByID := make(map[string]*graphql.Repository)
+	for _, repo := range repos {
+		repoByID[repo.ID] = repo
+	}
+
 	// Pre-compile all globs.
 	workspaceMatchers := make(map[batcheslib.WorkspaceConfiguration]glob.Glob)
 	for _, conf := range spec.Workspaces {
@@ -117,12 +127,71 @@ func findWorkspaces(
 				fetchWorkspace = false
 			}
 
+			repo, ok := repoByID[workspace.RepoID]
+			if !ok {
+				return nil, errors.New("invalid state, repo not found")
+			}
+
+			steps, err := stepsForRepo(spec, repo)
+			if err != nil {
+				return nil, err
+			}
+
+			// If the workspace doesn't have any steps we don't need to include it.
+			if len(steps) == 0 {
+				continue
+			}
+
 			workspaces = append(workspaces, RepoWorkspace{
 				RepoID:             workspace.RepoID,
 				Path:               path,
+				Steps:              steps,
 				OnlyFetchWorkspace: fetchWorkspace,
 			})
 		}
 	}
+
+	// Stable sorting.
+	sort.Slice(workspaces, func(i, j int) bool {
+		if workspaces[i].RepoID == workspaces[j].RepoID {
+			return workspaces[i].Path < workspaces[j].Path
+		}
+		return workspaces[i].RepoID < workspaces[j].RepoID
+	})
+
 	return workspaces, nil
 }
+
+// stepsForRepo calculates the steps required to run on the given repo.
+func stepsForRepo(spec *batcheslib.BatchSpec, r *graphql.Repository) ([]batcheslib.Step, error) {
+	taskSteps := []batcheslib.Step{}
+	for _, step := range spec.Steps {
+		// If no if condition is given, just go ahead and add the step to the list.
+		if step.IfCondition() == "" {
+			taskSteps = append(taskSteps, step)
+			continue
+		}
+
+		batchChange := template.BatchChangeAttributes{
+			Name:        spec.Name,
+			Description: spec.Description,
+		}
+		stepCtx := &template.StepContext{
+			Repository:  util.GraphQLRepoToTemplatingRepo(r),
+			BatchChange: batchChange,
+		}
+		static, boolVal, err := template.IsStaticBool(step.IfCondition(), stepCtx)
+		if err != nil {
+			return nil, err
+		}
+
+		// If we could evaluate the condition statically and the resulting
+		// boolean is false, we don't add that step.
+		if !static {
+			taskSteps = append(taskSteps, step)
+		} else if boolVal {
+			taskSteps = append(taskSteps, step)
+		}
+	}
+	return taskSteps, nil
+}