Move all internal APIs to pip._internal #4700

dstufft · 2017-08-31T15:52:05Z

It's not really going to be possible to review this PR, it touches basically every file in pip, but mostly to move them and rewrite imports. Functionally what this does is:

Move pip.* into pip._internal.*.
Move pip/ into src/pip/.

I was unsure about if this was a good idea or not, but as I implemented it, it has really grown on me. I think the biggest benefits will be:

Completely obvious that these APIs are internal and should not be depended on.
Paves the way for adding some public APIs (maybe for plugins?) with a clear distinction between what is and isn't public.

pfmoore · 2017-08-31T16:01:08Z

Yeah, no way I'll be able to review this, but I'm +1 on the idea and if tests pass that's probably all we can expect to check.

Probably worth taking care when we merge this - as you noted elsewhere it'll break pretty much every existing PR...

dstufft · 2017-08-31T16:20:20Z

@pfmoore Yea. I don't really have a good idea for how to take care of when we merge this. I think no matter what it's gonna suck and probably there's not really a good way forward besides just doing it.

pradyunsg · 2017-08-31T16:23:33Z

news/4700.removal

@@ -0,0 +1,2 @@
+Move all of pip's APIs into the pip._internal package, properly reflecting the


What happens if there are 2 news files?

If the contents are the same, it'll get rolled up into a single news entry, referencing multiple issues.

Oh, that's neat. I never realised this was possible.

pradyunsg · 2017-08-31T16:25:07Z

pip/models/__init__.py

@@ -1,4 +0,0 @@
-from pip.models.index import Index, PyPI


You might have removed 2 files unintentionally.

pip.models.__init__ and pip._vendor.cachecontrol.compat.

Both of those files still exist.

pradyunsg · 2017-08-31T16:27:09Z

src/pip/_internal/__init__.py

@@ -17,7 +17,9 @@
 # to add socks as yet another dependency for pip, nor do I want to allow-stder
 # in the DEP-8 tests, so just suppress the warning.  pdb tells me this has to
 # be done before the import of pip.vcs.
-from pip._vendor.requests.packages.urllib3.exceptions import DependencyWarning
+from pip._internal.vendor.requests.packages.urllib3.exceptions import (


How about keeping vendored packages in pip._vendor?

I know it's bike shedding but if everyone agrees it's a nicer colour, that's good too. :)

Fine with me.

ghost · 2017-08-31T18:19:37Z

Is this going to be merged soon? If so, then I probably need to merge this branch into my PR to avoid conflicts.

Since de-vendoring support exists only for downstream, and they need to patch pip to get that support anyways, it seems reasonable to push support for testing that configuration onto them. This is something they need to do anyways, since they need to test their versions of the vendored libraries. See pypa#4657 for more information.

dstufft · 2017-08-31T19:56:24Z

@xoviat Yes, right now actually.

benoit-pierre · 2017-09-01T16:15:14Z

Note that this will break virtualenv at some point, since even something like python -c 'import pip; pip.main(...) won't work anymore.

I know using pip's internal API is unsupported outside of pip, but there's a difference between not supporting outside use (and saying so when a related bug is open) and actively breaking every project in existence that might be using it...

pradyunsg · 2017-09-01T16:19:08Z

@benoit-pierre Sorry if I'm being nosy but could you explain how this affects virtualenv? I'm not understanding that...

benoit-pierre · 2017-09-01T16:21:29Z

@pradyunsg: see https://github.com/pypa/virtualenv/blob/master/virtualenv.py#L855

dstufft · 2017-09-01T16:23:47Z

It won't break virtualenv until virtualenv bundles a new copy of pip, at which point virtualenv can be updated.

ghost · 2017-09-01T16:24:31Z

As far as I know, that only depends on pip.main, which is "semi-suppported."

@dstufft What about importing pip.__main__:main into the pip namespace?

ghost · 2017-09-01T16:28:08Z

Okay then, apparently not going to happen.

pradyunsg · 2017-09-01T16:30:29Z

see https://github.com/pypa/virtualenv/blob/master/virtualenv.py#L855

So, if someone does pip install -U pip in an old virtualenv, their environment would break?

What about importing pip.__main__:main into the pip namespace?

Please no... That would defeat the whole purpose of doing this change.

benoit-pierre · 2017-09-01T16:30:54Z

What about this virtualenv API use-case:

python -c 'import sys, virtualenv; virtualenv.create_environment(sys.argv[1], search_dirs=sys.argv[2:]) venv directory_with_updated_wheels

?

RonnyPfannschmidt · 2017-09-01T16:32:23Z

@pradyunsg as far as i can tell the environment would not break at all - the script is only executed with a self-defined pythonpath

dstufft · 2017-09-01T16:35:29Z

Virtualenv uses a bundled copy of pip. The only way that would change is if someone has explicitly overridden the bundled copy of pip (as @benoit-pierre posted above). But we don't, and have never attempted to support arbitrary versions of pip in that (virtualenv does more than just call pip.main(), so it is vulnerable to API breakages).

techtonik · 2017-09-07T08:20:21Z

Why additional src level? What else is supposed to be placed there?

pfmoore · 2017-09-07T08:25:11Z

Because it avoids import pip in a pip checkout from accidentally picking up the development version rather than the installed version. Sometimes this is what you want, but from our overall experience, it's more often a mistake.

derekbekoe · 2017-09-12T19:48:43Z

This change will be in the next release of pip?
Is there currently a schedule for this?

dstufft · 2017-09-13T10:36:28Z

Yes it will be part of the next release, there is no ETA at the moment.

lock · 2019-06-02T19:05:26Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

dstufft added the skip news Does not need a NEWS file entry (eg: trivial changes) label Aug 31, 2017

dstufft mentioned this pull request Aug 31, 2017

Document the lack of an API #4696

Closed

dstufft removed the skip news Does not need a NEWS file entry (eg: trivial changes) label Aug 31, 2017

dstufft force-pushed the make-internal branch from 73f2f9d to 9f90e5b Compare August 31, 2017 16:13

pradyunsg reviewed Aug 31, 2017

View reviewed changes

dstufft force-pushed the make-internal branch from 389579b to 311f8c7 Compare August 31, 2017 17:36

dstufft changed the title ~~[WIP] Move all internal APIs to pip._internal~~ Move all internal APIs to pip._internal Aug 31, 2017

ghost mentioned this pull request Aug 31, 2017

pip refactoring will break pipdeptree tox-dev/pipdeptree#83

Closed

dstufft force-pushed the make-internal branch from 9dba3bc to d94f9b2 Compare August 31, 2017 18:37

dstufft added 2 commits August 31, 2017 14:53

Move all internal APIs to pip._internal

95bcf8c

dstufft force-pushed the make-internal branch from d94f9b2 to 8860dc0 Compare August 31, 2017 18:56

dstufft merged commit a9d56c7 into pypa:master Aug 31, 2017

dstufft deleted the make-internal branch August 31, 2017 19:59

di mentioned this pull request Sep 1, 2017

Don't assume master branch exists when reinstalling editable package from Git #4450

Merged

pradyunsg mentioned this pull request Sep 2, 2017

Add infrastructure support for static type-checking using mypy #4545

Merged

sbidoul mentioned this pull request Sep 10, 2017

question about locking of vcs requirements pypa/pipenv#508

Closed

pradyunsg added this to the 10.0 milestone Oct 2, 2017

pradyunsg added the type: maintenance Related to Development and Maintenance Processes label Oct 2, 2017

pradyunsg mentioned this pull request Oct 20, 2017

Deprecate pip, pipX, and pipX.Y #3164

Open

sourcenouveau mentioned this pull request Apr 2, 2018

Changes in pip-10.0.0b2 break pipdeptree tox-dev/pipdeptree#95

Closed

minusf mentioned this pull request Apr 17, 2018

pip 10 hides API, pipchecker falls on face django-extensions/django-extensions#1197

Closed

cmonr mentioned this pull request Apr 24, 2018

README - don't update to PIP version 10 ARMmbed/mbed-cli#658

Closed

hoefling mentioned this pull request May 1, 2018

Adapt to breaking API changes in pip>=10 nvdv/vprof#82

Merged

piraz mentioned this pull request May 6, 2018

Cannot install on pip 10. candango/firenado#197

Closed

Xetius mentioned this pull request May 21, 2018

Fixed import exception when using pip 10+ ansible/ansible-container#931

Closed

nbeaver mentioned this pull request Jun 29, 2018

ModuleNotFoundError: No module named 'pip.utils' nbeaver/pip_file_lookup#2

Closed

lueschem mentioned this pull request Nov 27, 2018

Cleanup handling of required packages. lueschem/edi#38

Closed

Harmon758 mentioned this pull request Jan 22, 2019

Installing tweepy/tweepy#956

Closed

Harmon758 referenced this pull request in tweepy/tweepy Jan 22, 2019

Updated requirements for pip 6.0+ to include a session

9b4cb9e

This was referenced Jan 22, 2019

pip version 6.0 changed the method signature for pip.req.parse_requirements. tweepy/tweepy#533

Closed

i have an issue with requirements tweepy/tweepy#564

Closed

lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019

lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019

		@@ -0,0 +1,2 @@
		Move all of pip's APIs into the pip._internal package, properly reflecting the

Move all internal APIs to pip._internal #4700

Move all internal APIs to pip._internal #4700

Uh oh!

Conversation

dstufft commented Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfmoore commented Aug 31, 2017

Uh oh!

dstufft commented Aug 31, 2017

Uh oh!

pradyunsg Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

dstufft Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

pradyunsg Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

pradyunsg Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

dstufft Aug 31, 2017 • edited by pradyunsg Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pradyunsg Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

dstufft Aug 31, 2017

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 31, 2017

Uh oh!

dstufft commented Aug 31, 2017

Uh oh!

benoit-pierre commented Sep 1, 2017

Uh oh!

pradyunsg commented Sep 1, 2017

Uh oh!

benoit-pierre commented Sep 1, 2017

Uh oh!

dstufft commented Sep 1, 2017

Uh oh!

ghost commented Sep 1, 2017

Uh oh!

ghost commented Sep 1, 2017

Uh oh!

pradyunsg commented Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benoit-pierre commented Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Sep 1, 2017

Uh oh!

dstufft commented Sep 1, 2017

Uh oh!

techtonik commented Sep 7, 2017

Uh oh!

pfmoore commented Sep 7, 2017

Uh oh!

derekbekoe commented Sep 12, 2017

Uh oh!

dstufft commented Sep 13, 2017

Uh oh!

lock bot commented Jun 2, 2019

Uh oh!

Uh oh!

dstufft commented Aug 31, 2017 •

edited

Loading

dstufft Aug 31, 2017 •

edited by pradyunsg

Loading

pradyunsg commented Sep 1, 2017 •

edited

Loading

benoit-pierre commented Sep 1, 2017 •

edited

Loading