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

Add a pip check command. #2492

Closed
wants to merge 1 commit into from

Conversation

msabramo
Copy link
Contributor

@msabramo msabramo commented Mar 6, 2015

I've taken the work from @Wilfred in #1001 and updated it to work with the latest code on develop and refactored a bit.

This command ensures that all packages installed have all the requirements they need, and that requirements have compatible versions. This is useful because pip can install incompatible dependencies, or a user may have manually (un)installed a package.

It looks like this if everything is okay:

[marca@marca-mac2 pip]$ pip check
[marca@marca-mac2 pip]$ echo $?
0

but if a package is missing:

[marca@marca-mac2 pip]$ pip check
pyramid 1.5.2 requires WebOb, which is not installed.
[marca@marca-mac2 pip]$ echo $?
1

or is the wrong version:

[marca@marca-mac2 pip]$ pip check
pyramid 1.5.2 has requirement WebOb>=1.3.1, but you have WebOb 0.8.
[marca@marca-mac2 pip]$ echo $?
1

Review on Reviewable

@piotr-dobrogost
Copy link

This is super useful taking into account that pip does not resolve dependencies (#988).

@xavfernandez
Copy link
Member

Quite nice to see it in pip :)
Maybe you should have kept Wilfred commit separated and do a clean merge commit.

A missing (maybe optional) feature would be to show the possible reason for a wrong version. Our internal tool outputs something like:

pkgA requires:
* pkgB [('>=', '1.50.0')] but installed version is 1.50.0.dev2015030301873
  Dependencies found:
    pkgB>=1.49.0 from pkgC 0.55.0.dev2015030500320,
    pkgB>=1.44.0 from pkgD 0.77.0.dev2015030301203,
    pkgB==1.50.0.dev2015030301873 from pkgE 0.10.0

which is useful to easily find the culprit.

@msabramo
Copy link
Contributor Author

msabramo commented Mar 6, 2015

Tests are failing on Python 3. I'll try to fix this later.

@msabramo
Copy link
Contributor Author

msabramo commented Mar 6, 2015

#1946 is kind of in the same vein, but a different spin.

@msabramo msabramo force-pushed the check_command_rebase_develop branch from d9195ad to e7f8afe Compare March 6, 2015 09:38
@msabramo
Copy link
Contributor Author

msabramo commented Mar 6, 2015

OK, I think e7f8afe should fix the Python 3 failures.

@pfmoore
Copy link
Member

pfmoore commented Mar 6, 2015

Can this be implemented as an external command? And if not, why not? To me this seems like a classic use case for a plugin command (see #1409 and #2329). I'm reluctant to keep adding more and more pip subcommands to the core, and yet without a better understanding of why standalone commands aren't feasible it's hard to justify adding a plugin system.

One key potential objection to an external command is that pip has no public API to allow external tools to interact with it - but that applies to plugins and core commands as well. Internal API changes could break plugins just as easily as external commands, and adding a command to the core just pushes the maintenance burden of dealing with internal API changes onto the core devs (the command still breaks, but everyone expects us to fix it then).

Any other reasons this can't be an external command? (I'm looking for good arguments in favour of a plugin system and/or adding a new core command, not objecting to the PR itself, btw...)

@xavfernandez
Copy link
Member

This can definitely be implemented as an external command (and already is) but I think it would make sense if pip directly provided this command.

@piotr-dobrogost
Copy link

It's already surprising to many that pip does not resolve dependencies. In addition having no way to check if dependencies are correctly installed (if only after the fact) would force users to check things manually or look for some external tool. Making sure dependencies are satisfied is the core competency of installer/package manager thus I don't see why it shouldn't be included in pip itself.

@msabramo
Copy link
Contributor Author

msabramo commented Mar 6, 2015

Another reason for not making it an external command is that I was imagining that in the future (i.e.: not in this PR), the code in pip/operation/check.py might be usable in other parts of pip (though maybe it should move to a different place...)

Example: How about if pip uninstall could do similar checks? E.g.: naïve user does something like:

$ pip install pyramid
...
$ pip uninstall -y webob
Uninstalling WebOb-1.4:
  Successfully uninstalled WebOb-1.4
$ pserve development.ini
...
ImportError: No module named webob.exc

Today they can uninstall webob and break their system. Maybe uninstall could be smarter though:

[marca@marca-mac2 pip]$ pip uninstall -y webob
pyramid 1.5.2 requires WebOb; removing it will break pyramid.
Use -f if you want to do this anyway.

apt-get for example does things like this.

Another possible use is a command for removing orphaned packages -- e.g.: a pip analogue of apt-get autoremove.

  autoremove
      autoremove is used to remove packages that were automatically installed 
      to satisfy dependencies for other packages and are now no longer needed.

e.g.:

$ pip uninstall pyramid
Uninstalling pyramid-1.6.dev0:
  Successfully uninstalled pyramid-1.6.dev0

$ pip autoremove
No packages depend on WebOb; removing
No packages depend on repoze.lru; removing
No packages depend on zope.interface; removing
No packages depend on zope.deprecation; removing
No packages depend on venusian; removing
No packages depend on translationstring; removing
No packages depend on PasteDeploy; removing

@msabramo
Copy link
Contributor Author

msabramo commented Mar 6, 2015

So hoping the above convinced @pfmoore but also what do other pip maintainers think of having a check command?

Cc: @pnasrat, @qwcode, @dstufft

@pfmoore
Copy link
Member

pfmoore commented Mar 6, 2015

So you're actually proposing a "check requirements" API (plus a convenience CLI interface to it)?

That's somewhat different, and does sound interesting. But I'd rather see the API properly designed in that case, not just put in an "pip.operations" submodule. Unless we start factoring out reusable APIs, we never get any closer to having a pip API that people (both 3rd party and internal) can rely on.

Would you be willing to refactor this PR into a check API (with proper docs,unit tests and in a suitable namespace)? I'm not promising that doing that would mean it'd get merged, just that it would be useful (to me at least) to see what the start of a gradual migration to a documented API might look like.

@msabramo
Copy link
Contributor Author

msabramo commented Mar 6, 2015

I guess I was proposing that without even realizing it :)

Probably not this week though. I wonder if maintainers would be amenable to merging this PR as-is, or with minor refinements and then then I'll submit another at a later date for the API?

I just fear that coming up with a good API could balloon into a big task and discussion and it might be good to "cash in" now and get some incremental progress before this gets lost in the shuffle.

@msabramo msabramo mentioned this pull request Mar 7, 2015


def test_check_missing_dependency_normalize_case(script):
# this will also install ipython, a dependency
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate this comment is probably my fault, but it's a copy-paste error :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Thanks! Just updated.

@Wilfred
Copy link
Contributor

Wilfred commented Mar 7, 2015

Fantastic, thanks for working on this. I think this would be a big improvement as it would enable users to check their virtualenvs are consistent.

This feature would be useful today -- what would the fine pip maintainers like to see, in order to get this merged?

This command ensures that all packages installed have all the
requirements they need, and that requirements have compatible
versions. This is useful because pip can install incompatible
dependencies[1], or a user may have manually (un)installed a
package.

[1] pypa#775
@msabramo msabramo force-pushed the check_command_rebase_develop branch from e7f8afe to 76c3561 Compare March 8, 2015 06:03
@guettli
Copy link

guettli commented Mar 18, 2015

+1

@benjaoming
Copy link

👍

This PR is unintrusive to existing codebase, code is clean, it holds big potential, and comes with a test. Also, it should definitely be a core feature as it can be used to test that other commands perform as expected.

@mathom
Copy link

mathom commented Sep 18, 2015

This would be really useful for us. 👍

@srinivasreddy
Copy link

Any blockers for merging this PR?

@wkingfly
Copy link

How's this going?

@dstufft dstufft closed this May 18, 2016
@dstufft
Copy link
Member

dstufft commented May 18, 2016

Accidentally closed this, reopening. Sorry!

@dstufft dstufft reopened this May 18, 2016
@wkingfly
Copy link

Well done.

@BrownTruck
Copy link
Contributor

Hello!

As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the master branch. However, this Pull Request was made against the develop branch so it will need to be resubmitted against master.

If you do nothing, this Pull Request will be automatically migrated by @BrownTruck for you.

If you would like to retain control over this pull request then you should resubmit it against the master branch, closing and referencing the original Pull Request.

If you choose to migrate this Pull Request yourself, here is an example message that you can copy and paste:

I've taken the work from @Wilfred in #1001 and updated it to work with the latest code on develop and refactored a bit.

This command ensures that all packages installed have all the requirements they need, and that requirements have compatible versions. This is useful because pip can install incompatible dependencies, or a user may have manually (un)installed a package.

It looks like this if everything is okay:

[marca@marca-mac2 pip]$ pip check
[marca@marca-mac2 pip]$ echo $?
0


but if a package is missing:

[marca@marca-mac2 pip]$ pip check
pyramid 1.5.2 requires WebOb, which is not installed.
[marca@marca-mac2 pip]$ echo $?
1


or is the wrong version:

[marca@marca-mac2 pip]$ pip check
pyramid 1.5.2 has requirement WebOb>=1.3.1, but you have WebOb 0.8.
[marca@marca-mac2 pip]$ echo $?
1

---

*This was migrated from pypa/pip#2492 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*

If this pull request is no longer needed, please feel free to close it.

@BrownTruck
Copy link
Contributor

This Pull Request has been automatically migrated to #3750 to reparent it to the master branch. Please see the new pull request for any new discussion.

@BrownTruck BrownTruck closed this May 26, 2016
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 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.