From abfb7138871993c53050a2d159ca0fae07bf65e8 Mon Sep 17 00:00:00 2001 From: Seongyeol Lim Date: Mon, 6 Oct 2014 03:15:09 -0700 Subject: [PATCH 1/2] Add support file name with whitespace for ADD and COPY command Closes #8318 Signed-off-by: Seongyeol Lim --- builder/parser/parser.go | 4 +- .../testfiles/ADD-COPY-with-JSON/Dockerfile | 9 ++ .../testfiles/ADD-COPY-with-JSON/result | 8 ++ integration-cli/docker_cli_build_test.go | 120 ++++++++++++++++++ 4 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 builder/parser/testfiles/ADD-COPY-with-JSON/Dockerfile create mode 100644 builder/parser/testfiles/ADD-COPY-with-JSON/result diff --git a/builder/parser/parser.go b/builder/parser/parser.go index ad42a1586e..6cec614db6 100644 --- a/builder/parser/parser.go +++ b/builder/parser/parser.go @@ -50,8 +50,8 @@ func init() { "env": parseEnv, "maintainer": parseString, "from": parseString, - "add": parseStringsWhitespaceDelimited, - "copy": parseStringsWhitespaceDelimited, + "add": parseMaybeJSONToList, + "copy": parseMaybeJSONToList, "run": parseMaybeJSON, "cmd": parseMaybeJSON, "entrypoint": parseMaybeJSON, diff --git a/builder/parser/testfiles/ADD-COPY-with-JSON/Dockerfile b/builder/parser/testfiles/ADD-COPY-with-JSON/Dockerfile new file mode 100644 index 0000000000..49372b0607 --- /dev/null +++ b/builder/parser/testfiles/ADD-COPY-with-JSON/Dockerfile @@ -0,0 +1,9 @@ +FROM ubuntu:14.04 +MAINTAINER Seongyeol Lim + +COPY . /go/src/github.com/docker/docker +ADD . / +ADD [ "vimrc", "/tmp" ] +COPY [ "bashrc", "/tmp" ] +COPY [ "test file", "/tmp" ] +ADD [ "test file", "/tmp/test file" ] diff --git a/builder/parser/testfiles/ADD-COPY-with-JSON/result b/builder/parser/testfiles/ADD-COPY-with-JSON/result new file mode 100644 index 0000000000..86c3fef726 --- /dev/null +++ b/builder/parser/testfiles/ADD-COPY-with-JSON/result @@ -0,0 +1,8 @@ +(from "ubuntu:14.04") +(maintainer "Seongyeol Lim ") +(copy "." "/go/src/github.com/docker/docker") +(add "." "/") +(add "vimrc" "/tmp") +(copy "bashrc" "/tmp") +(copy "test file" "/tmp") +(add "test file" "/tmp/test file") diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index e440bc7705..a3d2464d0b 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -834,6 +834,126 @@ func TestBuildCopyMultipleFilesToFile(t *testing.T) { logDone("build - multiple copy files to file") } +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 \"%s\") 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 \"%s\") got:\n%v", expected, err) + } + + logDone("build - multiple copy files to file with whitespace") +} + func TestBuildCopyWildcard(t *testing.T) { name := "testcopywildcard" defer deleteImages(name) From cf455017e059e4b52d8d0357070e24ee153e3069 Mon Sep 17 00:00:00 2001 From: Arnaud Porterie Date: Fri, 26 Dec 2014 16:30:34 -0800 Subject: [PATCH 2/2] Support whitespaces in ADD and COPY continued Add tests and documentation for this new feature. Signed-off-by: Arnaud Porterie --- docs/man/Dockerfile.5.md | 12 +++- docs/sources/reference/builder.md | 12 +++- integration-cli/docker_cli_build_test.go | 89 +++++++++++++++++++++--- 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/docs/man/Dockerfile.5.md b/docs/man/Dockerfile.5.md index 0114f30ba7..1630b5fc45 100644 --- a/docs/man/Dockerfile.5.md +++ b/docs/man/Dockerfile.5.md @@ -131,7 +131,11 @@ or interactively, as with the following command: **docker run -t -i image bash** **ADD** - --**ADD ... ** The ADD instruction copies new files, directories + --ADD has two forms: + **ADD ... ** + **ADD [""... ""]** 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 . Mutliple resources may be specified but if they are files or directories then they must be relative to the source directory that is being built @@ -141,7 +145,11 @@ or and gid of 0. **COPY** - --**COPY ** The COPY instruction copies new files from and + --COPY has two forms: + **COPY ... ** + **COPY [""... ""]** This form is required for paths containing + whitespace. + The COPY instruction copies new files from and adds them to the filesystem of the container at path . The 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 `` is an diff --git a/docs/sources/reference/builder.md b/docs/sources/reference/builder.md index fa7393efa3..d8e99613e0 100644 --- a/docs/sources/reference/builder.md +++ b/docs/sources/reference/builder.md @@ -381,7 +381,11 @@ change them using `docker run --env =`. ## ADD - ADD ... +ADD has two forms: + +- `ADD ... ` +- `ADD [""... ""]` (this form is required for paths containing +whitespace) The `ADD` instruction copies new files, directories or remote file URLs from `` and adds them to the filesystem of the container at the path ``. @@ -481,7 +485,11 @@ The copy obeys the following rules: ## COPY - COPY ... +COPY has two forms: + +- `COPY ... ` +- `COPY [""... ""]` (this form is required for paths containing +whitespace) The `COPY` instruction copies new files or directories from `` and adds them to the filesystem of the container at the path ``. diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index a3d2464d0b..0e7cd5a86c 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -762,7 +762,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) { @@ -770,7 +770,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", @@ -782,18 +782,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", @@ -805,18 +828,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", @@ -828,12 +874,35 @@ 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) @@ -913,7 +982,7 @@ func TestBuildAddMultipleFilesToFileWithWhitespace(t *testing.T) { defer deleteImages(name) ctx, err := fakeContext(`FROM busybox ADD [ "test file1", "test file2", "test" ] - `, + `, map[string]string{ "test file1": "test1", "test file2": "test2", @@ -925,7 +994,7 @@ func TestBuildAddMultipleFilesToFileWithWhitespace(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 with whitespace") @@ -948,7 +1017,7 @@ func TestBuildCopyMultipleFilesToFileWithWhitespace(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 with whitespace")