Skip to content

Revert "dpkg-divert" and change PATH instead #137

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

Merged
merged 2 commits into from
Aug 8, 2016

Conversation

yosifkit
Copy link
Member

@yosifkit yosifkit commented Aug 5, 2016

Reverting and changing #131 which was the fix for #129. Also fix some spaces to tabs like the rest of the file.

Fix #133, fix #134.

 - this reverts commit a819c92.
 - since Debian packages use an absolute path, they were broken by the diverts
 - ensure the local python is first in PATH
@JayH5
Copy link
Contributor

JayH5 commented Aug 8, 2016

FWIW, these images already have /usr/local/bin in their PATHs before /usr/bin. debian:jessie, buildpack-deps:jessie and buildpackdeps-wheezy all have a (very standard) PATH of /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin. But I guess you're wanting to make that explicit...

@tianon
Copy link
Member

tianon commented Aug 8, 2016

@JayH5 indeed, the point is to be 100% explicit to ensure that by default, python resolves to what we've created/added no matter what happens 👍

LGTM

@tianon tianon merged commit 2f0d83d into docker-library:master Aug 8, 2016
@tianon tianon deleted the everythings-shiny-capn branch August 8, 2016 16:58
tianon added a commit to infosiftr/stackbrew that referenced this pull request Aug 8, 2016
- `docker`: `${DOCKER_VERSION}` (cosmetic; docker-library/docker#16)
- `java`: Debian 9~b130
- `php`: note that `Dockerfile`s are generated (docker-library/php@e36077f)
- `python`: use `PATH` instead of `dpkg-divert` or `purge` (docker-library/python#137)
- `rocket.chat`: 0.36.0
JayH5 added a commit to praekeltfoundation/docker-alpine-python that referenced this pull request Aug 10, 2016
@darabos
Copy link

darabos commented Aug 11, 2016

Would it make sense to add automated tests for this issue? Python is one of the most popular images: an issue like this has a fairly large impact.

@yosifkit
Copy link
Member Author

@darabos, that sounds like a great idea. Do you have an idea of what would be the best indicator? Maybe just test that /usr/bin/python --version is Python 2.7.9? I am not sure what we are trying to test for so I am unsure where the test needs to fail.

We do have tests that we run from travis that are also part of accepting it into official-images for building and pushing to the hub: https://github.com/docker-library/official-images/tree/master/test.

@darabos
Copy link

darabos commented Aug 12, 2016

Sorry, I'm just a clueless user with no deep understanding of the issue. We just experienced that apt-get install -y openjdk-7-jre was failing.

Thanks for the link, I never knew about those tests! It looks like debian-apt-get/container.sh already tests apt-get install with a toy package. Would it be crazy to test with a more substantial package, like openjdk-7-jre?

Just testing /usr/bin/python --version like you propose would definitely be faster and more reliable. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Newest revision broke compatibility /usr/bin/python: bad interpreter: No such file or directory
4 participants