From 010adeec55c2d817a26f1d8fe1382c49d12fdab1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 7 Aug 2020 21:31:31 +0200 Subject: [PATCH] Builder: print relative path if COPY/ADD source path was not found Before this change, the error returned to the user would include the physical path inside the tmp dir on the daemon host. These paths should be considered an implementation detail, and provide no value to the user. Printing the tmp path can confuse users, and will be even more confusing if the daemon is running remotely (or in a VM, such as on Docker Desktop), in which case the path in the error message does not exist on the local machine; echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- . Sending build context to Docker daemon 1.57kB Step 1/2 : FROM busybox ---> 1c35c4412082 Step 2/2 : COPY /some/non-existing/file.txt . COPY failed: stat /var/lib/docker/tmp/docker-builder405687992/some/non-existing/file.txt: no such file or directory When copying files from an image or a build stage, using `--from`, the error is similarly confusing: echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- . Sending build context to Docker daemon 4.671kB Step 1/2 : FROM busybox ---> 018c9d7b792b Step 2/2 : COPY --from=busybox /some/non-existing/file.txt . COPY failed: stat /var/lib/docker/overlay2/ef34239c80526c779b7afaeaedbf11c1b201d7f7681d45613102c4541da0e156/merged/some/non-existing/file.txt: no such file or directory This patch updates the error messages to be more user-friendly. Changes are slightly different, depending on if the source was a local path, or an image (or build-stage), using `--from`. If `--from` is used, only the path is updated, and we print the relative path instead of the full path; echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- . Sending build context to Docker daemon 1.583kB Step 1/2 : FROM busybox ---> 018c9d7b792b Step 2/2 : COPY --from=busybox /some/non-existing/file.txt . COPY failed: stat some/non-existing/file.txt: file does not exist In other cases, additional information is added to mention "build context" and ".dockerignore", which could provide the user some hints to find the problem: echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- . Sending build context to Docker daemon 1.583kB Step 1/2 : FROM busybox ---> 018c9d7b792b Step 2/2 : COPY /some/non-existing/file.txt . COPY failed: file not found in build context or excluded by .dockerignore: stat some/non-existing/file.txt: file does not exist echo -e "FROM busybox\nADD /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- . Sending build context to Docker daemon 1.583kB Step 1/2 : FROM busybox ---> 018c9d7b792b Step 2/2 : ADD /some/non-existing/file.txt . ADD failed: file not found in build context or excluded by .dockerignore: stat some/non-existing/file.txt: file does not exist This patch only improves the error for the classic builder. Similar changes could be made for BuildKit, which produces equally, or even more confusing errors; echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=1 docker build -f- . [+] Building 1.2s (6/6) FINISHED => [internal] load build definition from Dockerfile 0.0s => => transferring dockerfile: 85B 0.0s => [internal] load .dockerignore 0.0s => => transferring context: 2B 0.0s => [internal] load metadata for docker.io/library/busybox:latest 1.2s => [internal] load build context 0.0s => => transferring context: 2B 0.0s => CACHED [1/2] FROM docker.io/library/busybox@sha256:4f47c01... 0.0s => ERROR [2/2] COPY /some/non-existing/file.txt . 0.0s ------ > [2/2] COPY /some/non-existing/file.txt .: ------ failed to compute cache key: failed to walk /var/lib/docker/tmp/buildkit-mount181923793/some/non-existing: lstat /var/lib/docker/tmp/buildkit-mount181923793/some/non-existing: no such file or directory echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=1 docker build -f- . [+] Building 2.5s (6/6) FINISHED => [internal] load build definition from Dockerfile 0.0s => => transferring dockerfile: 100B 0.0s => [internal] load .dockerignore 0.0s => => transferring context: 2B 0.0s => [internal] load metadata for docker.io/library/busybox:latest 1.2s => FROM docker.io/library/busybox:latest 1.2s => => resolve docker.io/library/busybox:latest 1.2s => CACHED [stage-0 1/2] FROM docker.io/library/busybox@sha256:4f47c01... 0.0s => ERROR [stage-0 2/2] COPY --from=busybox /some/non-existing/file.txt . 0.0s ------ > [stage-0 2/2] COPY --from=busybox /some/non-existing/file.txt .: ------ failed to compute cache key: failed to walk /var/lib/docker/overlay2/2a796d91e46fc038648c6010f062bdfd612ee62b0e8fe77bc632688e3fba32d9/merged/some/non-existing: lstat /var/lib/docker/overlay2/2a796d91e46fc038648c6010f062bdfd612ee62b0e8fe77bc632688e3fba32d9/merged/some/non-existing: no such file or directory Signed-off-by: Sebastiaan van Stijn --- builder/dockerfile/copy.go | 6 ++++++ integration-cli/docker_cli_build_test.go | 7 +------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index d3613dcb3e..1982ec9bde 100644 --- a/builder/dockerfile/copy.go +++ b/builder/dockerfile/copy.go @@ -242,6 +242,8 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo, // Deal with the single file case copyInfo, err := copyInfoForFile(o.source, origPath) switch { + case imageSource == nil && errors.Is(err, os.ErrNotExist): + return nil, errors.Wrapf(err, "file not found in build context or excluded by .dockerignore") case err != nil: return nil, err case copyInfo.hash != "": @@ -315,6 +317,10 @@ func (o *copier) copyWithWildcards(origPath string) ([]copyInfo, error) { func copyInfoForFile(source builder.Source, path string) (copyInfo, error) { fi, err := remotecontext.StatAt(source, path) if err != nil { + if errors.Is(err, os.ErrNotExist) { + // return the relative path in the error, which is more user-friendly than the full path to the tmp-dir + return copyInfo{}, errors.WithStack(&os.PathError{Op: "stat", Path: path, Err: os.ErrNotExist}) + } return copyInfo{}, err } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 977271baee..1acc49106b 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2189,18 +2189,13 @@ func (s *DockerSuite) TestBuildEntrypointRunCleanup(c *testing.T) { func (s *DockerSuite) TestBuildAddFileNotFound(c *testing.T) { name := "testbuildaddnotfound" - expected := "foo: no such file or directory" - - if testEnv.OSType == "windows" { - expected = "foo: The system cannot find the file specified" - } buildImage(name, build.WithBuildContext(c, build.WithFile("Dockerfile", `FROM `+minimalBaseImage()+` ADD foo /usr/local/bar`), build.WithFile("bar", "hello"))).Assert(c, icmd.Expected{ ExitCode: 1, - Err: expected, + Err: "stat foo: file does not exist", }) }