Add experimental tool lockfiles for Black, Isort, Yapf, Coverage.py, Lambdex, and Protobuf MyPy #12357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…x, and Protobuf MyPy # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
|
Blocked by #12200. The Black lockfile generated w/ Py36 does not work with 3.9, and vice versa. It can be fixed by manually adding environment markers. |
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Eric-Arellano
changed the title
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
stuhood
approved these changes
Jul 16, 2021
| # Register the UnionRule. | ||
| register_lockfile: ClassVar[bool] = False | ||
| default_lockfile_resource: ClassVar[tuple[str, str] | None] = None | ||
| default_lockfile_file_path: ClassVar[str | None] = None |
There was a problem hiding this comment.
Missed this the first time around, but: the fact that this gets baked into a Github URL will result in an incorrect URL if someone outside of the pantsbuild/pants repo declares a PythonToolRequirementsBase instance in a plugin.
Maybe make the field optional (even when default_lockfile_resource is set), and then declare it as:
default_lockfile_file_url: ClassVar[str | None] = None
...and have instances in pantsbuild/pants call git_url(cls.default_lockfile_file_path) explicitly? Callers outside the repo could construct a URL or not if they didn't want to bother.
There was a problem hiding this comment.
True, good point. Will fix here.
Yeah, Black support is technically added. Just the default file is not going to work if you don't use Python 3.6. But you can still generate your own lockfile. |
Eric-Arellano
changed the title
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
…more-lockfiles [ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
added a commit
that referenced
this pull request
Jul 26, 2021
These tool lockfiles are trickier than #12357 because they get their interpreter constraints from the users' code, rather than subsystems. So, when generating a lockfile, we query all their code to calculate the constraint. It's even trickier that Bandit and Flake8 partition by interpreter constraints, e.g. Py27 code running separately than Py37. Ideally, the single lockfile will be compatible with all partitions. To do this, we OR each distinct IC encountered. Using the technique from #12389, we'll try to generate a lockfile for each possible interpreter and then will merge into a single lockfile. Finally, this updates Black's generation to mirror our actual logic: if the user's code is Py38+ only, we automatically default to the ICs being Py38+ instead of Py36+. -- With this PR, we have now implemented lockfiles for: - Bandit - Black - Docformatter - Flake8 - isort - Yapf - Coverage.py - Lambdex - Protobuf MyPy plugin - setuptools These remain: - MyPy - Pylint - Pytest - IPython - pip-tools [ci skip-rust] [ci skip-build-wheels]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These tools are simple to support: they run entirely independently of user code, and they get their interpreter constraints from a subsystem rather than from user code.
*Black is temporarily not used because it does not play nicely with interpreter constraints: #12200.
Tools that still need lockfiles
Need to consider user interpreter constraints:
Need to be mixed into user lockfiles (or have shading):