Skip to content

Change error message for unreadable files#1053

Merged
thaJeztah merged 1 commit intodocker:masterfrom
justyntemme:patch-3
May 21, 2018
Merged

Change error message for unreadable files#1053
thaJeztah merged 1 commit intodocker:masterfrom
justyntemme:patch-3

Conversation

@justyntemme
Copy link
Copy Markdown
Contributor

Fix for docker/for-linux#44

- What I did
Change the error message for unreadable files to mention the possibility of a .Dockerignore entry for multi developer teams.
- How I did it
Simply changing the text in the error message
- How to verify it
Run docker and try to COPY a file with a .Dockerignore entry
- Description for the changelog

Changed error message for unreadable files to clarify possibility of a .Dockerignore entry

- A picture of a cute animal (not mandatory but encouraged)

if err != nil {
if os.IsPermission(err) {
return errors.Errorf("can't stat '%s'", filePath)
return errors.Errorf("can't stat: file not found or excluded by .dockerignore '%s'", filePath)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem right. This is a permission error, not a not found error.
In addition, we should just wrap the real error errors.Wrap(err, "error reading file"). Generally the file name should already be in the error.

Fix for docker/for-linux#44

Signed-off-by: Justyn Temme <justyntemme@gmail.com>
@justyntemme
Copy link
Copy Markdown
Contributor Author

I have updated the PR to the correct error which should be if os.IsNotExist

Copy link
Copy Markdown
Contributor

@vieux vieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah thaJeztah merged commit 9a89c32 into docker:master May 21, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.06.0 milestone May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants