Merge pull request #57 from laszlocph/depending-on-zero-step-pipe

Depending on zero step pipe

server/build.go

@@ -344,6 +344,7 @@ func PostApproval(c *gin.Context) {
 		store.UpdateBuild(c, build)
 		return
 	}
+	build = setBuildStepsOnBuild(b.Curr, buildItems)
 
 	err = store.FromContext(c).ProcCreate(build.Procs)
 	if err != nil {
@@ -558,6 +559,7 @@ func PostBuild(c *gin.Context) {
 		c.JSON(500, build)
 		return
 	}
+	build = setBuildStepsOnBuild(b.Curr, buildItems)
 
 	err = store.FromContext(c).ProcCreate(build.Procs)
 	if err != nil {

server/hook.go

@@ -177,7 +177,7 @@ func PostHook(c *gin.Context) {
 		return
 	}
 
-	if zeroSteps(*build, remoteYamlConfigs) {
+	if zeroSteps(build, remoteYamlConfigs) {
 		c.String(200, "Step conditions yield zero runnable steps")
 		return
 	}
@@ -261,6 +261,7 @@ func PostHook(c *gin.Context) {
 		store.UpdateBuild(c, build)
 		return
 	}
+	build = setBuildStepsOnBuild(b.Curr, buildItems)
 
 	err = store.FromContext(c).ProcCreate(build.Procs)
 	if err != nil {
@@ -298,11 +299,10 @@ func branchFiltered(build *model.Build, remoteYamlConfigs []*remote.FileMeta) bo
 	return true
 }
 
-// uses pass by value as procBuilder has side effects on build. Something to be fixed
+func zeroSteps(build *model.Build, remoteYamlConfigs []*remote.FileMeta) bool {
-func zeroSteps(build model.Build, remoteYamlConfigs []*remote.FileMeta) bool {
 	b := procBuilder{
 		Repo:  &model.Repo{},
-		Curr:  &build,
+		Curr:  build,
 		Last:  &model.Build{},
 		Netrc: &model.Netrc{},
 		Secs:  []*model.Secret{},

server/procBuilder.go

@@ -128,17 +128,50 @@ func (b *procBuilder) Build() ([]*buildItem, error) {
 				item.Labels = map[string]string{}
 			}
 
-			b.Curr.Procs = append(b.Curr.Procs, proc)
 			items = append(items, item)
 			pidSequence++
 		}
 	}
 
-	setBuildSteps(b.Curr, items)
+	items = filterItemsWithMissingDependencies(items)
 
 	return items, nil
 }
 
+func filterItemsWithMissingDependencies(items []*buildItem) []*buildItem {
+	itemsToRemove := make([]*buildItem, 0)
+
+	for _, item := range items {
+		for _, dep := range item.DependsOn {
+			if !containsItemWithName(dep, items) {
+				itemsToRemove = append(itemsToRemove, item)
+			}
+		}
+	}
+
+	if len(itemsToRemove) > 0 {
+		filtered := make([]*buildItem, 0)
+		for _, item := range items {
+			if !containsItemWithName(item.Proc.Name, itemsToRemove) {
+				filtered = append(filtered, item)
+			}
+		}
+		// Recursive to handle transitive deps
+		return filterItemsWithMissingDependencies(filtered)
+	}
+
+	return items
+}
+
+func containsItemWithName(name string, items []*buildItem) bool {
+	for _, item := range items {
+		if name == item.Proc.Name {
+			return true
+		}
+	}
+	return false
+}
+
 func (b *procBuilder) envsubst_(y string, environ map[string]string) (string, error) {
 	return envsubst.Eval(y, func(name string) string {
 		env := environ[name]
@@ -214,8 +247,12 @@ func (b *procBuilder) toInternalRepresentation(parsed *yaml.Config, environ map[
 	).Compile(parsed)
 }
 
-func setBuildSteps(build *model.Build, buildItems []*buildItem) {
+func setBuildStepsOnBuild(build *model.Build, buildItems []*buildItem) *model.Build {
-	pcounter := len(buildItems)
+	for _, item := range buildItems {
+		build.Procs = append(build.Procs, item.Proc)
+	}
+	pcounter := len(build.Procs)
+
 	for _, item := range buildItems {
 		for _, stage := range item.Config.Stages {
 			var gid int
@@ -239,6 +276,8 @@ func setBuildSteps(build *model.Build, buildItems []*buildItem) {
 			}
 		}
 	}
+
+	return build
 }
 
 // return the metadata from the cli context.

server/procBuilder_test.go

@@ -70,13 +70,11 @@ func TestMultiPipeline(t *testing.T) {
 pipeline:
   xxx:
     image: scratch
-    yyy: ${DRONE_COMMIT_MESSAGE}
 `)},
 			&remote.FileMeta{Data: []byte(`
 pipeline:
   build:
     image: scratch
-    yyy: ${DRONE_COMMIT_MESSAGE}
 `)},
 		},
 	}
@@ -100,6 +98,16 @@ func TestDependsOn(t *testing.T) {
 		Regs:  []*model.Registry{},
 		Link:  "",
 		Yamls: []*remote.FileMeta{
+			&remote.FileMeta{Name: "lint", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+`)},
+			&remote.FileMeta{Name: "test", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+`)},
 			&remote.FileMeta{Data: []byte(`
 pipeline:
   deploy:
@@ -108,7 +116,6 @@ pipeline:
 depends_on:
   - lint
   - test
-  - build
 `)},
 		},
 	}
@@ -117,7 +124,7 @@ depends_on:
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(buildItems[0].DependsOn) != 3 {
+	if len(buildItems[0].DependsOn) != 2 {
 		t.Fatal("Should have 3 dependencies")
 	}
 	if buildItems[0].DependsOn[1] != "test" {
@@ -173,14 +180,12 @@ func TestBranchFilter(t *testing.T) {
 pipeline:
   xxx:
     image: scratch
-    yyy: ${DRONE_COMMIT_MESSAGE}
 branches: master
 `)},
 			&remote.FileMeta{Data: []byte(`
 pipeline:
   build:
     image: scratch
-    yyy: ${DRONE_COMMIT_MESSAGE}
 `)},
 		},
 	}
@@ -224,7 +229,6 @@ pipeline:
     when:
       branch: notdev
     image: scratch
-    yyy: ${DRONE_COMMIT_MESSAGE}
 `)},
 		},
 	}
@@ -236,8 +240,103 @@ pipeline:
 	if len(buildItems) != 0 {
 		t.Fatal("Should not generate a build item if there are no steps")
 	}
-	if len(build.Procs) != 0 {
+}
-		t.Fatal("Should not generate a build item if there are no steps")
+
+func TestZeroStepsAsMultiPipelineDeps(t *testing.T) {
+	build := &model.Build{Branch: "dev"}
+
+	b := procBuilder{
+		Repo:  &model.Repo{},
+		Curr:  build,
+		Last:  &model.Build{},
+		Netrc: &model.Netrc{},
+		Secs:  []*model.Secret{},
+		Regs:  []*model.Registry{},
+		Link:  "",
+		Yamls: []*remote.FileMeta{
+			&remote.FileMeta{Name: "zerostep", Data: []byte(`
+skip_clone: true
+pipeline:
+  build:
+    when:
+      branch: notdev
+    image: scratch
+`)},
+			&remote.FileMeta{Name: "justastep", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+`)},
+			&remote.FileMeta{Name: "shouldbefiltered", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+depends_on: [ zerostep ]
+`)},
+		},
+	}
+
+	buildItems, err := b.Build()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(buildItems) != 1 {
+		t.Fatal("Zerostep and the step that depends on it should not generate a build item")
+	}
+	if "justastep" != buildItems[0].Proc.Name {
+		t.Fatal("justastep should have been generated")
+	}
+}
+
+func TestZeroStepsAsMultiPipelineTransitiveDeps(t *testing.T) {
+	build := &model.Build{Branch: "dev"}
+
+	b := procBuilder{
+		Repo:  &model.Repo{},
+		Curr:  build,
+		Last:  &model.Build{},
+		Netrc: &model.Netrc{},
+		Secs:  []*model.Secret{},
+		Regs:  []*model.Registry{},
+		Link:  "",
+		Yamls: []*remote.FileMeta{
+			&remote.FileMeta{Name: "zerostep", Data: []byte(`
+skip_clone: true
+pipeline:
+  build:
+    when:
+      branch: notdev
+    image: scratch
+`)},
+			&remote.FileMeta{Name: "justastep", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+`)},
+			&remote.FileMeta{Name: "shouldbefiltered", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+depends_on: [ zerostep ]
+`)},
+			&remote.FileMeta{Name: "shouldbefilteredtoo", Data: []byte(`
+pipeline:
+  build:
+    image: scratch
+depends_on: [ shouldbefiltered ]
+`)},
+		},
+	}
+
+	buildItems, err := b.Build()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(buildItems) != 1 {
+		t.Fatal("Zerostep and the step that depends on it, and the one depending on it should not generate a build item")
+	}
+	if "justastep" != buildItems[0].Proc.Name {
+		t.Fatal("justastep should have been generated")
 	}
 }
 
@@ -257,12 +356,12 @@ func TestTree(t *testing.T) {
 pipeline:
   build:
     image: scratch
-    yyy: ${DRONE_COMMIT_MESSAGE}
 `)},
 		},
 	}
 
-	_, err := b.Build()
+	buildItems, err := b.Build()
+	build = setBuildStepsOnBuild(build, buildItems)
 	if err != nil {
 		t.Fatal(err)
 	}