pip uninstall removes things it didn't install

[mithrandi]

Summary

If setup.py installs some modules into an existing package (via packages=["existing.package"]), this package will be listed in top_level.txt in the EGG-INFO; pip uninstall will then blow away the entire directory hierarchy on uninstallation, not just the modules that were installed.

Reproduction

I have made a bzr repo with some simple distributions that demonstrate this, available on Launchpad.

To reproduce:

pip install pip-testing/Parent
pip install pip-testing/Child_distutils
pip uninstall child

Observe that .../site-packages/parent is removed completely, not just .../parent/plugins/child_plugin.py.

Background

Twisted's plugin system locates plugins in modules included below a particular plugin package (twisted.plugins in the case of Twisted, but other projects using the plugin system will have their own plugin package, such as axiom.plugins or dosage.plugins). In the case of twisted.plugins (as well as most other users of the plugin system), twisted/plugins/__init__.py sets __path__ to include DIR/twisted/plugins/ for any DIR in sys.path; this allows placing the plugin alongside your main package when developing. However, at installation time, the usual thing to do is to install the plugin module into the site-wide Twisted installation directly, as my example setup.py demonstrates.

[carljm]

IMO this is simply a case where Twisted is doing it wrong; any setup.py that installs stuff under another project's top-level package is doing it wrong. This is generally not supported by the existing Python packaging ecosystem (and I don't think it should be). It's not just pip, the approach also breaks anytime eggs are used (thus your provided sample packages already don't work if used with easy_install, or even with just "python setup.py install" in the case of Child_setuptools). There is an accepted mechanism in place for doing this sort of thing (namespace packages), and pip works just fine if you use that.

Regardless, I don't think there's anything pip can do about this in the general case, given that we also uninstall stuff that was installed directly by distutils, setuptools, or easy_install -- and none of those record exactly which files were installed. We could try to address it only for packages that were installed by pip, but pip is currently consistent in assuming that anything under a top-level package is owned by the project that installed that top-level package, except in the case of namespace packages, and allowing projects to break that assumption only if they are installed by pip does not seem like an improvement.

In the long run, with PEP 376 installation format and the RECORD file, this should be generally possible - but I don't know whether distutils2 even allows projects to install modules underneath another projects' package without an explicitly declared namespace package (and if asked I would advise them not to).

[glyph]

It still seems to me that pip ought to actually complain about this at installation time, rather than blowing away a bunch of files at uninstall time. (Note that the installation does work.)

I should also note that Twisted's plugin system predates both eggs and namespace packages. This was a valid technique that worked fine, so I think it's Pip's fault (well, setuptools, really) that it causes spurious deletion of data.

However, we are already using a facility much like pkgutil.extend_path() at runtime, and I don't see anything about distutils in http://www.python.org/dev/peps/pep-0382/ - so what are we supposed to do in order to properly package such a thing?

[glyph]

Also: note that this bug report was motivated by a Stack Overflow question http://stackoverflow.com/questions/7275295/how-do-i-write-a-setup-py-for-a-twistd-twisted-plugin-that-works-with-setuptools which still has an open bounty. If you can answer it please do so :).

[mithrandi]

Note that I forgot to include the usual plugin module preamble that sets __path__ in parent/plugins/__init__.py. With that code inserted (I have pushed this change to my repo), the python setup.py install case using Child_setuptools works just fine; pip uninstall also works fine in this case, since parent doesn't end up in top_level.txt as the plugin module is installed into the egg, rather into parent itself.

[mithrandi]

Uh, correction, parent does end up in top_level.txt; but the uninstallation method is different, since pip uninstall just removes the egg, so the main parent dir doesn't get blown away.

[carljm]

It certainly would be preferable if pip could detect this situation at install-time and error out. That's a non-trivial ask, since pip just delegates actual installation to setuptools, but there might be ways to make it work. It's not a patch I'm likely to work on, but it's one I would accept. In the long run, the more important place to address this question is distutils2/packaging, since at some point pip will switch from delegating to setuptools to delegating (even more than it does now) to distutils2/packaging. I've raised the question with the d2 developers, and filed #357 to track the idea here.

When I referred to namespace packages, I didn't mean PEP 382; that PEP is not accepted yet (and may never be, if it is replaced by PEP 402) and is not implemented by any packaging tools that I know of. I meant setuptools' namespace packages feature (http://peak.telecommunity.com/DevCenter/setuptools#namespace-packages).

If Twisted itself were to declare twisted.plugins as a setuptools namespace package, then plugin-providing projects could do the same in their setup.py and provide sub-modules of that namespace package. This would be more compatible with existing packaging tools than the current approach.

[mithrandi]

Declaring parent.plugins as a namespace package runs into other trouble; however, at this stage, I feel like this conversation has gone way beyond the scope of this bug tracker. Can someone suggest a more appropriate venue? I assume there's a relevant mailing list, but I can't figure out which one it is.

[carljm]

@mithrandi - I think distutils-sig (http://www.python.org/community/sigs/current/distutils-sig/) would be the right place.

I'm curious what "other trouble" you run into with namespace packages; maybe I'll see a post on distutils-sig shortly that explains further ;-)

[mithrandi]

Okay, discussion continues here: http://mail.python.org/pipermail/distutils-sig/2011-September/018031.html

[carljm]

I'm rethinking my initial response to this bug.

I still think that dropping files inside other projects' top-level packages is fundamentally bad behavior for a setup.py in the distutils/setuptools ecosystem. It exploits what I consider to be a bug in distutils, that I would guess would have been explicitly disallowed had it been anticipated. And it only works at all in the presence of easy_install, eggs, or virtualenv (where twisted is installed outside the virtualenv and the plugin inside) thanks to some quite-surprising custom sys.path-lengthening code in __init__.py. So I'm quite hesitant to take any step that encourages Twisted to continue with this plugin system (or at least, to using it within the distutils/setuptools/pip packaging ecosystem).

But with that said, looking at pip's behavior on its own terms, it makes sense for pip to use the best information it has available when uninstalling, to uninstall exactly what was installed and no more. In the case where we find an installed-files.txt, that is the best information we have, more reliable than top_level.txt. In the case where there is no installed-files.txt (i.e. pip didn't do the installation), we'll continue to fall back on top_level.txt, in preference to not being able to uninstall at all.

At least that's my current thought, open to opinions from other pip maintainers. @jezdez?

[glyph]

carljm - thanks for the additional consideration! I agree: even if Twisted is doing something bad here, it's a bad thing which shouldn't result in delayed unrelated data-destruction.

For what it's worth, the "surprising" __path__-lengthing code (it does not lengthen sys.path!) was re-visited when eggs were introduced and was found to work for entirely reasonable relevant-PEP-friendly reasons :). The behavior where eggs, or other self-contained sys.path entries, can contain their own plugins, is very much intentional. On OS X, for example, Twisted is in /System/Library/... (DO NOT TOUCH :-)!) but plugins might be in /Library. The dist-packages/site-packages separation on Ubuntu is a similar distinction.

Our plugin system basically has a competing implementation of namespace packages, that resolves some issues that we found in practice with a completely-compatible namespace-package implementation: since Twisted is preinstalled on many platforms, it's not uncommon to have a system install and a different development install on PYTHONPATH. (Of course, in the pip universe, a no-site-packages virtualenv is the way to address this problem, but again, this solution predates that universe, and not everyone lives in that universe.)

I'm happy to try to evolve in a different direction, but unconditionally depending on setuptools would still be a problem now. In the future, depending on distutils2 or (eventually, in like 10 years, I guess) on 'packaging' would of course be fine.

[carljm]

@glyph - Whoops, you're right of course, __path__ is not the same thing as sys.path. And you're also right that what Twisted is doing in its twisted/plugins/__init__.py could be considered a reasonable alternative implementation of namespace packages. It just happens to not be the implementation that anyone outside the Twisted community uses, and relies on violating a constraint that I had previously taken to be an invariant of the distutils world, and would prefer it were :-)

[merwok]

It exploits what I consider to be a bug in distutils, that I would guess would have been explicitly
disallowed had it been anticipated.

Can you open a bug report about that so that distutils2 is better?

[carljm]

It exploits what I consider to be a bug in distutils, that I would guess would have been explicitly
disallowed had it been anticipated.

Can you open a bug report about that so that distutils2 is better?

I sent a mail a week or so ago about the issue to the "fellowship of the
packaging" mailing list, to prompt discussion of what the proper
behavior of distutils2 should be. Hasn't been any response yet, but
sure, I can go ahead and file a bug.

[glyph]

If distutils2 has an explicit namespace package mechanism that is built-in, I doubt that this will be a problem. The issue right now is really that pip depends on setuptools, but can install distutils-only packages. When we release a version of Twisted that supports distutils2, we can annotate our plugins package appropriately, since support for distutils2 will be a new feature.

[merwok]

FTR, distutils2 won’t get “an explicit namespace package mechanism”, but it will support namespace packages for Python versions that do include such a mechanism, i.e. hopefully 3.3+.

[carljm]

Just to clarify the status of this bug for anyone coming along: a pull request to make pip ignore top-level.txt if installed-files.txt is available would be accepted.

[carljm]

Fixed with merge of #437 - thanks @pjdelport