-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
patch: extend pip to honor "Extras" requirements #238
Conversation
…declared "Extras".
Cool, thanks! At the moment we're focused on verifying that we haven't broken anything in the py3k porting work, and we'd like to do a release that is purely adding py3k compatibility, without adding new features, to simplify debugging when we get issue reports from the release. But this looks pretty good to merge in after that release. The one thing I'm not sure about is automatic uninstalling of extras dependencies. That doesn't match pip's behavior otherwise: we don't do automatic uninstalling of dependencies in general, so I'm not sure why extras should get special treatment. The basic problem is that with the metadata provided by current tools, we have no way to know whether a package was installed "just as a dependency" or was also explicitly requested by the user for its own sake. In case of the latter, an automatic uninstall would be really unfriendly. Am I missing something? |
@carljm - Awesome, that makes sense. And this is a pretty simple patch, once we're ready for it. Regarding uninstalling extras, it's not entirely automatic, in the sense that there's the Y/N prompt for each package. Oddly, I didn't realize that pip doesn't offer to uninstall dependencies when it knows that those dependencies were installed by pip. That's what my patch does for extras, it prompts to uninstall them if they were extra dependencies of a package that pip knows it installed. (At least, I think that's what it's doing. It's using the egg that pip makes...) The one gotcha there, of course, is if you already had the extra package for other reasons and want to keep it, in which case you should answer "N" to the uninstall prompt for that package. More contextual explanation and prompting for this might be desirable. Perhaps pip should offer to uninstall all dependencies, if you give it a command line option (To Be Determined) indicating that you prefer that. There will definitely be situations where a user installs a package with pip, and knows for certain that they would like to uninstall it and every other package that came in along with it. I know that the Python community has long suffered the lack of a way to handle reverse-dependency mapping and managing, but it seems sensible to offer the user this option, since pip can easily tell what all of the packages the user might want to remove along with the current one are. Anyway, it's pretty easy to omit the uninstall portion of the patch. It just seemed odd to introduce an additional asymmetry between install and uninstall -- now you can have more "orphaned" packages, with no good way to discover them later for removal. I would argue, however, that at least if the user is actually saying, on the command line, "pip uninstall Paste[openid,Flup]", it makes sense to interpret this as meaning that the user would like to have the opportunity to remove those extras. (I see I made a typo in my pull request, in the first example I gave I typed 'pip uninstall "openid[openid, Flup,]"' when what I meant was 'pip uninstall "Paste[openid, Flup,]"'.) |
Pip doesn't ever really know that it installed something; the comments in the uninstall() method are a little misleading that way. It knows that something is installed in pip's installation style, but really other tools can install things that way too (for instance, setuptools if you use the --single-version-externally-managed flag). So whatever automatic-uninstall behavior we provide should not be dependent on the installation format, I don't think. There is no explicit metadata anywhere that says "I was installed by pip" (though there will be, I believe in PEP376/distutils2). I'm open to discussion of pip prompting for removal of dependencies in general with an optional flag, though I'm not totally convinced. PEP376 will have a metadata flag signalling whether a package was auto-installed as a dependency or explicitly installed; once we have that I'd definitely like to prompt for removal of auto-installed dependencies. In any case, this should be a separate ticket/pull-request, and removed from this one, as it's a mostly orthogonal issue. I'm not sure what to think of the "pip uninstall Paste[openid, Flup,]" scenario. I agree that I don't see any other interpretation of the purpose of explicitly specifying extras when uninstalling. I guess my gut feeling is still to say that we only uninstall directly-named packages for now, to keep things simple, clear, and consistent, and we'll make use of that extras hint whenever / if we introduce dependency-uninstalling as a feature. |
+1 I need this. |
+1 - Also needed here. Thanks. |
On My rationale for this is that extras behave like small, dependency-only packages. Paste[openid] is a virtual package that depends on Paste and one or several openid packages. Uninstalling dependency-only packages has no effect except removing a dependency relationship, and removing dependency relationships has no effect when pip doesn't track them. Since the |
please merge this patch. thanks in advance! |
@nisavid The patch isn't ready yet as there are no tests included. |
Anyone who wants this patch merged is welcome to add tests and/or address the comments above (i.e. remove the uninstallation of dependencies). |
Merged. Thanks @MatthewMaker for the patch and @ssaboum for tests. |
This should fix issue #7. Should work no matter where these Extras are requested. (if it's on the command line, of course you need to put quotes around any args that include spaces.)
examples used to test:
pip install Paste[Flup,openid]
pip uninstall Paste[openid,flup]
pip install Paste[flup]
pip uninstall Paste[flup]
pip install "Paste [Flup, openid]"
pip uninstall "Paste[openid, Flup,]"