Skip to content

Only install stable releases by default #834

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

Merged
merged 4 commits into from
Mar 12, 2013
Merged

Conversation

dstufft
Copy link
Member

@dstufft dstufft commented Mar 9, 2013

This implements #820.

  • Adds the --pre flag to pip install to specify that you wish to install pre-releases
  • If the version spec contains any pre-releases then continue to install pre-releases
  • vendorizes distlib as pip.vendor.distlib, which effectively drops py25 support in pip-1.4

dstufft added 2 commits March 9, 2013 10:27
* Adds the --pre flag to pip install to specify that you wish
  to install pre-releases
* If the version spec contains any pre-releases then continue to
  install pre-releases
@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

I think we should go ahead and install_requires distlib==0.1.0
https://pypi.python.org/pypi/distlib/0.1.0

we just have to commit that we're going to make sure virtualenv can support installing it by the next release. ( @pfmoore )

also, it seems like distlib.version.Version could use a is_prerelease function. ( @vsajip ? )

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Distlib doesn't support PEP426 yet which is why I didn't want to use distlib proper yet. PEP426 made some changes to restrict the allowed values and fixed some inconsistencies in the ordering. Since it wasn't yet updated I didn't feel comfortable including it yet. However for the use of determining if something was a pre release or not the differences between PEP386 and PEP426 doesn't matter so I stripped it down to just the parts I needed.

I don't know how I feel about install_requires in pip, sort of has a chicken and egg problem.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Assuming the tests pass I consider this ready for merging, whether it should be merged I leave as an exercise to the reader ;)

As I indicated to @qwcode I don't feel comfortable using distlib as a whole yet.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

but the parts you used are unaltered? if so, why do we care if other parts are not up to date?
I'm ready to commit to distlib. I don't want to start cutting out parts of it.

yes, install_requires is odd, and wouldn't be the vehicle for installing distlib in virtualenv, but I could see it being used for installing pip globally in the real world right now. (which in many cases is python setup.py install using Setuptools) but that's really a detail.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Yes the parts I used are unaltered. I just deleted things that I wasn't using (and in many cases are wrong in the contexts of PEP426).

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

It's worth noting the the part I used is slightly wrong in the context of PEP426 as well, but not in a way that negatively affects my use of it here. PEP386 allowed (a|b|c|rc)N[.N] whereas PEP426 only allows (a|b|c|rc)N. But since the PEP386 behavior is a super set of the PEP426 behavior and it doesn't really hamper a test of pre-release or not I decided it was better to not alter the imported distlib.version instead of fixing that minor inconsistency.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Oh and the reason I care is because I didn't want to include something that was not up to date/wrong in the context of PEP426 because I felt it was confusing if we should be using them or not.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

And tests pass ;) So I guess all that's left is determining if it should be merged and/or if it needs any changes ;)

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

sure, I care about distlib being all correct in the end, if we're going to get the most out of it, as a dependency.
but IMO, don't think we should wait for that. we're not blessing all of it, by using some of it.

not sure you read the really long thread on the mailing list about wheel and distlib, but my tentative intention at the moment is to depend on distlib for our wheel support as well, so depending on it for that, but yet, cutting our parts of it for this would be pretty odd.

worse case scenario if "distlib" development ends all of a sudden (or it ends up being to hard logistically to maintain the relationship), then we can fold what we need into pip. but I want to be more optimistic now.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Thinking about this some more, I'm against depending on distlib (as opposed to folding it in) for other reasons as well.

pip is not hardly a dependency of practically anyone's application. However pip's dependencies and the applications dependencies will exist in the same context. This means that if an app depends on distlib (or any other dependency of pip's if taken to a conclusion that we might want to do this in the future) that by simply using pip they have additional constraints on what they can install in their environment.

The other problem here is one of testing. For your typical project if one of your deps breaks things you can just uninstall it and try to install a different version. However if a new version of distlib comes out that breaks with older versions of pip it's not inconceivable that it can break pip in a way that uninstalling distlib and installing a known good version is not possible leaving the user to need to manually uninstall distlib and then easy_install a known good version. By bundling our dependencies all of pip is one unit making it easier to test the combination of dependencies (because there is only 1 combination) and be more assured that such a state cannot be encountered.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

by simply using pip they have additional constraints on what they can install in their environment.

well, that's true now with our dependency on Setuptools/Distribute.
the idea ultimately is cut out Setuptools/Distribute as a dependency, and just have "distlib".
distlib would be it. no more. that's the limit.
that's the reason, I was wanting to use distlib, and not wheel (for "pip wheel"), because it simplifies things.

[...] By bundling our dependencies all of pip [...]

I'd be for some kind of automated process of periodically inserting "distlib" into a package of pip to reduce all of this confusion, but just not picking out parts one by one, and then having to maintain the divergence.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

I'd be ok with folding all of distlib into pip here. I only uses these parts because I thought including the unready parts would be confusing.

Would that work for you? And if so want me to update the PR to do that?

@pfmoore
Copy link
Member

pfmoore commented Mar 9, 2013

There are a number of issues here. @dstufft has a good point about imposing dependencies on user code. But we already have that with setuptools/distribute, as @qwcode points out. Adding distlib makes the problem worse, but replacing setuptools with distlib does not. In fact, given the complexities of setuptools' install, a distlib dependency is better. And adding distlib short-term with the intention of removing setuptools is an overall win, as it avoids either reimplementing things that already exist, or further extending our dependency on setuptools (wheel support involves both of these issues - the current code implements our own install code, and depends via wheel on setuptools for building wheels).

The other issue is that setuptools and distlib both incorporate elements of installer support, and elements of package runtime support - specifically, the entry point stuff. It's a hard dependency in setuptools (exe wrppers and console scripts in general use pkg_resources entry point support) but I'm not sure how it works in distlib ("exports" as distlib calls them and script wrappers aren't used in the wild yet, AFAIK). It might be worth lobbying for distlib to separate out the subpackages intended for runtime use from those intended to help with implementing installers - that would ease the dependency issue even further.

I'm not too happy about including the whole of distlib into pip - pip is about 625K, and distlib is (already) 1M+. Trebling the size of pip seems like a bad idea :-(

Regarding virtualenv, I do intend to add support for installing distlib (probably by making the whole "what to install by default" process more configurable and flexible, and supporting install from wheels at the same time). If it's important to do so, though, (which I don't particularly expect) I can rush in minimal support for just installing distlib from source.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

The benefit to setuptools is that it does have a fairly good history of not breaking shit, distlib is an unknown quantity there. I don't even know that I'd want distlib to make that claim right now as it's still relatively new and evolving.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

Trebling the size of pip seems like a bad idea

you're worried about size? help me understand that?
but in any case, isn't more size worth cutting out the issues raised about environment complexity, and virtualenv is simpler that way?

@pfmoore
Copy link
Member

pfmoore commented Mar 9, 2013

Agreed - setuptools/distribute is stable and distlib isn't yet (nobody expects it to be this early in its development). But distlib is the reference implementation of code supporting the new standards. So we should be using it going forward. The problem is managing the inevitable risks of instability in the short term.

Question: how likely is it that a version of pip relying on distlib is going to be ready for release before distlib is sufficiently stable for us to depend on it? And even if that does happen, can't we simply bundle distlib then as a short-term solution?

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

I wouldn't want distlib to be considered stable until packaging has settled down. I wouldn't expect that to happen for at least a year.

@pfmoore
Copy link
Member

pfmoore commented Mar 9, 2013

you're worried about size? help me understand that?

Actually, I was mistaken. I was thinking about the speed impact of installing distribute from source on Windows that we talked about before. But in this situation, we're installing the same code (distlib and pip) in any case, so the analogy doesn't apply. Apologies for the confusion.

However, in my picture of virtualenv's future, all I do is add the distlib wheel to sys.path, then use distlib to install a predefined list of requirements from virtualenv_support, or by downloading as a fallback. So in that case an unbundled distlib is simpler to use.

But it's not a huge deal whatever way it ends up playing out, certainly.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

I'd be ok with folding all of distlib into pip here.
Would that work for you? And if so want me to update the PR to do that?

sounds good to me at the moment
if we do that, we're off and running. no immediate virtualenv changes needed.
and in the long run, only pip itself would be "tainting" someone's environment
we just have to agree that we're not altering distlib inside pip.
I think it should go in nested under something like "pip.ext" to make it clear, it's only for looking, not touching.

In my picture of virtualenv's future, all I do is add the distlib wheel to sys.path

instead, couldn't you add pip's wheel to sys.path, and use pip.ext.distlib.wheel to install whatever?

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Ok this PR just got a whole lot biger :)

pip.vendor.* will be for vendored items (in this case pip.vendor.distlib). Inside of pip/vendor is a vendor.txt and a Makefile, in order to add or update a vendored item it should be added to the vendor.txt (pip requirements file) pinned exactly and then make should be ran from within the pip/vendor directory.

I added a note to pip/vendor/init.py that those files should be considered immutable, and hopefully the automated script will make it more unlikely for them to be modified.

I've also updated the setup.py to use setuptool's find_packages() command and updated pip.util to use the newly vendored distlib instead of the partially vendored one.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Just a note, I chose pip.vendor over pip.ext only because the latter looks more like it might be extension instead of external.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Well crap, distlib is 2.6+ so depending on it will require dropping Python 2.5 support from pip. The parts I was using happened to be 2.5 compatible.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

dropping py2.5 came up for pip-1.4 on the mailing list in the wheel/distlib discussion.
we'd have to drop it to easily support wheel anyway.

I think its time. you could modify .travis.yml for this pull.
at some point this weekend, I'll post an email to the mailing list to shake out some more critique of all this.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

The tests pass on everything but Python 2.5. So we'll need to either use my original strategy or drop Python 2.5 support.

@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

Heh too fast for me. I'll go ahead and take it out then.

* Includes helper scripts to maintaing pip.vendor
@dstufft
Copy link
Member Author

dstufft commented Mar 9, 2013

And there's the passing tests.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

@dholth, given all this, pip.vendor.pkg_resources is looking attractive.

@pfmoore
Copy link
Member

pfmoore commented Mar 9, 2013

+1 although things like pip install still require an installed copy of setuptools to work, so it isn't a complete solution...

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

require an installed copy of setuptools to work, so it isn't a complete solution...

my interest in pip.vendor.pkg_resources for pip-1.4 is wheel support across the board (not just for distribute), assuming vinay makes that one other distlib change with the entry point filename or whatever.

@pfmoore
Copy link
Member

pfmoore commented Mar 9, 2013

my interest in pip.vendor.pkg_resources for pip-1.4 is wheel support
across the board (not just for distribute), assuming vinay makes that one
other distlib change with the entry point filename or whatever.

I'm not sure how pkg_resources helps wheel support (or even why wheel
support is "just for distribute" otherwise). But this is getting a bit off
topic for this issue...

@dholth
Copy link
Member

dholth commented Mar 9, 2013

It makes sense to include pkg_resources and merge the perfectly good wheel branch. It would be quick and it's not going to prevent distlib from happening later.

The wheel branch uses pkg_resources to get the dependencies from a wheel but is otherwise independent.

@vsajip
Copy link
Contributor

vsajip commented Mar 9, 2013

From: Donald Stufft [email protected]

Distlib doesn't support PEP426 yet which is why I didn't want to use distlib proper yet. PEP426 made some changes to restrict the allowed values and
fixed some inconsistencies in the ordering. Since it wasn't yet updated I didn't feel comfortable including it yet. However for the use of determining if
something was a pre release or not the differences between PEP386 and PEP426 doesn't matter so I stripped it down to just the parts I needed.

PEP 426 support is coming - awaiting it getting beyond Draft status. I've already started adding the fields, the version support is written in a Gist I posted to distutils-sig and can easily be set up in distlib.

@vsajip
Copy link
Contributor

vsajip commented Mar 9, 2013

From: dholth [email protected]
The wheel branch uses pkg_resources to get the dependencies from a wheel but is otherwise independent.

Do you mean a patched version of pkg_resources that understands environment markers? If not, how do you deal with dependencies that are environment-dependent?

@dholth
Copy link
Member

dholth commented Mar 9, 2013

distribute's pkg_resources has supported (PEP 345) markers since the end of August, including the "extras" extension.

@vsajip
Copy link
Contributor

vsajip commented Mar 9, 2013

From: dholth [email protected]

distribute's pkg_resources has supported (PEP 345) markers since the end of August.

Whoops, I forgot about that :-) I remember you telling me about markerlib.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

not sure how pkg_resources helps wheel support (or even why wheel support is "just for distribute" otherwise).

only Distribute's pkg_resources recognizes dist-info, which is how wheels are being installed.

@qwcode
Copy link
Contributor

qwcode commented Mar 9, 2013

we have enough consensus to merge at this point, except for clear agreement that we're ok with pip-1.4 dropping py25. going to list explicitly for that.

@dholth
Copy link
Member

dholth commented Mar 10, 2013

If you also vendorize pkg_resources then pip's dependency count (when installing wheels) is down to 0.

@qwcode
Copy link
Contributor

qwcode commented Mar 11, 2013

we have consensus IMO to drop py25.
I want to do a once over locally, later today, and then I'd be ready to merge.

@qwcode
Copy link
Contributor

qwcode commented Mar 12, 2013

merging

afterwards, I will:

  • maybe add 2 more unit tests that use a local file:// index (instead of direct find_links which are less commonly used)
  • add a pip install example, and maybe the pip install description should mention this too.
  • start off the 1.4 release notes with a bang. (distlib just got real, dropping py25, and fixing one of the most annoying pip limitations ever)

qwcode added a commit that referenced this pull request Mar 12, 2013
Only install stable releases by default
@qwcode qwcode merged commit 1b0d77a into develop Mar 12, 2013
@dstufft dstufft deleted the feature/stable-by-default branch March 12, 2013 12:14
@qwcode qwcode mentioned this pull request Mar 25, 2013
@@ -183,6 +183,9 @@ def mkurl_pypi_url(url):
logger.info("Ignoring link %s, version %s doesn't match %s"
% (link, version, ','.join([''.join(s) for s in req.req.specs])))
continue
elif is_prerelease(version) and not req.prereleases:
logger.info("Ignoring link %s, version %s is a pre-release (use --pre to allow)." % (link, version))
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize I'm way, way, behind on this, but was there any discussion of making this logged info loud at the default verbosity level (i.e. warning rather than info), at least for 1.4, since it's a significant change in behavior that might confuse people if it happens silently?

Copy link
Member

Choose a reason for hiding this comment

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

I wasn't around when this landed, but I agree, we should be loud about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not that I remember. I'm not opposed to that change though. Might be rather noisy though since it'll give a warning for every ==dev and such for any apps that have them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, yeah, that could get noisy. I guess what we'd ideally want is just a single warning iff a stable version is selected when a prerelease version would otherwise have been selected. But that's a more complex fix than just changing this info to warning.

@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