Skip to content
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

fix error when scanning folders for feteched update for Alpine vulnerabilities #299

Merged
merged 1 commit into from
Dec 30, 2016
Merged

Conversation

alexei-led
Copy link

Fix error updater: an error occured when fetching update 'alpine': open /private/tmp/alpine-secdb808866024/Makefile/main.yaml: not a directory.

This error happens on MacOS with go version go1.7.4 darwin/amd64.

For some strange reason File.Readdirnames returns names of directories and files. I've changed this code to use File.Readdir that returns []Fileinfo and thus adding additional validation that skips non-directories, using IsDir() method.

@jzelinskie
Copy link
Contributor

Thanks for the PR! Strange corner-case indeed.

Your commit seems to also make unrelated changes to the vendor directory, though.

@alexei-led
Copy link
Author

changes to vendor folder are not related indeed. I just run "glide install" to build and test clair and glide updated couple files in vendor folder. Ignore these, sorry.

@jzelinskie
Copy link
Contributor

If these changes are indeed pulled in by glide install, but are not currently apart of the vendor directory, we should commit them. However, they should be in a separate commit. Likewise if you'd like to add a .gitignore file, please put it in a separate commit. Having individual commits like this helps in the future when you need to use tools like git blame to remember why a specific change was made.

@alexei-led
Copy link
Author

alexei-led commented Dec 30, 2016

OK, fixed this PR to include only one change, that fixes described issue

Copy link
Contributor

@jzelinskie jzelinskie 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 for adjusting the PR!

@jzelinskie jzelinskie merged commit 7106f1c into quay:master Dec 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants