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

Windows Store installation of Python adds a pip.ini file which configures --user #103646

Open
pfmoore opened this issue Apr 20, 2023 · 9 comments
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@pfmoore
Copy link
Member

pfmoore commented Apr 20, 2023

Bug report

See pypa/pip#11982 for background. Basically, pip added a mechanism for users to configure the "base" (installation-wide) settings. This mechanism picks up a pip.ini file that is shipped with the Windows Store installation of Python. The problem is that the shipped config file sets the --user option globally, meaning that it gets picked up in virtual environments as well, which causes pip to fail.

IMO, the pip feature needs reconsidering, to allow for the possibility of something like this, but it's also true that we advise against setting --user in configuration files. So I think the Windows Store distribution should consider other mechanisms for handling whatever issue prompted the addition of this config. If that requires changes from pip's side, please reach out - we already have a check if site-packages is writeable and we fall back to --user if it isn't, so we could definitely consider extending that check.

Linked PRs

@pfmoore pfmoore added the type-bug An unexpected behavior, bug, or error label Apr 20, 2023
@zooba
Copy link
Member

zooba commented Apr 20, 2023

When did pip start looking in sys.base_prefix for configuration files? Presumably in 23.1, since this has never been reported before.

What's the best way to go back to the old behaviour of only looking in sys.prefix?

@zooba
Copy link
Member

zooba commented Apr 20, 2023

And the issue that prompted this option is that site-packages is always unwritable, and versions of pip at the time would simply fail. It's entirely possible that this particular case would work fine with the updated checks, though we can't remove the config file from any old releases, or any that shipped with a version of pip without the checks.

Other scenarios also benefit from using --user by default. For example, I know of at least two systems where default site-packages is writable, but if anything ever gets written there then the whole build process is aborted and on-call people get automated phone calls. So we put the config file into that CPython package so it never happens, even if someone forgets to pass --user (or they're using a wrapper around pip that doesn't even allow it to be passed).

Using a different filename for BASE config files, or require a allow_as_base=True option in any config file found by that scheme, would be a fine way.

Really, we probably need to get a lot clearer on whether a venv is isolation from the base environment or not. Historically, it's always been isolation as far as configuration goes. Changing that is a big deal.

@pfmoore
Copy link
Member Author

pfmoore commented Apr 20, 2023

Yes, this is new behaviour in pip 23.1 (we added a "base" config location, which uses sys.base_prefix, as well as the previous locations.

What's the best way to go back to the old behaviour of only looking in sys.prefix?

There isn't one. I'm planning on reverting this change temporarily while we figure out a better fix, but this was a deliberate change for a particular use case, so something like this will probably get added.

And the issue that prompted this option is that site-packages is always unwritable, and versions of pip at the time would simply fail.

That's perfectly typical, it's how Unix distros work. Pip automatically switches to user installs if site-packages is unwriteable, so this workaround shouldn't be needed (if it is, then that's a pip bug that we'd rather have reported so we can fix it). I don't recall when we added that behaviour - it's possible the store distribution put the config file hack in before then.

It's entirely possible that this particular case would work fine with the updated checks, though we can't remove the config file from any old releases, or any that shipped with a version of pip without the checks.

We may need to advise users to remove the file manually if they use an upgraded pip, but yes, we can't do much about old releases.

So we put the config file into that CPython package so it never happens, even if someone forgets to pass --user (or they're using a wrapper around pip that doesn't even allow it to be passed).

IMO, that's the wrong approach. The systems you mention might need something like this1, but making it a part of the default install is not warranted just because some systems have an issue. Pip's config files are for the user to manage, not for the distribution's use, so letting the users create such a config is fine, but shipping a config with Python isn't.

Really, we probably need to get a lot clearer on whether a venv is isolation from the base environment or not. Historically, it's always been isolation as far as configuration goes. Changing that is a big deal.

Agreed. Having some sort of CPython policy on that would be extremely useful. But there's always --system-site-packages, so just making a statement like "isolation is total" is not enough. You should also read the discussion in pypa/pip#9752 - while I don't particularly agree with the approach taken by the OP there, it's sufficiently valid that it persuaded other pip maintainers to approve this change.

One further thought - we've had enough trouble with users setting --user in pip config files, that I don't think it's completely unthinkable that pip might at some point remove support for doing so completely. While this issue shows that there would be compatibility problems with doing so, I do think we'd take the view that the config files are for the user or site admin's use, not for the distribution. So regardless of what happens here, I don't think that shipping a pip configuration file is the right solution, and we do need to look for a better approach for the future.

Footnotes

  1. And they can consider the trade-offs involved in adding this, given pip's stance that forcing --user in a config file is bad practice.

@zooba
Copy link
Member

zooba commented Apr 20, 2023

We may need to advise users to remove the file manually if they use an upgraded pip, but yes, we can't do much about old releases.

The config file is also in the unmodifiable area, so users can't get rid of it. We'd have to make new CPython releases for 3.7+.

The systems you mention might need something like this1, but making it a part of the default install is not warranted just because some systems have an issue.

To be clear, it's part of the default install for that system. Nobody outside of that system gets the same install (unless they deliberately seek it out). The Store package isn't the package they use - they have their own private build and package of CPython.

So regardless of what happens here, I don't think that shipping a pip configuration file is the right solution, and we do need to look for a better approach for the future.

The "best" approach not involving pip here is to patch sysconfig to treat a different (per user) location as the main stdlib, and then add the fixed stdlib path into sys.path explicitly (so that users always have a copy of pip available, even when they blow away all their stored files). I don't know whether you think that's better or not, but I didn't, which is why I went for adapting pip instead. If it is considered to be better, then I can change it for 3.12, and we'll have to tell users somehow not to upgrade pip too far on earlier releases.

@pfmoore
Copy link
Member Author

pfmoore commented Apr 20, 2023

The config file is also in the unmodifiable area, so users can't get rid of it. We'd have to make new CPython releases for 3.7+.

Ouch. That seems like a really bad situation, shipping an unmodifiable copy of a configuration file specifically intended for user consumption :-( I guess we could advise users to override the global config in a user config file, but that's going to be less than ideal...

The Store package isn't the package they use - they have their own private build and package of CPython.

OK, that's fine. For a custom build they can (and should) do what suits their use case. But for a general-purpose build like the Store package, different rules apply (IMO).

I don't know whether you think that's better or not, but I didn't, which is why I went for adapting pip instead. If it is considered to be better, then I can change it for 3.12, and we'll have to tell users somehow not to upgrade pip too far on earlier releases.

Personally, I think that pip's built in behaviour of switching to user installs if site-packages isn't writeable should be sufficient, and we should just remove the config file for all releases that ship with a version of pip recent enough to have that behaviour. Although obviously, I'd advise that we check that the behaviour does work as expected (I don't think anyone has tested it with the Store Python, and if we need to refine the check for writeability we can do that).

More generally, I'd rather the user experience with Store Python work more like Linux distros (which have had read-only site-packages for a long time now). So my initial question with any proposal is "do Linux distros do it like this", followed by "why can't you do the same as Linux" if the answer was "no"...

@zooba
Copy link
Member

zooba commented Apr 20, 2023

The Linux distros don't, because they want their user site-packages to be for system packages as well. Plus they have a step where they modify the Python sources before shipping, while we don't.

But I think you're right that the writeability check should handle it (assuming you're literally just trying to write a file, and not doing anything more "clever"). When I get my current work cleared, I'll have a look at it.

@pfmoore
Copy link
Member Author

pfmoore commented Apr 20, 2023

Update Ignore what's below, I think setting $env:PIP_USER overrides the "do we default to --user?" logic. But I have no other means of testing as I can't remove or even rename the pip.ini file... So you'll have to test this yourself, using whatever means you have to set up an installation that doesn't have that file present.


[Left here as evidence of what I tried, even though it's wrong]

Hmm, it looks like the permissions check doesn't work for the Store Python:

❯ $env:PIP_USER = 0
❯ python3.11 -m pip install editables
Collecting editables
  Using cached editables-0.3-py3-none-any.whl (4.7 kB)
Installing collected packages: editables
ERROR: Could not install packages due to an OSError: [WinError 5] Access is denied: 'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.11_3.11.1008.0_x64__qbz5n2kfra8p0\\Lib\\site-packages\\editables'
Consider using the `--user` option or check the permissions.

I'm not sure why, as pip's test_writable_dir works as expected:

>>> from pip._internal.utils.filesystem import test_writable_dir
>>> import sys
>>> sys.path
[
    '',
    'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.11_3.11.1008.0_x64__qbz5n2kfra8p0\\python311.zip',
    'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.11_3.11.1008.0_x64__qbz5n2kfra8p0\\DLLs',
    'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.11_3.11.1008.0_x64__qbz5n2kfra8p0\\Lib',
    'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.11_3.11.1008.0_x64__qbz5n2kfra8p0',
    'C:\\Work\\Scratch\\.v1',
    'C:\\Work\\Scratch\\.v1\\Lib\\site-packages',
    'C:\\Work\\Projects\\pip\\src',
    'C:\\Users\\Gustav\\.local\\config\\pyrepl-deps.zip'
]
>>> test_writable_dir('C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.11_3.11.1008.0_x64__qbz5n2kfra8p0\\Lib')
False

But that's "just" a bug in pip, which we can (and should) solve...

@zooba
Copy link
Member

zooba commented May 29, 2023

The pip.ini should no longer be included in Store releases as of 3.12.0b2 (this week), which I achieved by updated --preset-appx to no longer imply --include-pip-user.

I suspect there's next-to-no usage of the --include-pip-user option, so it's likely fine to remove it entirely. But I don't think there's any hurry. If anyone thinks there is a hurry, we can get onto it. Otherwise, it can sit there for a while.

@pfmoore
Copy link
Member Author

pfmoore commented May 29, 2023

Sounds reasonable to me. I didn't realise the code was used outside of the CPython release process, but yes, if it might be, we should be cautious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants