From aba163aa5a762a497a5b7a0f1a43fb756c916d64 Mon Sep 17 00:00:00 2001
From: Tyler Cipriani <tcipriani@wikimedia.org>
Date: Thu, 19 Jul 2018 10:26:20 -0600
Subject: Add slash to directory path if copying > 1 file

Summary:
Ran into a Docker error when attempting to build a node project with
more than one requirements files:

    When using COPY with more than one source file, the destination must
    be a directory and end with a /

Reviewers: dduvall, #release-engineering-team

Reviewed By: dduvall, #release-engineering-team

Tags: #release-engineering-team

Differential Revision: https://phabricator.wikimedia.org/D1080
---
 build/instructions.go       | 10 +++++++++-
 build/instructions_test.go  |  8 ++++----
 docker/instructions_test.go | 10 +++++-----
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/build/instructions.go b/build/instructions.go
index 0167a8a..d6cc1c3 100644
--- a/build/instructions.go
+++ b/build/instructions.go
@@ -75,7 +75,15 @@ type Copy struct {
 // Compile quotes the defined source files/directories and destination.
 //
 func (copy Copy) Compile() []string {
-	return append(quoteAll(copy.Sources), quote(copy.Destination))
+	dest := copy.Destination
+
+	// If there is more than 1 file being copied, the destination must be a
+	// directory ending with "/"
+	if len(copy.Sources) > 1 && !strings.HasSuffix(copy.Destination, "/") {
+		dest = dest + "/"
+	}
+
+	return append(quoteAll(copy.Sources), quote(dest))
 }
 
 // CopyAs is a concrete build instruction for copying source
diff --git a/build/instructions_test.go b/build/instructions_test.go
index 77938dd..561bf80 100644
--- a/build/instructions_test.go
+++ b/build/instructions_test.go
@@ -33,7 +33,7 @@ func TestRunAll(t *testing.T) {
 func TestCopy(t *testing.T) {
 	i := build.Copy{[]string{"source1", "source2"}, "dest"}
 
-	assert.Equal(t, []string{`"source1"`, `"source2"`, `"dest"`}, i.Compile())
+	assert.Equal(t, []string{`"source1"`, `"source2"`, `"dest/"`}, i.Compile())
 }
 
 func TestCopyAs(t *testing.T) {
@@ -44,7 +44,7 @@ func TestCopyAs(t *testing.T) {
 			build.Copy{[]string{"source1", "source2"}, "dest"},
 		}
 
-		assert.Equal(t, []string{"123:124", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+		assert.Equal(t, []string{"123:124", `"source1"`, `"source2"`, `"dest/"`}, i.Compile())
 	})
 
 	t.Run("wrapping CopyFrom", func(t *testing.T) {
@@ -54,14 +54,14 @@ func TestCopyAs(t *testing.T) {
 			build.CopyFrom{"foo", build.Copy{[]string{"source1", "source2"}, "dest"}},
 		}
 
-		assert.Equal(t, []string{"123:124", "foo", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+		assert.Equal(t, []string{"123:124", "foo", `"source1"`, `"source2"`, `"dest/"`}, i.Compile())
 	})
 }
 
 func TestCopyFrom(t *testing.T) {
 	i := build.CopyFrom{"foo", build.Copy{[]string{"source1", "source2"}, "dest"}}
 
-	assert.Equal(t, []string{"foo", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+	assert.Equal(t, []string{"foo", `"source1"`, `"source2"`, `"dest/"`}, i.Compile())
 }
 
 func TestEntryPoint(t *testing.T) {
diff --git a/docker/instructions_test.go b/docker/instructions_test.go
index 07b71c7..9bdbeb1 100644
--- a/docker/instructions_test.go
+++ b/docker/instructions_test.go
@@ -37,7 +37,7 @@ func TestCopy(t *testing.T) {
 	di, err := docker.NewInstruction(i)
 
 	if assert.NoError(t, err) {
-		assert.Equal(t, "COPY [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+		assert.Equal(t, "COPY [\"foo1\", \"foo2\", \"bar/\"]\n", di.Compile())
 	}
 }
 
@@ -48,7 +48,7 @@ func TestCopyAs(t *testing.T) {
 		di, err := docker.NewInstruction(i)
 
 		if assert.NoError(t, err) {
-			assert.Equal(t, "COPY --chown=123:124 [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+			assert.Equal(t, "COPY --chown=123:124 [\"foo1\", \"foo2\", \"bar/\"]\n", di.Compile())
 		}
 	})
 
@@ -58,7 +58,7 @@ func TestCopyAs(t *testing.T) {
 		di, err := docker.NewInstruction(i)
 
 		if assert.NoError(t, err) {
-			assert.Equal(t, "COPY --chown=123:124 --from=foo [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+			assert.Equal(t, "COPY --chown=123:124 --from=foo [\"foo1\", \"foo2\", \"bar/\"]\n", di.Compile())
 		}
 	})
 }
@@ -69,7 +69,7 @@ func TestCopyFrom(t *testing.T) {
 	di, err := docker.NewInstruction(i)
 
 	if assert.NoError(t, err) {
-		assert.Equal(t, "COPY --from=foo [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+		assert.Equal(t, "COPY --from=foo [\"foo1\", \"foo2\", \"bar/\"]\n", di.Compile())
 	}
 }
 
@@ -149,7 +149,7 @@ func TestEscapeCopy(t *testing.T) {
 	di, err := docker.NewInstruction(i)
 
 	if assert.NoError(t, err) {
-		assert.Equal(t, "COPY [\"file.a\", \"file.b\", \"dest\"]\n", di.Compile())
+		assert.Equal(t, "COPY [\"file.a\", \"file.b\", \"dest/\"]\n", di.Compile())
 	}
 }
 
-- 
cgit v1.2.1