Compile only unique Docker stages determined from artifacts

Summary:
The previous implementation for iterating over defined artifacts did not
take into account the possibility for multiple artifacts with the same
`from` variant name and would compile stages with duplicate names. This
fixes that behavior by iterating over a unique set of names.

Test Plan: Run `go test ./...` or `arc unit`.

Reviewers: thcipriani, mmodell, #release-engineering-team

Reviewed By: thcipriani, #release-engineering-team

Tags: #release-engineering-team

Differential Revision: https://phabricator.wikimedia.org/D758

2 files changed, 45 insertions, 7 deletions

diff --git a/docker/compiler.go b/docker/compiler.go
index 05daeaa..e77691f 100644
--- a/docker/compiler.go
+++ b/docker/compiler.go
@@ -17,16 +17,25 @@ func Compile(cfg *config.Config, variant string) *bytes.Buffer {
 		// omit the main stage name unless multi-stage is required below
 		mainStage := ""
 
-		// write multi-stage sections for each artifact dependency
+		// get unique set of artifact/variant dependencies
+		existing := map[string]bool{}
+		stages := []string{}
+
 		for _, artifact := range vcfg.Artifacts {
-			if artifact.From != "" {
+			if stage := artifact.From; stage != "" && !existing[stage] {
+				existing[stage] = true
+				stages = append(stages, stage)
+
 				mainStage = variant
+			}
+		}
 
-				dependency, err := config.ExpandVariant(cfg, artifact.From)
+		// write multi-stage sections for each artifact/variant dependency
+		for _, stage := range stages {
+			dependency, err := config.ExpandVariant(cfg, stage)
 
-				if err == nil {
-					CompileStage(buffer, artifact.From, dependency)
-				}
+			if err == nil {
+				CompileStage(buffer, stage, dependency)
 			}
 		}
 
@@ -88,7 +97,7 @@ func CompileStage(buffer *bytes.Buffer, stage string, vcfg *config.VariantConfig
 func CompilePhase(buffer *bytes.Buffer, vcfg *config.VariantConfig, phase build.Phase) {
 	for _, instruction := range vcfg.InstructionsForPhase(phase) {
 		dockerInstruction, _ := NewDockerInstruction(instruction)
-		Writeln(buffer, dockerInstruction.Compile())
+		Write(buffer, dockerInstruction.Compile())
 	}
 }
 
diff --git a/docker/compiler_test.go b/docker/compiler_test.go
index d6a1a53..0d327cb 100644
--- a/docker/compiler_test.go
+++ b/docker/compiler_test.go
@@ -1,6 +1,7 @@
 package docker_test
 
 import (
+	"strings"
 	"testing"
 
 	"gopkg.in/stretchr/testify.v1/assert"
@@ -39,4 +40,32 @@ func TestMultiStageIncludesStageNames(t *testing.T) {
 
 	assert.Contains(t, dockerfile, "FROM foo/bar AS build\n")
 	assert.Contains(t, dockerfile, "FROM foo/bar AS production\n")
+
+	assert.Equal(t, 1, strings.Count(dockerfile, "FROM foo/bar AS build\n"))
+	assert.Equal(t, 1, strings.Count(dockerfile, "FROM foo/bar AS production\n"))
+}
+
+func TestMultipleArtifactsFromSameStage(t *testing.T) {
+	cfg, err := config.ReadConfig([]byte(`---
+    base: foo/bar
+    variants:
+      build: {}
+      production:
+        artifacts:
+          - from: build
+            source: .
+            destination: .
+          - from: build
+            source: bar
+            destination: bar`))
+
+	assert.Nil(t, err)
+
+	dockerfile := docker.Compile(cfg, "production").String()
+
+	assert.Contains(t, dockerfile, "FROM foo/bar AS build\n")
+	assert.Contains(t, dockerfile, "FROM foo/bar AS production\n")
+
+	assert.Equal(t, 1, strings.Count(dockerfile, "FROM foo/bar AS build\n"))
+	assert.Equal(t, 1, strings.Count(dockerfile, "FROM foo/bar AS production\n"))
 }