-
Notifications
You must be signed in to change notification settings - Fork 237
Search for Python version also in /usr/lib/python #157
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
Path to Python is different on deb-based distributions and rpm-based distributions (e.g. Fedora, CentOS).
CLAs look good, thanks! |
differs/pip_diff.go
Outdated
libPath := filepath.Join(pathToLayer, "usr/local/lib") | ||
libContents, err := ioutil.ReadDir(libPath) | ||
if err != nil { | ||
return matches, err | ||
logrus.Warnf("Could not find /usr/local/lib to determine Python version, trying /usr/lib: %s", err) |
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.
This might read a little bit cleaner if you move all of this into a for loop. Something like:
// First gather all entries from both search paths
libPaths := []string{"usr/local/lib", "usr/lib"}
libContents := []os.FileInfo{}
for _, lp := range libPaths {
libPath := filepath.Join(pathToLayer, lp)
entries, err := ioutil.ReadDir(libPath)
if err != nil {
logrus.Warnf(...)
continue
}
libContents = append(libContents, entries)
}
// Then look through the appended contents
for _, file := range libContents {
...
}
What do you think?
…into fridex-pip-fedora
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
ebbdff7
to
973d9af
Compare
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.
Made the swap to a for loop. Now it should find all the different python versions from all locations.
Thanks @dlorenc. This was on my TODO list, but I didn't find time to modify according to review comments. Thanks for refactoring! |
Path to Python is different on deb-based distributions and rpm-based
distributions (e.g. Fedora, CentOS).