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

PEP 453: enable pip by default in the Windows binary installers #63927

Closed
ncoghlan opened this issue Nov 23, 2013 · 23 comments
Closed

PEP 453: enable pip by default in the Windows binary installers #63927

ncoghlan opened this issue Nov 23, 2013 · 23 comments
Assignees
Labels
release-blocker type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

BPO 19728
Nosy @loewis, @ncoghlan, @larryhastings, @ned-deily, @dstufft

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/loewis'
closed_at = <Date 2014-01-02.13:13:35.298>
created_at = <Date 2013-11-23.01:47:23.918>
labels = ['type-feature', 'release-blocker']
title = 'PEP 453: enable pip by default in the Windows binary installers'
updated_at = <Date 2014-01-02.13:13:35.297>
user = 'https://github.com/ncoghlan'

bugs.python.org fields:

activity = <Date 2014-01-02.13:13:35.297>
actor = 'loewis'
assignee = 'loewis'
closed = True
closed_date = <Date 2014-01-02.13:13:35.298>
closer = 'loewis'
components = []
creation = <Date 2013-11-23.01:47:23.918>
creator = 'ncoghlan'
dependencies = []
files = []
hgrepos = []
issue_num = 19728
keywords = []
message_count = 23.0
messages = ['203949', '203951', '203952', '203953', '203971', '203982', '204011', '204032', '204034', '204040', '204296', '204776', '204779', '204780', '204782', '204784', '204876', '205791', '205796', '206851', '206852', '207160', '207161']
nosy_count = 6.0
nosy_names = ['loewis', 'ncoghlan', 'larry', 'ned.deily', 'python-dev', 'dstufft']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19728'
versions = ['Python 3.4']

@ncoghlan
Copy link
Contributor Author

While implementing the Windows installer for PEP-453 in bpo-19550, MvL pointed out that, *by default*, the installers should be able to uninstall everything they install.

At the moment, if they install pip by default, that's not accurate, as we don't currently have the appropriate command defined to do the uninstall.

For beta 1, Martin just went with the option of leaving the option off by default, but it would be good to resolve the uninstallation problem and allow it to be enabled by default.

My suggestion for a design requirement is:

If a user installs CPython use the binary installer, makes no changes to the installation options, makes no use of the installed pip and then uninstalls CPython, then there should be no CPython related files left on their system.

If they *do* use the installed pip (whether to upgrade pip itself or to install other packages), then files may potentially be left behind, since they're seen as user data files by CPython.

I also think it's OK if uninstalling CPython removes setuptools and pip from that installation's site-packages, even if they had been upgraded following the original installation.

@ncoghlan ncoghlan added the type-feature A feature request or enhancement label Nov 23, 2013
@dstufft
Copy link
Member

dstufft commented Nov 23, 2013

I don't know much about installers, can they execute code as part of their uninstall process?

@dstufft
Copy link
Member

dstufft commented Nov 23, 2013

Also does the PEP need updated? It specifically called out this problem and said that it will leave pip behind?

@ned-deily
Copy link
Member

FTR, on OS X, there is no standard mechanism provided by the Apple-supplied installer program to uninstall binary packages. Thus, most third-party binary distributions for OS X that use the OS X installer can only be uninstalled manually unless they provide some custom uninstall script or package. The python.org OS X installers have not provided such up to now, although it has been suggested in the past. So this discussion does not currently apply to them.

@ned-deily
Copy link
Member

Also, FTR, the "install or upgrade pip" option is currently enabled by default for the OS X installers.

@loewis
Copy link
Mannequin

loewis mannequin commented Nov 23, 2013

This "clean uninstall" is a convention at least on Windows and Linux (except that on Linux, you have the option of leaving manually-edited configuration files behind if you wish).

An MSI can indeed invoke commands on uninstall (preferably of course before removing the actual Python interpreter, if the command needs Python). This is currently commented out in msi.py.

I agree that the current implementation correctly implements the PEP. The PEP says that it will leave files behind on uninstall, and it does. The PEP is silent on whether the option to install pip should be enabled by default, so it is off by default. If people really really want it, I could enable it by default, making it leave files behind. I would do so protestingly, and predict that users would complain.

The PEP also states that uninstallation is beyond its scope - so if uninstallation was added, no PEP change would be needed.

MSI also supports "authored" removal of files, i.e. I could explicitly hard-code a list of files to be removed on uninstall, see

http://msdn.microsoft.com/en-us/library/aa371201(v=vs.85).aspx

This supports wildcard file names, so I would only need to hardcode the list of directories to be removed.

Removal would only happen when the user originally selected to install pip. If it is done through a command, the command could opt to not remove pip if pip had been updated; the "authored" removal would remove the files regardless whether the had been updated (entirely new directories created in an update would be untouched).

One issue could be upgrades: currently, an upgrade (say to 3.4.1) is done by removing all files from the previous version, then installing all files of the new version. A command to remove should then not remove if the installed version is newer than the bundled one, or else the upgrade might downgrade pip.

@dstufft
Copy link
Member

dstufft commented Nov 23, 2013

Well the PEP does state that the option will be checked by default, but I'm not arguing that we shouldn't implement uninstall if Windows users expect it, I was just trying to figure out if we needed to update the PEP.

So unilaterally removing on uninstall sounds easy enough since you said you can use wildcards and such. The other option is to implement the uninstallation inside of ensurepip and just execute python -m pip uninstall setuptools pip in that case we can check version numbers and only uninstall if the version number is equal to the bundled one.

Unfortunately I'm not sure I'm going to have time to get an uninstall command implemented before the 24th.

@ncoghlan
Copy link
Contributor Author

This one isn't a feature freeze issue - what MvL has done already is fine
for now, and we can still add internal helper APIs during the beta.

@ncoghlan
Copy link
Contributor Author

As far as updating the PEP goes, it's a rare PEP indeed that is implemented
exactly according to spec - the integration process almost always uncovers
details that don't quite work out the way we thought they would.

For minor issues, we usually handle such changes without updating the PEP -
the issue tracker and the reference docs become the authoritative source.

Larger issues get discussed again on python-dev, and may lead to PEP
updates or even an additional PEP. That's pretty rare, though, as it
requires the original PEP discussion to miss something big.

@loewis
Copy link
Mannequin

loewis mannequin commented Nov 23, 2013

I agree with Nick; this is really a minor issue and we can resolve it here in the tracker. I missed the place in the PEP where it said that the option should be checked, since it mentions changes to the installers twice, and in "implementation strategy", it doesn't mention a default.

I'd prefer if uninstall was "symmetric" to install, i.e. if I need to run a command on install (rather than deploying files directly), then I also should run a reverse command on uninstall.

@ncoghlan
Copy link
Contributor Author

Given MvL's comment above, my suggestion is that we add an "ensurepip._uninstall" submodule that uninstalls pip and setuptools if it is invoked as __main__ and the following snippet results in uinstall being set to True:

try:
    import pip
except ImportError:
    uninstall = False
else:
    uninstall = (pip.__version__ == ensurepip.version())

(I believe PIP_VERSION in ensurepip is currently wrong, as it has an extra dot that shouldn't be there, but we can fix that as part of implementing this, and tweak the test in test_venv to ensure it doesn't get out of sync again)

@ncoghlan ncoghlan changed the title PEP 453: enable pip by default in the binary installers PEP 453: enable pip by default in the Windows binary installers Nov 25, 2013
@ncoghlan
Copy link
Contributor Author

I'm going to run with the approach of adding a private _uninstall function in ensurepip (that includes the version guard), and then a "python -m ensurepip._uninstall" submodule to invoke it from the command line.

I'll also extend the with_pip test in test_venv to cover uninstallation (it won't be a venv feature, it's just a convenient way to test that the uninstall command actually does the right thing).

@ncoghlan ncoghlan self-assigned this Nov 30, 2013
@python-dev
Copy link
Mannequin

python-dev mannequin commented Nov 30, 2013

New changeset 7dc4bf283857 by Nick Coghlan in branch 'default':
Issue bpo-19728: add private ensurepip._uninstall CLI
http://hg.python.org/cpython/rev/7dc4bf283857

@ncoghlan
Copy link
Contributor Author

Running this command in the installer should now clean up pip and setuptools:

python -m ensurepip._uninstall

If pip is not installed, it silently does nothing.

If pip is installed, but doesn't match ensurepip.version(), it throws RuntimeError.

If pip is installed and is the expected version, it removes both pip and setuptools (there's no separate version check before removing setuptools)

The tests in test_ensurepip are mocked out (to avoid any side effects on the test process), but test_venv now covers both installation and uninstallation (since it invokes a subprocess, thus avoiding the side effect problem).

@ncoghlan ncoghlan assigned loewis and unassigned ncoghlan Nov 30, 2013
@ned-deily
Copy link
Member

Looks like the buildbots are complaining, for example:

http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%203.x/builds/3300/steps/test/logs/stdio

======================================================================
FAIL: test_with_pip (test.test_venv.EnsurePipTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_venv.py", line 342, in test_with_pip
    self.assertEqual(err, "")
AssertionError: '/tmp/tmp0hm1zng7/bin/python: No module named ensurepip._uninstall\n' != ''
- /tmp/tmp0hm1zng7/bin/python: No module named ensurepip._uninstall

@python-dev
Copy link
Mannequin

python-dev mannequin commented Nov 30, 2013

New changeset a0ec33efa743 by Nick Coghlan in branch 'default':
Issue bpo-19728: don't be sensitive to line endings
http://hg.python.org/cpython/rev/a0ec33efa743

@ncoghlan
Copy link
Contributor Author

ncoghlan commented Dec 1, 2013

Oops, fumble fingered the issue number in one of the commits. This was the commit for the missing hg add Ned pointed above: http://hg.python.org/cpython/rev/04088790c077

@dstufft
Copy link
Member

dstufft commented Dec 10, 2013

Is there anything left in this ticket to be done?

@ncoghlan
Copy link
Contributor Author

Yes, it still needs to be integrated into the installer and the default state of the feature changed.

I expect MvL will look into that before creating the beta 2 installer.

Bumping the priority accordingly, though (I originally set it to high when we weren't sure if we were going to fix it for 3.4)

@python-dev
Copy link
Mannequin

python-dev mannequin commented Dec 23, 2013

New changeset cd62fc2488cf by Nick Coghlan in branch 'default':
Issue bpo-19728: fix ensurepip name clash with submodule
http://hg.python.org/cpython/rev/cd62fc2488cf

@ncoghlan
Copy link
Contributor Author

I noticed that actually importing ensurepip._uninstall would clobber the helper function, so I renamed it and added some new tests to check the basics of the command line functionality from the main ensurepip tests.

@python-dev
Copy link
Mannequin

python-dev mannequin commented Jan 2, 2014

New changeset 4c7b3e7fd4ca by Martin v. Löwis in branch 'default':
Issue bpo-19728: Enable pip installation by default on Windows.
http://hg.python.org/cpython/rev/4c7b3e7fd4ca

@loewis
Copy link
Mannequin

loewis mannequin commented Jan 2, 2014

The command works fine; I have now integrated it into the installer.

@loewis loewis mannequin closed this as completed Jan 2, 2014
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants