Allow python tools to be installed from a user resolve.#18418
Allow python tools to be installed from a user resolve.#18418benjyw merged 4 commits intopantsbuild:mainfrom
Conversation
| class PipRequirement: | ||
| """A Pip-style requirement. | ||
|
|
||
| Currently just a drop-in replacement for pkg_resources.Requirement. |
There was a problem hiding this comment.
Unrelated to the rest of this change, but I noticed that the docstring was out of date.
| default_interpreter_constraints: ClassVar[Sequence[str]] = [] | ||
| register_interpreter_constraints: ClassVar[bool] = False | ||
|
|
||
| # If this tool does not mix with user requirements (e.g. Flake8 and Isort, but not Pylint and |
There was a problem hiding this comment.
We generate lockfiles for all of those tools (in fact for all tools, period). So this comment was out of date.
b9c64f5 to
3fe2134
Compare
| metadata=metadata, | ||
| validation=validation, | ||
| lockfile=lockfile, | ||
| is_old_style_tool_lockfile=lockfile.resolve_name not in python_setup.resolves, |
There was a problem hiding this comment.
The changes in this file are just improvements to the error message generation. The proof is in the test, which hardly changed at all.
| # Subclasses may set to override the value computed from default_version and | ||
| # default_extra_requirements. | ||
| # TODO: Once we get rid of those options, subclasses must set this. | ||
| default_package_names: Sequence[str] = [] |
There was a problem hiding this comment.
I think package suffers from overloading in this space. This is not a package as in module/package - it's a distribution package (as you word it elsewhere in this PR). The distribution package - a .whl or .tar.gz/.zip - is the wrong layer of abstraction though. I think s/package/project/ correctly names this. Now correct vs what a User will understand are two different things that don't always coincide. So, with all that - as you see fit.
There was a problem hiding this comment.
My vote leans towards correctness with docs/help to aid users when/where needed--hopefully leads to fewer surprises overall.
There was a problem hiding this comment.
Yeah I wrote and rewrote this name six times before I landed on "package", as short for "distribution package", and I'm still very unsure that is the right name. I can change to project, you're right that this is the most technically correct. And since I'm not sure what users will find comprehensible, we may as well fall back on technical correctness.
|
|
||
| install_from_resolve = StrOption( | ||
| advanced=True, | ||
| default=None, |
There was a problem hiding this comment.
It would be great if this / rule code using it eventually got smart enough to only require saying from which resolve if you have more than one resolve. It would be really great if Pants rule code grew a step where it tried something like pex3 lock export --lock user.lock tool-req1 tool-req2, and if it got a success, just automatically used the user lock file.
| default_lockfile_resource: ClassVar[tuple[str, str] | None] = None | ||
| default_lockfile_url: ClassVar[str | None] = None | ||
|
|
||
| install_from_resolve = StrOption( |
There was a problem hiding this comment.
Does the export implementation need to be updated to account for this option? i.e. if I set this for pylint and then pants export --resolve=pylint, should it still work? Or should it error out and recommend running "export <user-resolve>" instead?
There was a problem hiding this comment.
It will still work as before if pylint is still a "tool lockfile". If you switch to using this new feature then presumably pylint is now the name of a "user resolve", in which case it should, again, work as before.
In a followup we can audit all the tools and loosen some of these.
#18418 added support for resolving tools from a user resolve. 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 intended to eventually replace the "tool lockfile" functionality, so that we have just one uniform way of generating lockfiles.