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.

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

[WIP] Implement --global and try to default to --user when it makes sense #2418

Closed
wants to merge 1 commit into from

Conversation

dstufft
Copy link
Member

@dstufft dstufft commented Feb 10, 2015

Fixes #1668 by implementing:

  • When --user is passed we always install into the user site packages.
  • When --global is passed we always install into the global site packages.
  • When neither is passed we implement a fallback method which:
    1. Detects if we're in a virtual environment and if so acts as if --global was passed.
    2. Detects if user site-packages is disabled and if so acts as if --global was passed.
    3. Looks at default-install and if that exists uses --user or -global based on it.
    4. Detects if the effective user does not have write permissions to global site packages, and if they do not act as if --user was passed.
    5. Finally, if all other detection methods failed, act as if --global was passed.
  • Display a warning when installing a script into --user and the user bin isn't on $PATH.

Implementation TODOs:

  • Make sure that our detection logic works when one of the directories does not exist.
  • Figure out what our detection logic should do when things like --root and such are passed.
  • Write tests.

This is currently untested and is just a first pass but I figured I'd throw it up here for people to see.

Review on Reviewable

@dstufft dstufft mentioned this pull request Feb 10, 2015
# --isolated and such options as well.
# TODO: Figure out if this works when the directories don't exist.
for path in distutils_scheme("").values():
if not os.access(path, os.W_OK):
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to behave properly on Windows. os.access(r'C:\Windows', os.W_OK) is True for me, but open(r'C:\Windows\test.file', 'w').close() fails with errors on 2.7 and 3.4.

I was expecting this check to happen at a higher level - about the point where pip would terminate with an error but instead retry with --user.

Copy link
Member Author

Choose a reason for hiding this comment

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

Retrying with --user seems like the wrong answer to me to be honest. It sounds like it'd be easy to lead to half way installed things and it would be a lot more fragile. If os.access doesn't work we can probably do something based on trying to open file with w to test it.

Copy link
Contributor

Choose a reason for hiding this comment

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

How soon do we know the destination subfolder? Can we switch when trying to create that (and maybe bring creation sooner if necessary)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, but thinking about it more the other side of this is that we don't want pip install foo and pip install bar to install to different locations, so this should ideally be a check that is independent of the thing we're installing.

Copy link
Member

Choose a reason for hiding this comment

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

os.access currently always returns True for directories on Windows: http://bugs.python.org/issue2528

@dstufft
Copy link
Member Author

dstufft commented Feb 11, 2015

Oh, we'll want to figure out what happens if you do sudo pip install --global requests followed by a pip install --user requests. Will pip try to uninstall the global requests? I'm thinking it should probably just leave it alone.

@pfmoore
Copy link
Member

pfmoore commented Feb 11, 2015

Probably leave it alone, yes. But what then if you do pip uninstall requests? You'll get the global one back, and you'll need to uninstall again to fully get rid of it. If you want to get rid of the global install you can do `pip uninstall --global requests`` which presumably would leave the user install alone.

But scripts probably want a way to remove all traces of a package. I guess doing both global and user uninstalls is OK, but checking for errors could be annoying (if there's a good chance that one or other of the global and user uninstalls will fail because the package isn't installed).

So:

  1. pip uninstall might need --global and --user flags. At a minimum there needs to be a way to uninstall the global installation when there's a user install you want to preserve.
  2. pip uninstall without either uninstalls the first of user or global that it finds? (Or maybe it uninstalls all copies in that case, if you have flags to select the specific locations?)

@pfmoore
Copy link
Member

pfmoore commented Feb 11, 2015

Also, suppose I do pip install --user foo==1.0 and then pip install --global foo==1.1. That could happen in a few ways (the global install is done by a different user, sudo means that the user's site directory isn't visible, etc).

Questions:

  1. Is it OK that the user installed 1.1, but Python still sees 1.0?
  2. If the user then does pip install --user bar and bar depends on foo==1.0, is that OK? Is it still OK if the user installs bar globally?
  3. If global has 1.0 and user has 1.1, and we install bar (which depends on 1.1) globally, this needs to fail, as otherwise other users will see a broken bar installation.

I have a feeling that allowing there to ever be 2 different versions of a package installed will come with a lot of odd edge cases. But of course, we have to allow that as otherwise a user without access to the global location won't be able to upgrade a globally-installed package.

Please note - I've deliberately not used sudo here. If you do sudo pip install --global you get a completely different set of conflicts, as the user location that's visible is root's rather than your own (which is hopefully empty). The sudo case may be easier to reason about (largely because root's user site will hopefully be empty) but the non-sudo case will be what happens on Windows.

The answer to all of these questions might well be just to write the code naturally, and wait for bug reports - that may be the only way of getting meaningful feedback on what are real issues, as opposed to theoretical risks.

@dstufft
Copy link
Member Author

dstufft commented Feb 11, 2015

Yea they are certainly edge cases we're likely to run into, especially since i don't know how much --user is used right now. I do think though that it's existed for long enough that we're unlikely to get more people using it without pushing it forward and then dealing with what breaks from it.

To answer your questions:

I think that we're not going to get around having multiple versions of the same thing installed and only one version visible at a time. This is actually already the case on Debuntu systems where apt-get will install into /usr/lib/pythonX.Y/dist-packages and pip will install into /usr/local/lib/pythonX.Y/.dist-packages and /usr/local takes precedence.

I do think we need to ensure that all of the dependencies are satisfied at an equal level or later level in the hierarchy. What I mean is that pip install --global foo should make sure that all of it's dependencies are available in global (or eventually vendor if I get that into Python) and it should ignore --user for the purposes of doing that install. However a package installed with pip install --user foo should be able to get dependencies from --global and --vendor too.

It might make sense when doing pip install --global foo if foo or one of its dependencies is going to be shadowed by --user, however I don't think that's a mandatory feature, more of a nice to have. It might also make sense to add a similar check to the script wrappers (although it might not, our script wrappers are a lot faster than setuptools because we don't have any of that type of code executing when you run them).

@piotr-dobrogost
Copy link

@dstufft

However a package installed with pip install --user foo should be able to get dependencies from --global and --vendor too.

Did you mean (...) from --global and --local too.?


# If any of our potentional locations for writing files is not writable by
# us then we want to use the --user scheme instead of the --global scheme.
# TODO: We should figure out a way to make this work for --root and
Copy link
Contributor

Choose a reason for hiding this comment

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

if someone intentionally specifies a --root, but they don't have permissions, it should just fail I think, and never switch to a user install. same thing when specifying custom --install-option arguments.

note that our distutils_scheme function reads custom locations from distutils config files, so with your implementation as is, you will be switching users to a user install if any of those custom paths are not writable, which I think is misleading.

@msabramo
Copy link
Contributor

msabramo commented Mar 6, 2015

Should this and #1668 have some kind of "pip 7" tag? I presume this wouldn't be merged now while there are still 6.x releases planned and I believe I saw that 6.1.0 is the next planned release?

@dstufft
Copy link
Member Author

dstufft commented Mar 6, 2015

6.1 is the next planned only because nothing has been merged that would warrant a 7.0.

@msabramo
Copy link
Contributor

msabramo commented Mar 6, 2015

When neither is passed we implement a fallback method which:
Detects if we're in a virtual environment and if so acts as if --global was passed.

Why would it act like --global when no flags were given and they're in a virtualenv? That wouldn't be what I would expect, so maybe I'm not understanding this feature the right way.

I would think that if you're in a virtualenv, it installs to the virtualenv by default always and only does something else if you pass --user or --global.

@dstufft
Copy link
Member Author

dstufft commented Mar 6, 2015

Inside of a virtual environment --global is $VENV/lib/pythonX.Y/site-packages. These are the paths we inspect from the environment and the virtual environment changes the global path.

@msabramo
Copy link
Contributor

msabramo commented Mar 6, 2015

Wouldn't hurt to add "7.0" and "Improve our User Experience" milestones?

@dstufft dstufft modified the milestones: 7.0, Improve our User Experience Mar 6, 2015
@dstufft
Copy link
Member Author

dstufft commented Mar 6, 2015

It can only be in one milestone, so I added it to 7.0.

@msabramo
Copy link
Contributor

msabramo commented Mar 6, 2015

Oh so --global inside a virtualenv means "in the virtualenv"?

That is unintuitive to me. Especially when one of the items above says:

When --global is passed we always install into the global site packages.

But I guess if it makes sense if global in a virtualenv means "not user but global within the virtualenv"

But that seems super confusing. What terminology do use to distinguish "global in the virtualenv" to "global global" ("system global"?)

@msabramo
Copy link
Contributor

msabramo commented Mar 6, 2015

I guess I was thinking that there are 3 places a package can be installed.

  • system/global
  • user home directory
  • virtualenv

But maybe that is the wrong model.

@msabramo
Copy link
Contributor

msabramo commented Mar 6, 2015

It sounds like maybe "global" the way you use the term here means "non-user" (not in the user's directory). I think I assumed "global" to mean "system". Is that right?

@zooba
Copy link
Contributor

zooba commented Mar 6, 2015

"global" here (AIUI) basically means sysconfig.get_paths('nt') or sysconfig.get_paths('posix_prefix'), while "user" means sysconfig.get_paths('nt_user') or sysconfig.get_paths('posix_user').

action='store_false',
help='Install into the global site packages.')

cmd_opts.add_option(
'--egg',
Copy link
Member

Choose a reason for hiding this comment

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

I don't know enough about how command options translate to environment variables and/or config file entries. Does this mean that PIP_GLOBAL or a global setting in the config file would make global the default (once the hard-coded default becomes "user")? If not, how would someone say they wanted to have global as the default?

@dstufft dstufft modified the milestones: 8.0, 7.0 May 9, 2015
@dstufft dstufft added this to the 8.1 milestone Jan 19, 2016
@dstufft dstufft removed this from the 8.0 milestone Jan 19, 2016
@dstufft dstufft removed this from the 8.1 milestone Mar 3, 2016
@dstufft dstufft closed this May 18, 2016
@dstufft
Copy link
Member Author

dstufft commented May 18, 2016

Accidentally closed this, reopening. Sorry!

@dstufft dstufft reopened this May 18, 2016
@BrownTruck
Copy link
Contributor

Hello!

As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the master branch. However, this Pull Request was made against the develop branch so it will need to be resubmitted against master. Unfortunately, this pull request does not cleanly merge against the current master branch.

If you do nothing, this Pull Request will be automatically closed by @BrownTruck since it cannot be merged.

If this pull request is still valid, please rebase it against master (or merge master into it) and resubmit it against the master branch, closing and referencing the original Pull Request.

If you choose to rebase/merge and resubmit this Pull Request, here is an example message that you can copy and paste:

Fixes #1668 by implementing:



 When --user is passed we always install into the user site packages.

 When --global is passed we always install into the global site packages.

 When neither is passed we implement a fallback method which:



 Detects if we're in a virtual environment and if so acts as if --global was passed.

 Detects if user site-packages is disabled and if so acts as if --global was passed.

 Looks at default-install and if that exists uses --user or -global based on it.

 Detects if the effective user does not have write permissions to global site packages, and if they do not act as if --user was passed.

 Finally, if all other detection methods failed, act as if --global was passed.



 Display a warning when installing a script into --user and the user bin isn't on $PATH.


Implementation TODOs:



 Make sure that our detection logic works when one of the directories does not exist.

 Figure out what our detection logic should do when things like --root and such are passed.

 Write tests.


This is currently untested and is just a first pass but I figured I'd throw it up here for people to see.

---

*This was migrated from pypa/pip#2418 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*

@BrownTruck BrownTruck added asked to reparent auto-bitrotten PRs that died because they weren't updated labels May 19, 2016
@BrownTruck
Copy link
Contributor

This Pull Request was closed because it cannot be automatically reparented to the master branch and it appears to have bit rotted.

Please feel free to re-open it or re-submit it if it is still valid and you have rebased it onto master or merged master into it.

@BrownTruck BrownTruck closed this May 26, 2016
@dstufft dstufft deleted the default-user branch April 2, 2017 15:43
@dstufft dstufft restored the default-user branch April 2, 2017 15:43
@pradyunsg
Copy link
Member

Would it be fine if I try to take this forward?

@dstufft
Copy link
Member Author

dstufft commented Jun 14, 2017

Sure

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-bitrotten PRs that died because they weren't updated auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants