Audit and update requirements for Python tools.#18431
Audit and update requirements for Python tools.#18431benjyw merged 4 commits intopantsbuild:mainfrom
Conversation
jsirois
left a comment
There was a problem hiding this comment.
I've started using the dedicated semver compatible syntax in my Pants using projects for this: https://peps.python.org/pep-0440/#compatible-release
|
I'm going to hold off on merging this until I've written docs for the new way of updating versions. Upgrading some tools may break things for users! E.g., mypy will report things it did not previously report. I'm not sure how to resolve that short of providing a very easy escape hatch. The alternative is to only upgrade tools on a Pants major version bump, which seems extreme. |
|
Although I guess 2.16 is just starting its stabilization so I have time to write docs... |
fd8dc6a to
b3407b8
Compare
b3407b8 to
20e19a3
Compare
1f3e513 to
b88c8aa
Compare
b88c8aa to
bc3c062
Compare
kaos
left a comment
There was a problem hiding this comment.
Sorry for the large number of comments. Took me half-way through before I had most of the details here figured out.
The main theme concludes to:
- I think that we want to swap the order of declaring defaults so we can do
default_requirements = ["proj>=X.Y,<Z", "other", "stuff"]
default_version = default_requirements[0]
default_extra_requirements = default_requirements[1:]Then we are in a good position to remove default_version and default_extra_requirements.
-
I take it we employ a "default policy" of floating versions on a single major. Makes sense to me. Exceptions are to be expected, but when we find them, I think we would do well to document them.
-
This is a large enough change already, so this may be for another PR. This line struck out at me:
default_interpreter_constraints = ["CPython>=3.7,<4"]At the very least we may want to move our "default default_interpreter_constraints" value to a CONST for re-use to DRY this up a bit.
I had thought of that, but that seems pretty error prone, if someone changes
So that's not quite the policy. It's more that we've established a baseline, and we'll float above it as we upgrade versions in the future, until we make changes to how we use the tool that makes us incompatible with earlier versions, in which case we raise the floor. Note that this is all educated guesswork. The only tool versions we're really confident about are the one's we've tested against. But we're roughly assuming minor version float is safe. So what this is capturing, over time, is "in the past we tested against 1.2.3, so 1.x above that is compatible, and now we're testing against 2.3.4, so 2.x is also compatible, so the total compatible range so far is >=1.2.3,<3"). In the future that might extend up (<4) but we might have to raise the floor, if we start relying on a tool feature that wasn't available in 1.x.
Agreed, but this is a separate audit and this change is already a giant PITA, so I'd prefer to do that in another PR. There is still some stuff to figure out about ICs. For example, which ones we use to generate the default lockfiles (these should be broad) vs which interpreter we use to run the tool (that's a bit chaotic today, I think, but in practice I believe can always be the one your code is compatible with, since that's how the python ecosystem seems to work). |
|
To expand on my comment - |
|
Bah.. normally there is a cancel button next to the green one, but here it was a close button 🤢 |
|
@kaos Can you re-review taking my clarifications into account? Your detailed comments are, afaict, all covered by those clarifications. Thanks! |
Specifically, it added a `requirements` option for each tool that is expected to contain very loose requirements that we expect Pants to support. We initialized the default value for that option from the existing `version` and`extra_requirements` fields. However those were often overly-constrained, sometimes to a single version. This PR audits all the tools' defaults and in various cases does one or both of: 1. Update `default_version` to a newer version so we can regen more up-to-date lockfiles. 2. Explicitly set `default_requirements` to something loose but likely compatible. Of course it's hard to know which currently available tool versions Pants is compatible with, unless we test against them. And it's impossible to know which future versions Pants will remain compatible with. So we assume that tools are semver-adherent, so that floating up in the same major version is safe. Updating to newer versions lets us at least start to test against some newer versions, and is also overdue, since many of our default tool versions are quite old. The proof that these upgrades are compatible is in the passing tests. At some point soon we will `deprecate` `version` and `extra_requirements` in favor of using user lockfiles for custom tool versions.
Specifically, it added a `requirements` option for each tool that is expected to contain very loose requirements that we expect Pants to support. We initialized the default value for that option from the existing `version` and`extra_requirements` fields. However those were often overly-constrained, sometimes to a single version. This PR audits all the tools' defaults and in various cases does one or both of: 1. Update `default_version` to a newer version so we can regen more up-to-date lockfiles. 2. Explicitly set `default_requirements` to something loose but likely compatible. Of course it's hard to know which currently available tool versions Pants is compatible with, unless we test against them. And it's impossible to know which future versions Pants will remain compatible with. So we assume that tools are semver-adherent, so that floating up in the same major version is safe. Updating to newer versions lets us at least start to test against some newer versions, and is also overdue, since many of our default tool versions are quite old. The proof that these upgrades are compatible is in the passing tests. At some point soon we will `deprecate` `version` and `extra_requirements` in favor of using user lockfiles for custom tool versions.
…18460) Specifically, it added a `requirements` option for each tool that is expected to contain very loose requirements that we expect Pants to support. We initialized the default value for that option from the existing `version` and`extra_requirements` fields. However those were often overly-constrained, sometimes to a single version. This PR audits all the tools' defaults and in various cases does one or both of: 1. Update `default_version` to a newer version so we can regen more up-to-date lockfiles. 2. Explicitly set `default_requirements` to something loose but likely compatible. Of course it's hard to know which currently available tool versions Pants is compatible with, unless we test against them. And it's impossible to know which future versions Pants will remain compatible with. So we assume that tools are semver-adherent, so that floating up in the same major version is safe. Updating to newer versions lets us at least start to test against some newer versions, and is also overdue, since many of our default tool versions are quite old. The proof that these upgrades are compatible is in the passing tests. At some point soon we will `deprecate` `version` and `extra_requirements` in favor of using user lockfiles for custom tool versions.
This is based on changes in pants: - pantsbuild/pants#18431 - pantsbuild/pants#21894 Lockfile diff: lockfiles/twine.lock [twine] == Upgraded dependencies == jeepney 0.8.0 --> 0.9.0 nh3 0.2.20 --> 0.2.21 pkginfo 1.12.0 --> 1.12.1.2 twine 3.7.1 --> 4.0.2 == !! Downgraded dependencies !! == importlib-metadata 8.5.0 --> 7.2.1 == Added dependencies == markdown-it-py 3.0.0 mdurl 0.1.2 rich 13.9.4 typing-extensions 4.12.2 == Removed dependencies == tqdm 4.67.1
This is based on changes in pants: - pantsbuild/pants#18431 - pantsbuild/pants#21894 Lockfile diff: lockfiles/twine.lock [twine] == Upgraded dependencies == jeepney 0.8.0 --> 0.9.0 nh3 0.2.20 --> 0.2.21 pkginfo 1.12.0 --> 1.12.1.2 twine 3.7.1 --> 4.0.2 == !! Downgraded dependencies !! == importlib-metadata 8.5.0 --> 7.2.1 == Added dependencies == markdown-it-py 3.0.0 mdurl 0.1.2 rich 13.9.4 typing-extensions 4.12.2 == Removed dependencies == tqdm 4.67.1
#18418 added support for resolving tools from a user resolve.
Specifically, it added a
requirementsoption for each tool that is expected to containvery loose requirements that we expect Pants to support.
We initialized the default value for that option from the existing
versionandextra_requirementsfields. However those were often overly-constrained, sometimesto a single version.
This PR audits all the tools' defaults and in various cases does one or both of:
default_versionto a newer version so we can regen more up-to-date lockfiles.default_requirementsto something loose but likely compatible.Of course it's hard to know which currently available tool versions Pants is compatible with,
unless we test against them. And it's impossible to know which future versions Pants will remain
compatible with.
So we assume that tools are semver-adherent, so that floating up in the same major version is safe.
Updating to newer versions lets us at least start to test against some newer versions, and is
also overdue, since many of our default tool versions are quite old. The proof that these upgrades
are compatible is in the passing tests.
At some point soon we will
deprecateversionandextra_requirementsin favor of usinguser lockfiles for custom tool versions.