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

wheels for markupsafe #45

Closed
srkunze opened this issue Sep 10, 2015 · 38 comments · Fixed by #104
Closed

wheels for markupsafe #45

srkunze opened this issue Sep 10, 2015 · 38 comments · Fixed by #104

Comments

@srkunze
Copy link

srkunze commented Sep 10, 2015

Do you consider providing Python wheels for markupsafe? cf. http://pythonwheels.com/

@graingert
Copy link

@pallets you should separate out the binary speedups from the main source universal wheel

@davidism
Copy link
Member

davidism commented Nov 7, 2016

There's not much point in universal wheels beyond a minor size reduction and a green bar on that site. In this case it would also give a false sense of what's happening.

I am working on figuring this out, but creating a comprehensive set of platform wheels is not well documented, and is even harder when I only have access to Linux.

@graingert
Copy link

graingert commented Nov 7, 2016

@davidism the speedups are optional, so I'd argue it should be in an extras_require value, e.g. pip install markupsafe[speedups].

@davidism
Copy link
Member

davidism commented Nov 7, 2016

That's not the plan. Other libraries like SQLAlchemy bundle their speedups as well. The speedups are optional already, if they can't compile the pure Python version is used.

@graingert
Copy link

@davidism Twisted plan on using this approach for wheels: https://twistedmatrix.com/trac/ticket/7945

There are other advantages: You'd only need to rebuild the wheels for speedups changes, which presumably happen less often than the rest of the package.

@graingert
Copy link

SQLAlchemy is anti async/await and wheel so I don't think it's a good idea to follow their approach.

@mjpieters
Copy link

Why produce a universal wheel at all? That's not more useful than the current distribution formats for this project.

If you are going to distribute wheels, then people are going to expect those to include the compiled C speed-ups. That's the real strength of wheels; to distribute pre-built binaries.

@davidism
Copy link
Member

davidism commented Feb 9, 2017

We need Linux, Mac, and Windows; 32 and 64 bit; 2.6, 2.7, 3.3, 3.4, 3.5, and 3.6 builds. Travis can handle Linux (manylinux) and Mac; Appveyor does Windows. If anyone has experience setting these up contributions are welcome. Otherwise I'll keep working away at it when I have time.

@graingert
Copy link

@mjpieters @davidism wheels are much faster to install (even than pure python sdists), and when you're installing hundreds of packages it makes a big difference.

@graingert
Copy link

graingert commented Feb 9, 2017

Also the markupsafe C module is only an optional speedup, so would work very well as a separate module.

@mitsuhiko
Copy link
Contributor

mitsuhiko commented Feb 9, 2017

It's a massive pain to set up these things. If someone wants to do it, you can use getsentry/libsourcemap and getsentry/symsynd as a point of reference.

//EDIT: I want to clarify one thing: there will not be wheels without the speedups because then nobody would be able to install the speedups any more.

@graingert
Copy link

@mitsuhiko what makes you think that nobody would be able to install the speedups if they were a separate package?

@mitsuhiko
Copy link
Contributor

The entire point of markupsafe is the speedups. The only reason we have a fallback is because of user friendliness. Nobody should ever use markupsafe without the speedups. Feature flags barely work, definitely do not work with wheels and a markupsafe-fast as a separate package most people will miss entirely. So definitely not a fan.

@graingert
Copy link

@mitsuhiko I was actually thinking that you'd have the speedups in pip install markupsafe which would depend on the pure python in markupsafe-slow

@mitsuhiko
Copy link
Contributor

Since this hypothetical markupsafe does not exist right now (we do not build binaries yet) not sure what the value in this is. If anything this will make everything more complex now because what if people have dependencies to markupsafe but you only have markupsafe-slow installed? Eg: Jinja2 and Flask depend on markupsafe.

Also this will shave off basically no time since pip caches wheels locally anyways.

@graingert
Copy link

Yeah it would not be a great improvement but it means that the pure python would not go stale when a new binary was published

@mitsuhiko
Copy link
Contributor

I just don't get what exactly this is supposed to solve. MarkupSafe installs very quickly if there is no c compiler on the system. How much time are we talking about here?

@graingert
Copy link

@mitsuhiko
Copy link
Contributor

@graingert not sure how that is in any way relevant to the installation speed of markupsafe.

@graingert
Copy link

In case you're wondering the difference that wheels can make:

@graingert
Copy link

not sure if I linked to the correct comment, but it should be one benchmarking a pure-python wheel

@mitsuhiko
Copy link
Contributor

So from what I can tell downloading + installing markupsafe without compiling here takes 1.1 seconds with compiling 2 seconds and downloading and installing markupsafe from a wheel takes 0.8 seconds. That does not seem at all important in the grand scheme of things.

Even more so now that pip caches wheels locally so once you have installed markupsafe once it will just reused the wheel from the cache.

@mitsuhiko
Copy link
Contributor

I can't reproduce the 1.1sec vs 0.8 sec even. They are more or less the same when run multiple times. That also makes sense because there is barely any sourcecode in markupsafe and few files.

@m-messiah
Copy link

You said markupsafe installs quickly now, but you missed one fact for current master - it installs from github, instead of PyPi. So, while existence of wheels is a question, PyPi packages has big advantages after git:

  • quick version check, without downloading
  • declarative requirements without reinstalling
  • devpi proxy for PyPI

So, although there is no wheels, I think we still need PyPi release.

@mitsuhiko
Copy link
Contributor

I pushed up 1.0.

@m-messiah
Copy link

Thanks @mitsuhiko !

@edmorley
Copy link

Fwiw the performance improvement is not the only benefit, distributing as a wheel also:

  • Allows the wheel cache to work even when using the new pip hash-checking mode
  • Allows tools to statically inspect the package metadata without having to execute untrusted code.

If it helps, there's now a demo project for using manylinux1:
https://github.com/pypa/python-manylinux-demo

@davidism
Copy link
Member

davidism commented Jul 19, 2017

The manylinux example has been around for a while, and is great. The issue is figuring out the huge matrix of Mac and Windows builds, which have very sparse documentation as far as I can find. #45 (comment) I don't want to release a wheel just to satisfy a green check on a website, I want to release a good set of wheels to actually be useful.

Paul Kehrer has a good talk about the permutations to keep in mind: https://youtu.be/-j4lolWgD6Q?t=16m33s.

#65 adds support for building Windows wheels, but I have not had time to review it. If anyone is familiar and wants to help review, that would be great. I would also appreciate any help getting Travis set up to produce Mac builds (and manylinux too).

@edmorley
Copy link

I agree it would be good to cover all platforms, however there's no disadvantage to rolling out wheels builds incrementally for certain platforms first - especially given that certain combinations (eg linux) will cover a significant proportion of downloads :-)

@davidism
Copy link
Member

davidism commented Jul 19, 2017

I'm not very concerned with Linux since the extension requires nothing besides a compiler and the Python headers. Obviously it would be good to have, but I'm not going to close this issue just with that.

@davidism
Copy link
Member

@pombredanne
Copy link
Contributor

pombredanne commented Sep 25, 2017

@davidism if this can help I have CI repos I use specifically to build wheel on Linux, macOS and Windows at https://github.com/pombreda/thirdparty and https://github.com/pombreda/thirdparty-manylinux/
I will add markupsafe and kick in builds for 1.0 and the latest master.
At the minimum this can show if this builds alright.

@pombredanne
Copy link
Contributor

@edmorley FWIW if this can help, you can use a check or tarbal and run python setup.py --without-speedups bdist_wheel to build a wheel in dist/without the speedups locally.

@davidism unrelated but the setuptools Feature feature used for the optional speedups is marked as deprecated in the latest setuptools code.

@davidism
Copy link
Member

From what I remember, it's gone back and forth a few times. I think the alternative is Extension, but I'm not familiar with the differences.

@davidism
Copy link
Member

davidism commented Oct 8, 2017

Thanks to @dougthor42 #65 we now have Windows wheels for 2.6-3.6 32/64. I will upload them with the next release.

@ddanilko
Copy link

Could someone please upload a wheel for MarkupSafe 1.0?
I am having issue #86 due to pip trying to install from .tar.gz
I build a wheel from sources (it succeeded with some warnings that speedups could not have been built) and I managed to install it but I would rather not host it myself.

@pombredanne
Copy link
Contributor

thank you!

@davidism
Copy link
Member

davidism commented Nov 5, 2018

PyPI has wheels for MarkupSafe 1.1.0.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants