Merge pull request #9826 from icecrime/8318_whitespace_add_copy

Support whitespace in paths for ADD and COPY

builder/parser/parser.go

@@ -51,8 +51,8 @@ func init() {
 		"env":        parseEnv,
 		"maintainer": parseString,
 		"from":       parseString,
-		"add":        parseStringsWhitespaceDelimited,
-		"copy":       parseStringsWhitespaceDelimited,
+		"add":        parseMaybeJSONToList,
+		"copy":       parseMaybeJSONToList,
 		"run":        parseMaybeJSON,
 		"cmd":        parseMaybeJSON,
 		"entrypoint": parseMaybeJSON,

builder/parser/testfiles/ADD-COPY-with-JSON/Dockerfile

@@ -0,0 +1,9 @@
+FROM	ubuntu:14.04
+MAINTAINER	Seongyeol Lim <seongyeol37@gmail.com>
+
+COPY	.	/go/src/github.com/docker/docker
+ADD		.	/
+ADD		[ "vimrc", "/tmp" ]
+COPY	[ "bashrc", "/tmp" ]
+COPY	[ "test file", "/tmp" ]
+ADD		[ "test file", "/tmp/test file" ]

builder/parser/testfiles/ADD-COPY-with-JSON/result

@@ -0,0 +1,8 @@
+(from "ubuntu:14.04")
+(maintainer "Seongyeol Lim <seongyeol37@gmail.com>")
+(copy "." "/go/src/github.com/docker/docker")
+(add "." "/")
+(add "vimrc" "/tmp")
+(copy "bashrc" "/tmp")
+(copy "test file" "/tmp")
+(add "test file" "/tmp/test file")

docs/man/Dockerfile.5.md

@@ -132,7 +132,11 @@ or
  interactively, as with the following command: **docker run -t -i image bash**
 
 **ADD**
- --**ADD <src>... <dest>** The ADD instruction copies new files, directories
+ --ADD has two forms:
+ **ADD <src>... <dest>**
+ **ADD ["<src>"... "<dest>"]** This form is required for paths containing
+ whitespace.
+ The ADD instruction copies new files, directories
  or remote file URLs to the filesystem of the container at path <dest>.
  Mutliple <src> resources may be specified but if they are files or directories
  then they must be relative to the source directory that is being built
@@ -142,7 +146,11 @@ or
  and gid of 0.
 
 **COPY**
- --**COPY <src> <dest>** The COPY instruction copies new files from <src> and
+ --COPY has two forms:
+ **COPY <src>... <dest>**
+ **COPY ["<src>"... "<dest>"]** This form is required for paths containing
+ whitespace.
+ The COPY instruction copies new files from <src> and
  adds them to the filesystem of the container at path <dest>. The <src> must be
  the path to a file or directory relative to the source directory that is
  being built (the context of the build) or a remote file URL. The `<dest>` is an

docs/sources/reference/builder.md

@@ -388,7 +388,11 @@ change them using `docker run --env <key>=<value>`.
 
 ## ADD
 
-    ADD <src>... <dest>
+ADD has two forms:
+
+- `ADD <src>... <dest>`
+- `ADD ["<src>"... "<dest>"]` (this form is required for paths containing
+whitespace)
 
 The `ADD` instruction copies new files, directories or remote file URLs from `<src>`
 and adds them to the filesystem of the container at the path `<dest>`.  
@@ -488,7 +492,11 @@ The copy obeys the following rules:
 
 ## COPY
 
-    COPY <src>... <dest>
+COPY has two forms:
+
+- `COPY <src>... <dest>`
+- `COPY ["<src>"... "<dest>"]` (this form is required for paths containing
+whitespace)
 
 The `COPY` instruction copies new files or directories from `<src>`
 and adds them to the filesystem of the container at the path `<dest>`.

integration-cli/docker_cli_build_test.go

@@ -802,7 +802,7 @@ RUN [ $(ls -l /exists/exists_file | awk '{print $3":"$4}') = 'dockerio:dockerio'
 	if _, err := buildImageFromContext(name, ctx, true); err != nil {
 		t.Fatal(err)
 	}
-	logDone("build - mulitple file copy/add tests")
+	logDone("build - multiple file copy/add tests")
 }
 
 func TestBuildAddMultipleFilesToFile(t *testing.T) {
@@ -810,7 +810,7 @@ func TestBuildAddMultipleFilesToFile(t *testing.T) {
 	defer deleteImages(name)
 	ctx, err := fakeContext(`FROM scratch
 	ADD file1.txt file2.txt test
-        `,
+	`,
 		map[string]string{
 			"file1.txt": "test1",
 			"file2.txt": "test1",
@@ -822,18 +822,41 @@ func TestBuildAddMultipleFilesToFile(t *testing.T) {
 
 	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
 	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
-		t.Fatalf("Wrong error: (should contain \"%s\") got:\n%v", expected, err)
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
 	}
 
 	logDone("build - multiple add files to file")
 }
 
+func TestBuildJSONAddMultipleFilesToFile(t *testing.T) {
+	name := "testjsonaddmultiplefilestofile"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM scratch
+	ADD ["file1.txt", "file2.txt", "test"]
+	`,
+		map[string]string{
+			"file1.txt": "test1",
+			"file2.txt": "test1",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
+	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
+	}
+
+	logDone("build - multiple add files to file json syntax")
+}
+
 func TestBuildAddMultipleFilesToFileWild(t *testing.T) {
 	name := "testaddmultiplefilestofilewild"
 	defer deleteImages(name)
 	ctx, err := fakeContext(`FROM scratch
 	ADD file*.txt test
-        `,
+	`,
 		map[string]string{
 			"file1.txt": "test1",
 			"file2.txt": "test1",
@@ -845,18 +868,41 @@ func TestBuildAddMultipleFilesToFileWild(t *testing.T) {
 
 	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
 	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
-		t.Fatalf("Wrong error: (should contain \"%s\") got:\n%v", expected, err)
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
 	}
 
 	logDone("build - multiple add files to file wild")
 }
 
+func TestBuildJSONAddMultipleFilesToFileWild(t *testing.T) {
+	name := "testjsonaddmultiplefilestofilewild"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM scratch
+	ADD ["file*.txt", "test"]
+	`,
+		map[string]string{
+			"file1.txt": "test1",
+			"file2.txt": "test1",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
+	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
+	}
+
+	logDone("build - multiple add files to file wild json syntax")
+}
+
 func TestBuildCopyMultipleFilesToFile(t *testing.T) {
 	name := "testcopymultiplefilestofile"
 	defer deleteImages(name)
 	ctx, err := fakeContext(`FROM scratch
 	COPY file1.txt file2.txt test
-        `,
+	`,
 		map[string]string{
 			"file1.txt": "test1",
 			"file2.txt": "test1",
@@ -868,12 +914,155 @@ func TestBuildCopyMultipleFilesToFile(t *testing.T) {
 
 	expected := "When using COPY with more than one source file, the destination must be a directory and end with a /"
 	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
-		t.Fatalf("Wrong error: (should contain \"%s\") got:\n%v", expected, err)
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
 	}
 
 	logDone("build - multiple copy files to file")
 }
 
+func TestBuildJSONCopyMultipleFilesToFile(t *testing.T) {
+	name := "testjsoncopymultiplefilestofile"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM scratch
+	COPY ["file1.txt", "file2.txt", "test"]
+	`,
+		map[string]string{
+			"file1.txt": "test1",
+			"file2.txt": "test1",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := "When using COPY with more than one source file, the destination must be a directory and end with a /"
+	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
+	}
+
+	logDone("build - multiple copy files to file json syntax")
+}
+
+func TestBuildAddFileWithWhitespace(t *testing.T) {
+	name := "testaddfilewithwhitespace"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM busybox
+RUN mkdir "/test dir"
+RUN mkdir "/test_dir"
+ADD [ "test file1", "/test_file1" ]
+ADD [ "test_file2", "/test file2" ]
+ADD [ "test file3", "/test file3" ]
+ADD [ "test dir/test_file4", "/test_dir/test_file4" ]
+ADD [ "test_dir/test_file5", "/test dir/test_file5" ]
+ADD [ "test dir/test_file6", "/test dir/test_file6" ]
+RUN [ $(cat "/test_file1") = 'test1' ]
+RUN [ $(cat "/test file2") = 'test2' ]
+RUN [ $(cat "/test file3") = 'test3' ]
+RUN [ $(cat "/test_dir/test_file4") = 'test4' ]
+RUN [ $(cat "/test dir/test_file5") = 'test5' ]
+RUN [ $(cat "/test dir/test_file6") = 'test6' ]`,
+		map[string]string{
+			"test file1":          "test1",
+			"test_file2":          "test2",
+			"test file3":          "test3",
+			"test dir/test_file4": "test4",
+			"test_dir/test_file5": "test5",
+			"test dir/test_file6": "test6",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatal(err)
+	}
+	logDone("build - add file with whitespace")
+}
+
+func TestBuildCopyFileWithWhitespace(t *testing.T) {
+	name := "testcopyfilewithwhitespace"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM busybox
+RUN mkdir "/test dir"
+RUN mkdir "/test_dir"
+COPY [ "test file1", "/test_file1" ]
+COPY [ "test_file2", "/test file2" ]
+COPY [ "test file3", "/test file3" ]
+COPY [ "test dir/test_file4", "/test_dir/test_file4" ]
+COPY [ "test_dir/test_file5", "/test dir/test_file5" ]
+COPY [ "test dir/test_file6", "/test dir/test_file6" ]
+RUN [ $(cat "/test_file1") = 'test1' ]
+RUN [ $(cat "/test file2") = 'test2' ]
+RUN [ $(cat "/test file3") = 'test3' ]
+RUN [ $(cat "/test_dir/test_file4") = 'test4' ]
+RUN [ $(cat "/test dir/test_file5") = 'test5' ]
+RUN [ $(cat "/test dir/test_file6") = 'test6' ]`,
+		map[string]string{
+			"test file1":          "test1",
+			"test_file2":          "test2",
+			"test file3":          "test3",
+			"test dir/test_file4": "test4",
+			"test_dir/test_file5": "test5",
+			"test dir/test_file6": "test6",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatal(err)
+	}
+	logDone("build - copy file with whitespace")
+}
+
+func TestBuildAddMultipleFilesToFileWithWhitespace(t *testing.T) {
+	name := "testaddmultiplefilestofilewithwhitespace"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM busybox
+	ADD [ "test file1", "test file2", "test" ]
+    `,
+		map[string]string{
+			"test file1": "test1",
+			"test file2": "test2",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := "When using ADD with more than one source file, the destination must be a directory and end with a /"
+	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
+	}
+
+	logDone("build - multiple add files to file with whitespace")
+}
+
+func TestBuildCopyMultipleFilesToFileWithWhitespace(t *testing.T) {
+	name := "testcopymultiplefilestofilewithwhitespace"
+	defer deleteImages(name)
+	ctx, err := fakeContext(`FROM busybox
+	COPY [ "test file1", "test file2", "test" ]
+        `,
+		map[string]string{
+			"test file1": "test1",
+			"test file2": "test2",
+		})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := "When using COPY with more than one source file, the destination must be a directory and end with a /"
+	if _, err := buildImageFromContext(name, ctx, true); err == nil || !strings.Contains(err.Error(), expected) {
+		t.Fatalf("Wrong error: (should contain %q) got:\n%v", expected, err)
+	}
+
+	logDone("build - multiple copy files to file with whitespace")
+}
+
 func TestBuildCopyWildcard(t *testing.T) {
 	name := "testcopywildcard"
 	defer deleteImages(name)