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.

Sign up for GitHub

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

Jump to bottom

MRG: force use of PYTHON_EXE, PIP_CMD #315

Closed
wants to merge 1 commit into from
Closed

MRG: force use of PYTHON_EXE, PIP_CMD #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common_utils.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ function install_pip {
# Travis VMS now install pip for system python by default - force install
# even if installed already.
$PYTHON_EXE $get_pip_path --ignore-installed $pip_args
PIP_CMD=$(dirname $PYTHON_EXE)/pip$py_mm
PIP_CMD="$PYTHON_EXE -m pip"
if [ "$USER" != "root" ]; then
# inside a docker, there is no sudo but the user is already root
PIP_CMD="sudo $PIP_CMD"
Expand Down
1 change: 1 addition & 0 deletions osx_utils.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ function install_mac_cpython {
sudo installer -pkg $inst_path -target /
local py_mm=${py_version:0:3}
PYTHON_EXE=$MACPYTHON_PY_PREFIX/$py_mm/bin/python$py_mm
PIP_CMD="sudo $PYTHON_EXE -m pip"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this right, that it needs sudo? Won't it depend on whether this is a virtualenv or not?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The test in tests/test_python_install.sh checks sudo, which predates this change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's odd - no? I can't see how sudo would have got into the command in the code previous to these commits, outside the case where the user is root - can you?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In an unrelated PR, it is assigned on common_utils.sh:552 in the install_pip function. Going backward in the log it seems that comes from get_macpython_environment which calls install_pip which sets PIP_CMD with sudo.

So I guess this PR is not needed and should simply be closed.

# Install certificates for Python 3.6
local inst_cmd="/Applications/Python ${py_mm}/Install Certificates.command"
if [ -e "$inst_cmd" ]; then
Expand Down
2 changes: 1 addition & 1 deletion tests/test_python_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ else # not virtualenv
if [ "$PYTHON_EXE" != "$macpie_bin/python$python_mm" ]; then
ingest "Wrong macpython python cmd '$PYTHON_EXE'"
fi
if [ "$PIP_CMD" != "sudo $macpie_bin/pip${python_mm}${expected_pip_args}" ]; then
if [ "$PIP_CMD" != "sudo $PYTHON_EXE -m pip${expected_pip_args}" ]; then
ingest "Wrong macpython pip '$PIP_CMD'"
fi
fi
Expand Down