-
Notifications
You must be signed in to change notification settings - Fork 3k
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 pip install --target #4111
Fix pip install --target #4111
Conversation
This needs a rebase. |
@@ -266,6 +276,7 @@ def run(self, options, args): | |||
"continue." | |||
) | |||
install_options.append('--home=' + temp_target_dir) | |||
install_options.append('--prefix=') |
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.
Why add --prefix
here?
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.
I added the same thing as already exists for the --user
command (line 260).
An empty prefix
is set in order to overwrite one set as a default somewhere else (e.g. as it is the case with homebrew python).
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 branch should be dead because of exception above. Filled #4132
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.
Ok. Tests are failing. Looks like this piece of code is still needed.
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.
To my understanding the exception only avoids both options being set at the pip command line.
This line ensures that a prefix
set somwhere else is effectively overriden as it does basically the same as the workaround there:
https://github.com/Homebrew/brew/blob/master/docs/Homebrew-and-Python.md#note-on-pip-install---user
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.
I do not fully understand, how install_options
and distutils_scheme()
in locations.py do work together, but it seems to work this way.
I just wanted to add a follow-up to the discussion that started in #4103 . Regarding mutual exclusivity of the options: I would also suggest that we can assume that if one of the flags is given on the command line (or in a |
I'm interested in this change. Will we see it go forward any time soon? |
Is there anyone we should ping to review the code or something? |
Looking forward to this fix landing. |
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
Also override defaults set somewhere (e.g. .pydistutils.cfg) when command line switches are given.
Replace ``--no-use-wheel`` with ``--no-binary=:all:``
Replace --no-use-wheel with --no-binary=:all: and remove assertion of deprecation message.
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
For those who want to see this fixed, here is the way:
Even partial reports can help. |
This workaround fixed it for me: |
@halmi try to resolve conflict. It is just a few lines fix. |
This is now handled in #4557. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
As I messed up my branch, github force closed my last PR.
So this is a new, clean version of PR #4103 .
Sorry for the mess.
For reference: the original description of PR #4103:
This change is