-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove duplicate "not found" from some error messages #2047
Conversation
I noticed this when building a Dockerfile that failed because a file didn't exist, so went through error messages that looked like they had a duplicate "not found" in the output; [+] Building 0.9s (6/9) => [internal] load build definition from Dockerfile 0.2s => => transferring dockerfile: 306B 0.0s => [internal] load .dockerignore 0.1s => => transferring context: 2B 0.0s => [internal] load metadata for docker.io/library/alpine:latest 0.0s => CACHED [1/5] FROM docker.io/library/alpine 0.0s => [internal] load build context 0.6s => => transferring context: 701B 0.5s => ERROR [2/5] ADD no-such-file.txt / 0.0s ------ > [2/5] ADD no-such-file.txt /: ------ failed to compute cache key: "/no-such-file.txt" not found: not found Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@@ -610,7 +610,7 @@ func (cc *cacheContext) checksum(ctx context.Context, root *iradix.Node, txn *ir | |||
return nil, false, err | |||
} | |||
if cr == nil { | |||
return nil, false, errors.Wrapf(errNotFound, "%q not found", convertKeyToPath(origk)) | |||
return nil, false, errors.Wrapf(errNotFound, "%q", convertKeyToPath(origk)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept the %q
formatting here, but perhaps it's not needed, and could be changed to
return nil, false, errors.Wrapf(errNotFound, "%q", convertKeyToPath(origk)) | |
return nil, false, errors.Wrap(errNotFound, convertKeyToPath(origk)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually; I think the %q
could be useful; looking at the original error;
failed to compute cache key: "/no-such-file.txt" not found: not found
Which would now become
failed to compute cache key: "/no-such-file.txt": not found
Removing the quotes would make it;
failed to compute cache key: /no-such-file.txt: not found
Which "works" for simple cases, but possibly in other cases, e.g. empty filename, or path with spaces, would be not so clear.
I guess a cleaner way is to use a typed error with a property for the path. Not found cases can also use stdlib |
Ah, sorry, didn't come round to looking into that; let me know if you still want me to do so |
I noticed this when building a Dockerfile that failed because a file didn't
exist, so went through error messages that looked like they had a duplicate
"not found" in the output;