search

pallets / markupsafe

Safely add untrusted strings to HTML/XML markup.
https://markupsafe.palletsprojects.com
BSD 3-Clause "New" or "Revised" License
634 stars 156 forks source link

Please provide a way to explicitly request pure Python build #471

Closed mgorny closed 3 weeks ago

mgorny commented 1 month ago

Currently, MarkupSafe supports falling back to pure Python version if extension build fails or an "unsupported" platform is used. Would you please consider supporting disabling it explicitly (e.g. via an additional environment variable) too? If anything, this could make it easier to test pure Python codepaths and compare results between CPython and PyPy.

I can submit a PR if you agree.

davidism commented 1 month ago

Is there a specific thing that's breaking that you're trying to work around with this? We already support PyPy by not building the speedups, and we already run our tests on both the Python and C implementations. That doesn't mean we don't have bugs, but I'd rather fix the bugs than disable the speedups.

mgorny commented 1 month ago

Not right now. We're working on uniformity across packages where speedups are optional, and while this wasn't the case for MarkupSafe, we've seen packages where the fastest way towards adding Python 3.13 support was to disable speedups until they were fixed.

davidism commented 3 weeks ago

It's possible to request the build without speedups by calling run_setup(False) in setup.py, which it does in various scenarios. I'm reluctant to add a public way for anyone installing to skip this build, as it's really not intended except for compatibility. I'm assuming you're working on gentoo, so could you patch setup.py directly in your case if you really need this?

mgorny commented 3 weeks ago

Sure. I was just hoping I'd avoid having to maintain a dirty patch forever.

davidism commented 3 weeks ago

After giving this thought, I've decided not to add it. Thanks for answering my questions along the way.

It's hard for me to think of the speedups as truly "optional", where a user would opt to use or not use them. Instead, they're optional in the sense of "this library works where speedups don't" but we still want users using the speedups in all other cases. Adding an env var to control that is counter to that goal.

Thanks to cibuildwheel, we provide a comprehensive set of ~60 wheels that should cover the vast, vast majority of users, so that they never need to worry about whether they can build the speedups from source. Because of that, I've actually considered making the build fail if there's no compiler available, rather than falling back. We're also considering a Rust implementation, so I'm not entirely sure what the build process will look like at that point.

Finally, our setup.py file is already set up to be able to force a build type with run_setup(with_binary=False). For specialized testing like you describe, I don't feel it's a big problem to maintain a patch that replaces the if/else block at the end to do what you want.