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

Use proper environment markers #706

Merged
merged 2 commits into from Sep 22, 2015
Merged

Conversation

sigmavirus24
Copy link
Contributor

This will intelligently install the right dependencies for a "secure"
urllib3 but only if you have a recent enough version of setuptools and
pip.

Closes #684

This will intelligently install the right dependencies for a "secure"
urllib3 but only if you have a recent enough version of setuptools and
pip.

Closes urllib3#684
@Lukasa
Copy link
Sponsor Contributor

Lukasa commented Sep 15, 2015

Note that while this works it places some relatively new requirements on setuptools and pip. Given how widely used urllib3 is we may want to be a bit cautious there. Just a thought. Otherwise I'm +1.

@shazow
Copy link
Member

shazow commented Sep 15, 2015

Will it explode for older versions of setuptools/pip, or just do the same ol' thing?

@Lukasa
Copy link
Sponsor Contributor

Lukasa commented Sep 15, 2015

For sufficiently old versions, it'll explode.

On 15 Sep 2015, at 19:31, Andrey Petrov notifications@github.com wrote:

Will it explode for older versions of setuptools/pip, or just do the same ol' thing?


Reply to this email directly or view it on GitHub.

@shazow
Copy link
Member

shazow commented Sep 15, 2015

What's a... sufficiently old verison? :P (cc @dstufft?)

@shazow
Copy link
Member

shazow commented Sep 15, 2015

More specifically, will it explode any moreso than what we already have with incorrect syntax?

@sigmavirus24
Copy link
Contributor Author

More specifically, will it explode any moreso than what we already have with incorrect syntax?

Yes. I think we might be able to move this to setup.cfg and get the benefits for wheels but not for tar files. In those we'll have to do stupid version checking hacks to not install things on versions of python we don't want them on.

@dstufft
Copy link
Contributor

dstufft commented Sep 15, 2015

pip < 6 and setuptools < uh, 0.7 I think.

@shazow
Copy link
Member

shazow commented Sep 15, 2015

@dstufft what's your gut feeling about when we should merge this?

And yea I'm happy to merge a setup.cfg workaround if it's safer for now.

@dstufft
Copy link
Contributor

dstufft commented Sep 15, 2015

I think that pip and setuptools older than that won't understand the syntax and will just ignore it, so it depends on how important you think it is that pip install urllib3[secure] actually installs secure. The cost of maintaining the setup.cfg work around is pretty small, but it relies on duplicating your dependencies so that's a possible downside.

@shazow
Copy link
Member

shazow commented Sep 15, 2015

Given that we have naggy warnings if you don't have [secure], I think it's reasonably okay if pip install urllib3[secure] doesn't succeed.

Wonder if we can print somekind of message on install to indicate that state. <- might be a great idea?

@dstufft
Copy link
Contributor

dstufft commented Sep 15, 2015

You can't really tell what version of pip you're being installed with.

@shazow
Copy link
Member

shazow commented Sep 15, 2015

No, I mean based on the state of import availability of dependencies for [secure].

@dstufft
Copy link
Contributor

dstufft commented Sep 15, 2015

You probably don't want to depend on install_requires being installed before urllib3 itself is installed.

@shazow
Copy link
Member

shazow commented Sep 15, 2015

@dstufft informs me that modern pip blocks stdout/stderr unless install fails, so can't do that kind of trickery anymore.

Opened pypa/packaging-problems#77 but not expecting it to be available in the foreseeable future.

@sigmavirus24
Copy link
Contributor Author

Looks like wheel/setuptools don't have a way of parsing a setup.cfg for extras without using pbr.

@sigmavirus24
Copy link
Contributor Author

I just updated this to use setup.cfg. It took some work to figure out the syntax for setup.cfg but it now works. This should let older pip/setuptools to do almost the right thing when installing from an sdist while always doing the right thing when installing from a wheel. (Also, we should make sure we ship the setup.cfg in sdists)

@shazow
Copy link
Member

shazow commented Sep 22, 2015

Looks good!

(Also, we should make sure we ship the setup.cfg in sdists)

How do we do that? My release script is here: https://github.com/shazow/urllib3/blob/master/release.sh#L36

@sigmavirus24
Copy link
Contributor Author

Just checked an sdist and it is. So we're good (that means if someone uses an sdist and then uses a new enough pip, it will build a wheel for them. (The only downside with this is that people using different packages will get different results, but that's better than having a totally broken package.)

shazow added a commit that referenced this pull request Sep 22, 2015
@shazow shazow merged commit 65b352f into urllib3:master Sep 22, 2015
shazow added a commit that referenced this pull request Sep 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants