fixup! Init and run the git component only when needed

kufu-ai · Jan 24, 2024 · f182687 · f182687
1 parent dd9dbf0
commit f182687
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 49 deletions.
diff --git a/workflow.go b/workflow.go
@@ -199,41 +199,7 @@ func (wf *Workflow) load(path, baseDir string, components map[string]Component)
 	for name, c := range components {
 		subPath := ID(path, name)
 
-		var needs []string
-		for _, n := range c.Needs {
-			needs = append(needs, ID(path, n))
-
-			if n == gitJob {
-				// This is to ensure that the git job is managed by the topological sorter.
-				//
-				// And we do this only when any of the components needs the git job.
-				// Otherwise, we end up initializing (and possibly failing) the git component
-				// even when no other component needs it.
-				wf.deps[gitJob] = []string{}
-			}
-		}
-
-		dir := c.Dir
-		if dir == "" {
-			dir = baseDir
-		} else {
-			if dir[0] == '/' {
-				dir = filepath.Join(wf.Dir, dir)
-			} else {
-				dir = filepath.Join(baseDir, dir)
-			}
-		}
-
-		driver, err := newDriver(subPath, dir, c, wf.Options)
-		if err != nil {
-			return fmt.Errorf("component %q: %w", name, err)
-		}
-
-		wf.WorkflowJobs[subPath] = &WorkflowJob{
-			Dir:    dir,
-			Needs:  needs,
-			Driver: driver,
-		}
+		j := &WorkflowJob{}
 
 		//
 		// We can override the component's skipped flag via options
@@ -245,13 +211,11 @@ func (wf *Workflow) load(path, baseDir string, components map[string]Component)
 		} else {
 			outs = map[string]map[string]string{}
 		}
-
 		if len(outs) != len(wf.Options.Skip) {
 			return fmt.Errorf("the number of skipped jobs (%d) doesn't match the number of skipped jobs outputs (%d)", len(wf.Options.Skip), len(outs))
 		}
 
 		var skipped bool
-
 		for _, s := range wf.Options.Skip {
 			if s == subPath {
 				var m map[string]string
@@ -261,23 +225,57 @@ func (wf *Workflow) load(path, baseDir string, components map[string]Component)
 					m = map[string]string{}
 				}
 
-				wf.WorkflowJobs[subPath].Skipped = m
+				j.Skipped = m
 				skipped = true
 				break
 			}
 		}
 
+		var needs []string
+		if !skipped {
+			for _, n := range c.Needs {
+				needs = append(needs, ID(path, n))
+
+				if n == gitJob {
+					// This is to ensure that the git job is managed by the topological sorter.
+					//
+					// And we do this only when any of the components needs the git job.
+					// Otherwise, we end up initializing (and possibly failing) the git component
+					// even when no other component needs it.
+					wf.deps[gitJob] = []string{}
+				}
+			}
+		}
+
+		dir := c.Dir
+		if dir == "" {
+			dir = baseDir
+		} else {
+			if dir[0] == '/' {
+				dir = filepath.Join(wf.Dir, dir)
+			} else {
+				dir = filepath.Join(baseDir, dir)
+			}
+		}
+
+		driver, err := newDriver(subPath, dir, c, wf.Options)
+		if err != nil {
+			return fmt.Errorf("component %q: %w", name, err)
+		}
+
+		j.Dir = dir
+		j.Needs = needs
+		j.Driver = driver
+
+		wf.WorkflowJobs[subPath] = j
+
 		if len(c.Components) > 0 {
 			if err := wf.load(subPath, dir, c.Components); err != nil {
 				return err
 			}
 		}
 
-		if skipped {
-			wf.deps[subPath] = []string{}
-		} else {
-			wf.deps[subPath] = needs
-		}
+		wf.deps[subPath] = needs
 	}
 
 	return nil

diff --git a/workflow_test.go b/workflow_test.go
@@ -70,7 +70,7 @@ func TestWorkflowLoad(t *testing.T) {
 
 func TestWorkflowLoad_SkipImage(t *testing.T) {
 	plan := [][]string{
-		{"git", "image", "prereq"},
+		{"image", "prereq"},
 		{"deploy"},
 	}
 
@@ -98,19 +98,19 @@ func TestWorkflowLoad_SkipImage(t *testing.T) {
 
 func TestWorkflowLoad_SkipTransitive(t *testing.T) {
 	plan := [][]string{
-		{"git", "image", "prereq"},
+		{"image", "prereq"},
 		{"deploy"},
 	}
 
 	opts := func(o *kanvas.Options) {
-		o.Skip = []string{"image", "git", "prereq"}
+		o.Skip = []string{"image", "prereq"}
 		o.SkippedJobsOutputs = map[string]map[string]string{
 			"image": {
 				"tag": "foobar",
 			},
-			"git": {
-				"sha": "123d3c1a9a669f45c17a25cf5222c0cc0b630738",
-			},
+			// "git": {
+			// 	"sha": "123d3c1a9a669f45c17a25cf5222c0cc0b630738",
+			// },
 			"prereq": {},
 		}
 	}
@@ -131,3 +131,29 @@ func TestWorkflowLoad_SkipTransitive(t *testing.T) {
 
 	require.NoError(t, i.Diff())
 }
+
+func TestWorkflowLoad_TryingToSkipUnneededGit(t *testing.T) {
+	opts := func(o *kanvas.Options) {
+		o.Skip = []string{"image", "prereq"}
+		o.SkippedJobsOutputs = map[string]map[string]string{
+			"image": {
+				"tag": "foobar",
+			},
+			"git": {
+				"sha": "123d3c1a9a669f45c17a25cf5222c0cc0b630738",
+			},
+			"prereq": {},
+		}
+	}
+
+	c := newComponent()
+	o := kanvas.Options{
+		TempDir: t.TempDir(),
+	}
+
+	opts(&o)
+
+	_, err := kanvas.NewWorkflow(c, o)
+	require.Error(t, err)
+	require.Equal(t, `loading "" "testdata/workflow": the number of skipped jobs (2) doesn't match the number of skipped jobs outputs (3)`, err.Error())
+}