-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: allow specifying build constraints for dependencies #10388
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis pull request introduces support for specifying build constraints for dependencies via a new [tool.poetry.build-constraints] section in pyproject.toml, propagating these constraints through the installation and build process, updating the relevant core logic, and adding comprehensive tests and documentation. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @radoering - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
f440bc7 to
92eeb2d
Compare
|
Deploy preview for website ready! ✅ Preview Built with commit 780c7cc. |
92eeb2d to
568add6
Compare
568add6 to
780c7cc
Compare
|
I've tested this out with an internal scenario (see linked issue) and it seemed to work as expected! Testing with a constrained setuptools[project]
name = "tmp-6g7i6voyom"
version = "0.1.0"
description = ""
authors = [
{name = "Your Name",email = "[email protected]"}
]
readme = "README.md"
requires-python = ">=3.11, <4"
dependencies = [
]
[tool.poetry]
packages = [{include = "tmp"}]
[build-system]
requires = ["poetry-core>=2.0.0,<3.0.0"]
build-backend = "poetry.core.masonry.api"
[tool.poetry.build-constraints]
smbus-cffi = { setuptools = "<80.7.0" }
[[tool.poetry.source]]
name = "upstream-pypi"
url = "https://nexus.xes-mad.com/repository/upstream-pypi/simple"
[[tool.poetry.source]]
name = "xes-pypi"
url = "https://nexus.xes-mad.com/repository/xes-pypi/simple"Testing with a constrained cffi[project]
name = "tmp-6g7i6voyom"
version = "0.1.0"
description = ""
authors = [
{name = "Your Name",email = "[email protected]"}
]
readme = "README.md"
requires-python = ">=3.11, <4"
dependencies = [
"xes-bscan (==0.9)"
]
[tool.poetry]
packages = [{include = "tmp"}]
[build-system]
requires = ["poetry-core>=2.0.0,<3.0.0"]
build-backend = "poetry.core.masonry.api"
[tool.poetry.build-constraints]
smbus-cffi = { cffi = "<2.0.0" }
[[tool.poetry.source]]
name = "upstream-pypi"
url = "https://nexus.xes-mad.com/repository/upstream-pypi/simple"
[[tool.poetry.source]]
name = "xes-pypi"
url = "https://nexus.xes-mad.com/repository/xes-pypi/simple" |
|
I tried getting hacky to add pip a la: so the build would work with an older setuptools, but pip was not inserted into the environment. So existing dependencies can be constrained but new dependencies cannot be inserted (or at least not pip). |
This is expected because:
If we decided to allow to add dependencies, |
I think this is absolutely fine, i just wanted to mention it. This PR functions as I expected. I don't know if you want input from others i know its been open for a while. |
|
lmk if there's anything else I can help with. |
Thanks for trying it out. I have it on my agenda for the next minor release. Since this is not imminent, I will probably keep this PR open a bit longer and merge it as soon as the next release is near (assuming there is no other feedback in the meantime). |
|
Out of curiosity, is there a milestone or target for 2.3.x where this might be included? I'm trying to decide if I can hold out for that release or if I should stay pinned on the current version of poetry we have for an upcoming release. Thanks! |
|
Things may change but I would not expect it in the next few weeks. |
Pull Request Check List
Resolves: #10299
Example:
At some point, I wondered if build constraints should be part of the pyproject.toml or part of the configuration. I stuck with the pyproject.toml because it is easier to implement.
A small shortcoming is that the constraints are only applied when installing dependencies - not when building a package to extract metadata. I think we can live with that for the time being.
Another shortcoming is that the constraints are not really constraints but implemented as dependencies of an optional group, which is not installed. In most cases, this will not make a difference but in rare cases it could result in unnecessary solver errors. Implementing real constraints will cost more effort and could be done later if it proves to be necessary.
Summary by Sourcery
Introduce support for specifying build constraints for dependencies in pyproject.toml, allowing users to control build-time dependency versions for specific packages.
New Features:
Enhancements:
Documentation:
Tests: