python-poetry / poetry-plugin-export

Poetry plugin to export the dependencies to various formats
MIT License
244 stars 54 forks source link

Allow exporting constraints for pip #125

Closed bmw closed 2 years ago

bmw commented 2 years ago

I've been using poetry export to generate a constraints file for use with pip. Sometime after the poetry 1.2.0a2 and poetry-core 1.1.0a7 releases, this command is now includes extras in its output such as docker[ssh]==6.0.0 which is not supported in pip constraints files.

Would you be open to at least adding an option to change this behavior? The easiest to implement would probably be something like a --no-extras flag (which may need a better name to avoid confusion with --extras). Another option would be to add support for a constraints.txt format.

neersighted commented 2 years ago

We would definitely do this as constraints.txt instead of requirements.txt (there are other differences) -- you are welcome to implement a new exporter, but I don't think anyone who works on this repo regularly has expressed any interest in writing it.

Also, if you're exporting pinned versions, what use case requires constraints.txt instead of requirements.txt? This seems fairly unique in the ecosystem.

bmw commented 2 years ago

Thanks for the quick feedback. I'll take a deeper look and see how feasible it'd be for me to write the new exporter myself.

My use case is for https://github.com/certbot/certbot. It's a large monorepo containing many Python packages, some of which need to be able to function without the others, and the dependencies vary based on the OS and Python version. To help us pin everything down more reliably while allowing us to install only the subset of packages that's actually needed, we have dummy pyproject.toml files under https://github.com/certbot/certbot/tree/master/tools/pinning and scripts that use poetry to generate and export files which we then use as constraints for pip.

bmw commented 2 years ago

Adding a constraints export method to https://github.com/python-poetry/poetry-plugin-export/blob/main/src/poetry_plugin_export/exporter.py doesn't look too bad. Do you know if this text from https://pip.pypa.io/en/latest/user_guide/#constraints-files contains the full list of differences that you'd want?

Their syntax and contents is a subset of Requirements Files, with several kinds of syntax not allowed: constraints must have a name, they cannot be editable, and they cannot specify extras.

I'm imagining just dropping any extras declared for the package. For the two conditions of "constraints must have a name" and "they cannot be editable", I'd either error out or exclude the package based on y'all's preference.

How does that sound?

neersighted commented 2 years ago

I personally would factor out exporter.py into a non-concrete base class, then add two concrete implementations that set booleans on the base exporter. The base exporter could then output slightly differently based on the booleans set by each implementation.

You have found the canonical source for the format -- I would suggest failing for now myself on any unexportable constraints.

bmw commented 2 years ago

Thanks for the guidance.

To be clear, are you OK with the constraints exporter dropping extras (e.g. turning docker[ssh]==6.0.0 into docker==6.0.0) instead of erroring out? This behavior is what I personally want for my use case but I also think it makes the most sense for a constraints format.

neersighted commented 2 years ago

I think it only makes sense to drop extras, as by definition constraints.txt is 'versions without install details.' The same packages will be captured anyway as Poetry locks all requested extras.

Editable installs are ambiguous and I think we should error for them. You won't need to deal with unnamed constraints as the exporter already names them with <name> @ <url>.

Looking at the code instead of remembering what it used to look like, it looks like we've already got support for format-based dynamic dispatch. As such I'd suggest that you simply move the body of _export_requirements_txt into a new method that takes some boolean values, rewrite the method _export_requirements_txt to invoke the new common method, and introduce _export_constraits_txt using the existing dynamic dispatch.

dimbleby commented 2 years ago

This also came up in #108

Not to discourage contributions - quite the opposite, this is evidence that there's real demand for this! - but as I suggested there, you might find it considerably less effort to sed the extras away so that the exported requirements.txt works for you...

neersighted commented 2 years ago

I don't disagree @dimbleby, but I do think that given certbot does seem to need 'real' constraints, first class support is a good idea so that they don't break in the future because of a brittle reliance on sed/"don't set develop = true".

dimbleby commented 2 years ago

I'm absolutely not against Someone adding this function - as I say, evidence is mounting that it's a thing people want.

I do think it's only fair to be sure that OP is aware that they probably have quicker (and dirtier) solutions available before they spend a bunch of time being that Someone.

bmw commented 2 years ago

I opened https://github.com/python-poetry/poetry-plugin-export/pull/128 for this.