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

wants to merge 1 commit into from

Conversation

matthew-brett
Copy link
Collaborator

Experimenting with forcing PYTHON_EXE PIP_CMD

@matthew-brett matthew-brett mentioned this pull request Mar 3, 2020
Merged
@matthew-brett matthew-brett changed the title WIP: Do not merge MRG: force use of PYTHON_EXE, PIP_CMD Mar 3, 2020
@mattip
Copy link
Collaborator

mattip commented Mar 8, 2020

This is still needed since install_pypy does not set a symbolic link pypy -> python so numpy-wheels pypy build is failing.

@matthew-brett
Copy link
Collaborator Author

I think it's reasonable to always do:

py_exe=${PYTHON_EXE:-python}

This was referenced Mar 17, 2020
Open
Merged
matthew-brett added a commit that referenced this pull request Mar 17, 2020
@matthew-brett
Merge pull request #323 from mattip/specific-python-pip2
1a9ddbe 
MRG: Specify python, pip using PYTHON_EXE, PYTHON _EXE -mpip

Replaces gh-315: rebased and fixed one more occurrence

Related to MacPython/pandas-wheels#79
@mattip
Copy link
Collaborator

mattip commented Apr 15, 2020

I ran into this on macOS when trying to use latest devel. @matthew-brett would you mind if I take over to update this?

@matthew-brett
Copy link
Collaborator Author

Yes, please, go ahead.

@mattip
Copy link
Collaborator

mattip commented Apr 15, 2020

It turns out much of this was already merged, I think in gh-314. All that is left is to set PIP_CMD on macOS in the path where it was missing (using a native python with no virtualenv). I also took the opportunity to change from pip to python -m pip which solves all the issues around pip vs. pip3. What do you think of using python -m ensurepip instead of curl get-pip.py; python get-pip.py? It is supported on pyhon 2.7 and from 3.3 up.

@matthew-brett
Copy link
Collaborator Author

matthew-brett commented Apr 15, 2020 via email

@mattip mattip force-pushed the specific-python-pip branch 2 times, most recently from c697bd4 to a75c39e Compare April 28, 2020 23:09
@mattip
Copy link
Collaborator

mattip commented Apr 29, 2020

Many of the macOS 3.5 builds are failing with

The 'pip==19.3.1' distribution was not found and is required by the application

Does that have something to do with this PR?

@mattip mattip mentioned this pull request Apr 29, 2020
Merged
@matthew-brett
Copy link
Collaborator Author

Can you replicate in a virtualenv?

@mattip
Copy link
Collaborator

mattip commented Apr 29, 2020

ensurepip seems reasonable ...

Dropped this, it seems to break things and should be in a different PR.

Can you replicate in a virtualenv?

I don't have access to a macOS system

@matthew-brett
Copy link
Collaborator Author

It doesn't happen on Linux?

@mattip
Copy link
Collaborator

mattip commented Apr 29, 2020

As far as I can tell the only missing use case is setting PIP_CMD on macos when not using a virtualenv. I dropped trying to move to ensurepip since it seems some of the installed pythons (again, only on macos) need to do pip install --upgrade pip setuptools anyway, so might as well stay with get-pip,

@mattip
Copy link
Collaborator

mattip commented Apr 30, 2020

Tests are passing

@mattip
Copy link
Collaborator

mattip commented May 5, 2020

@matthew-brett any thoughts here?

matthew-brett
matthew-brett commented May 5, 2020
View reviewed changes
osx_utils.sh
@@ -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.

@mattip
Copy link
Collaborator

mattip commented Jul 23, 2020

Closing. Please reopen if I am mistaken

@mattip mattip closed this Jul 23, 2020
@mattip mattip mentioned this pull request Jan 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattip mattip mattip left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matthew-brett @mattip