Improve error messages for non-existing or excluded files

[thaJeztah]

Relates to moby/moby#41327 (moby/moby#41327 (comment)) and moby/moby#34986

Docker produces a confusing error message when building a Dockerfile that ADD or COPY's a file that doesn't exist or is excluded by .dockerignore.

Example 1; when copying from a local source:

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

Example 2; when copying from a stage or image:

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

As can be seen in the output above, the error returned to the user includes 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. I frequently see users being confused by this (paths in Dockerfiles can already be confusing, and sometimes users expect them to be absolute paths on the host, so these error messages make that even more confusing).

BuildKit should produce a more useful error message for these situations;

• If possible, omit the failed to walk error, and instead print an error about the actual file (file.txt) that is attempted to be COPY/ADD-ed
• Print the path relative to the build-context (/some/non-existing/file.txt) instead of the physical path (/var/lib/docker/overlay2/....)
• In situations where the file may not be found because it was excluded by .dockerignore (example 1), add a "hint" to the error message that the file may exist but could be excluded by .dockerignore
  • Ideally, BuildKit would detect that it was excluded (not "missing"), but from a conversation with @tonistiigi, this is technically not possible

Opening this ticket to track this enhancement. I haven't looked into the code yet, and may not be able to look at that soon, so whoever wants to work on this, feel free to!

[thaJeztah]

/cc @tonistiigi @AkihiroSuda

[tonistiigi]

Ideally, BuildKit would detect that it was excluded (not "missing"), but from a conversation with @tonistiigi, this is technically not possible

You could write a pre-validation step that checks .dockerignore contents against the paths in the COPY statements before the actual LLB build starts and fail early. Seems a bit dangerous but don't think there is a way it would cause a false positive.

[thaJeztah]

Also reported in; docker/cli#2690

[thaJeztah]

/cc @tiborvass