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

patch: extend pip to honor "Extras" requirements #238

Closed
wants to merge 1 commit into from

Conversation

MatthewMaker
Copy link
Contributor

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,]"

@carljm
Copy link
Contributor

carljm commented Mar 22, 2011

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?

@MatthewMaker
Copy link
Contributor Author

@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,]"'.)

@carljm
Copy link
Contributor

carljm commented Mar 22, 2011

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.

@jvantuyl
Copy link

+1

I need this.

@dsully
Copy link

dsully commented Apr 27, 2011

+1 - Also needed here. Thanks.

@g2p
Copy link
Contributor

g2p commented Jun 7, 2011

On pip uninstall Paste[openid, Flup,], I don't think it should be valid at all when pip doesn't support marking packages unused. If pip supported it, like aptitude with manually installed / automatically installed packages, I would understand it as removing the Paste dependencies on openid and Flup, but not removing Paste itself, and not removing openid and Flup unless nothing else depends on them anymore.

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 uninstall Paste[openid] syntax might become useful in the future, but having different pip versions doing different things silently would be a pain, right now the uninstall syntax shouldn't be valid at all.

@nisavid
Copy link

nisavid commented Aug 30, 2011

please merge this patch. thanks in advance!

@jezdez
Copy link
Member

jezdez commented Sep 13, 2011

@nisavid The patch isn't ready yet as there are no tests included.

@carljm
Copy link
Contributor

carljm commented Oct 23, 2011

Anyone who wants this patch merged is welcome to add tests and/or address the comments above (i.e. remove the uninstallation of dependencies).

@carljm
Copy link
Contributor

carljm commented Dec 10, 2011

Merged. Thanks @MatthewMaker for the patch and @ssaboum for tests.

@carljm carljm closed this Dec 10, 2011
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants