Use interpreter constraints to verify lockfile environment, rather than invalidation inputs #12566

chrisjrn · 2021-08-13T20:20:57Z

This closes #12542.

Still to do: Replace all of the existing lockfiles with ones that match the new format.

Open questions:

Behavior when lockfile does not specify interpreter constraints -- what should we do here?
Behavior when a tool does not specify a lockfile (e.g. Pytest Runner)

Eric-Arellano

Yay!

This PR should regenerate the lockfiles imo, as if we had the consumption option set to error. Given that the format is changing.

All call sites will also need to be updated to no longer do that await Get(PythonLockfileRequest, ToolSentinel) - that's the whole motivation for this change, that we don't need to evaluate the global ICs for that tool anymore, only the context's ICs. That can be in a followup, though. I recommend splitting this up into 3 PRs:

Add the InterpreterConstraints.is_compatible() method.
Land this PR, which changes the data format to JSON and treats ICs as a separate field.
Leverage this PR to update call sites so that they're not wasting work as much as they do now.

src/python/pants/backend/experimental/python/lockfile.py

src/python/pants/backend/experimental/python/lockfile_metadata.py

src/python/pants/backend/experimental/python/lockfile_metadata_test.py

src/python/pants/backend/python/util_rules/interpreter_constraints.py

src/python/pants/backend/python/util_rules/interpreter_constraints_test.py

src/python/pants/backend/experimental/python/lockfile_metadata.py

chrisjrn · 2021-08-13T21:45:13Z

Yes, the PR should generate the lockfiles; I held up on committing updated lockfiles as I was having difficulty verifying whether they were working. Let's talk about that separately.

…tion (#12567) This adds two methods to `InterpreterConstraints`: * `enumerate_python_versions` produces a set of plausibly valid Python interpreter versions that match the given `InterpreterConstraints` (within a given universe of minor versions) * `contains` checks whether the plausible versions specified by `InterpreterConstraints` contains all of the plausible versions specified by another `InterpreterConstraints`. This contains some of the work done in #12566.

chrisjrn · 2021-08-16T18:48:50Z

@Eric-Arellano I've addressed most of the concerns from code review; remaining issues are still up for discussion

… for interpreter constraints [ci skip-rust] [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]

Eric-Arellano

Thanks! Lock files should be regenerated before landing.

src/python/pants/backend/experimental/python/lockfile_metadata.py

src/python/pants/backend/experimental/python/lockfile_metadata_test.py

# 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]

chrisjrn · 2021-08-17T00:19:41Z

@Eric-Arellano I've addressed all remaining feedback

…aints-as-data-not-hash-input [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]

* Clean up the config parsing code. ([pantsbuild#12678](pantsbuild#12678)) * [internal] `--generate-lockfiles-resolve` does not determine `PythonLockfileRequest` for unspecified tools ([pantsbuild#12692](pantsbuild#12692)) * [internal] Add missing `resources` targets for default tool lockfiles ([pantsbuild#12670](pantsbuild#12670)) * [internal] Reorganize lockfile code to no longer be in `backend/python/experimental` ([pantsbuild#12669](pantsbuild#12669)) * [internal] Test that default tool lockfiles work on macOS ([pantsbuild#12666](pantsbuild#12666)) * [internal] Improve the error message for stale/invalid lockfiles ([pantsbuild#12618](pantsbuild#12618)) * [internal] Rename `./pants lock` to `./pants generate-user-lockfile` and `./pants tool-lock` to `./pants generate-lockfiles` ([pantsbuild#12641](pantsbuild#12641)) * [internal] Only run macOS tests in CI annotated with `@pytest.mark.platform_specific_behavior` ([pantsbuild#12665](pantsbuild#12665)) * [internal] Test that Python tools work with all expected interpreter versions ([pantsbuild#12656](pantsbuild#12656)) * [internal] Factor out a helper rule for setting up `--resolve-all-constraints` ([pantsbuild#12630](pantsbuild#12630)) * [internal] Add links to changelog and Twitter in Pants PyPI project page. ([pantsbuild#12636](pantsbuild#12636)) * [internal] Update CoC with proper markdown + https links. ([pantsbuild#12637](pantsbuild#12637)) * GitHub issue templates ([pantsbuild#12617](pantsbuild#12617)) * Register subsystems and target types by backend/plugin. ([pantsbuild#12623](pantsbuild#12623)) * [internal] Sort input requirements for `LockfileMetadata` ([pantsbuild#12615](pantsbuild#12615)) * [internal] Expect lockfile metadata to be defined ([pantsbuild#12616](pantsbuild#12616)) * Simplify subsystem registration. ([pantsbuild#12620](pantsbuild#12620)) * [internal] Use constants for magic strings `<none>` and `<default>` for tool lockfiles ([pantsbuild#12613](pantsbuild#12613)) * [internal] Refactor `lockfile_metadata.py` to be more class-based ([pantsbuild#12611](pantsbuild#12611)) * Embrace that help strings are markdown. ([pantsbuild#12606](pantsbuild#12606)) * Stop fetching a lockfile request just to figure out the requirements digest ([pantsbuild#12607](pantsbuild#12607)) * [internal] Refactor MyPy first party plugins ([pantsbuild#12599](pantsbuild#12599)) * [internal] Refactor MyPy config file setup ([pantsbuild#12593](pantsbuild#12593)) * [internal] use `is_union()` rather than `union.is_instance()`. ([pantsbuild#12595](pantsbuild#12595)) * fs_util can use mTLS and ignore certificates ([pantsbuild#12586](pantsbuild#12586)) * [internal] Fix Pylint lockfile to actually be wired up ([pantsbuild#12590](pantsbuild#12590)) * [internal] Update lockfile TODOs with current state of project ([pantsbuild#12592](pantsbuild#12592)) * [internal] Check for lockfile staleness by using context's interpreter constraints, rather than global constraints ([pantsbuild#12566](pantsbuild#12566)) * [internal] Refactor Pylint first-party plugins ([pantsbuild#12583](pantsbuild#12583)) * [internal] Fix Pytest and IPython to check for stale lockfiles ([pantsbuild#12582](pantsbuild#12582)) * [internal] Do not cache lockfile generation ([pantsbuild#12674](pantsbuild#12674))

* Clean up the config parsing code. ([#12678](#12678)) * [internal] `--generate-lockfiles-resolve` does not determine `PythonLockfileRequest` for unspecified tools ([#12692](#12692)) * [internal] Add missing `resources` targets for default tool lockfiles ([#12670](#12670)) * [internal] Reorganize lockfile code to no longer be in `backend/python/experimental` ([#12669](#12669)) * [internal] Test that default tool lockfiles work on macOS ([#12666](#12666)) * [internal] Improve the error message for stale/invalid lockfiles ([#12618](#12618)) * [internal] Rename `./pants lock` to `./pants generate-user-lockfile` and `./pants tool-lock` to `./pants generate-lockfiles` ([#12641](#12641)) * [internal] Only run macOS tests in CI annotated with `@pytest.mark.platform_specific_behavior` ([#12665](#12665)) * [internal] Test that Python tools work with all expected interpreter versions ([#12656](#12656)) * [internal] Factor out a helper rule for setting up `--resolve-all-constraints` ([#12630](#12630)) * [internal] Add links to changelog and Twitter in Pants PyPI project page. ([#12636](#12636)) * [internal] Update CoC with proper markdown + https links. ([#12637](#12637)) * GitHub issue templates ([#12617](#12617)) * Register subsystems and target types by backend/plugin. ([#12623](#12623)) * [internal] Sort input requirements for `LockfileMetadata` ([#12615](#12615)) * [internal] Expect lockfile metadata to be defined ([#12616](#12616)) * Simplify subsystem registration. ([#12620](#12620)) * [internal] Use constants for magic strings `<none>` and `<default>` for tool lockfiles ([#12613](#12613)) * [internal] Refactor `lockfile_metadata.py` to be more class-based ([#12611](#12611)) * Embrace that help strings are markdown. ([#12606](#12606)) * Stop fetching a lockfile request just to figure out the requirements digest ([#12607](#12607)) * [internal] Refactor MyPy first party plugins ([#12599](#12599)) * [internal] Refactor MyPy config file setup ([#12593](#12593)) * [internal] use `is_union()` rather than `union.is_instance()`. ([#12595](#12595)) * fs_util can use mTLS and ignore certificates ([#12586](#12586)) * [internal] Fix Pylint lockfile to actually be wired up ([#12590](#12590)) * [internal] Update lockfile TODOs with current state of project ([#12592](#12592)) * [internal] Check for lockfile staleness by using context's interpreter constraints, rather than global constraints ([#12566](#12566)) * [internal] Refactor Pylint first-party plugins ([#12583](#12583)) * [internal] Fix Pytest and IPython to check for stale lockfiles ([#12582](#12582)) * [internal] Do not cache lockfile generation ([#12674](#12674))

chrisjrn requested a review from Eric-Arellano August 13, 2021 20:21

stuhood requested a review from jsirois August 13, 2021 20:34

Eric-Arellano reviewed Aug 13, 2021

View reviewed changes

chrisjrn mentioned this pull request Aug 13, 2021

[internal] AddInterpreterConstraints.contains() for lockfile validation #12567

Merged

chrisjrn force-pushed the chrisjrn/12542-constraints-as-data-not-hash-input branch 2 times, most recently from ca1f2b0 to 5583db0 Compare August 16, 2021 18:31

Christopher Neugebauer added 6 commits August 16, 2021 13:48

Changes lockfile invalidation calculation and storage format to allow…

63f2687

… for interpreter constraints [ci skip-rust] [ci skip-build-wheels]

Use is_valid_for to verify lockfile compatability

bb2a845

[ci skip-rust] [ci skip-build-wheels]

Feedback arising from code review

e8d0f38

# 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]

Use contains to verify InterpreterConstrains overlap; update comments

6d33a6b

# 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]

Renames hex_digest to requirements_hex_digest

775c9cb

# 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]

Better encapsulates LockfileMetadata (de)serialisation

ad9f857

# 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 reviewed Aug 16, 2021

View reviewed changes

src/python/pants/backend/experimental/python/lockfile_metadata.py Outdated Show resolved Hide resolved

src/python/pants/backend/experimental/python/lockfile_metadata_test.py Outdated Show resolved Hide resolved

Christopher Neugebauer added 5 commits August 16, 2021 17:19

Adds missing pytest lockfile consumption

672b0eb

# 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]

Stop failing on invalid user lockfiles

df1ac15

# 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]

Updates pex_test to use new lockfile format

8df02e0

# 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]

Addresses code review feedback

9618b64

# 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]

Adds updated lockfiles

4fa5158

# 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]

chrisjrn force-pushed the chrisjrn/12542-constraints-as-data-not-hash-input branch from 5583db0 to 4fa5158 Compare August 17, 2021 00:19

Eric-Arellano approved these changes Aug 17, 2021

View reviewed changes

Christopher Neugebauer added 3 commits August 17, 2021 07:52

Merge remote-tracking branch 'origin/main' into chrisjrn/12542-constr…

53dd4ec

…aints-as-data-not-hash-input [ci skip-rust] [ci skip-build-wheels]

Fix incorrect attribute name in repl.py

bce95ee

# 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]

Really fix incorrect attribute name

fcb2b80

# 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 merged commit 7a2bbe2 into pantsbuild:main Aug 17, 2021

Eric-Arellano mentioned this pull request Aug 19, 2021

Add [python-setup].disable_mixed_interpreter_constraints for repositories not using the interpreter_constraints field #12495

Closed

illicitonion mentioned this pull request Aug 30, 2021

Prep for 2.7.0rc0 #12695

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use interpreter constraints to verify lockfile environment, rather than invalidation inputs #12566

Use interpreter constraints to verify lockfile environment, rather than invalidation inputs #12566

chrisjrn commented Aug 13, 2021

Eric-Arellano left a comment

chrisjrn commented Aug 13, 2021

chrisjrn commented Aug 16, 2021

Eric-Arellano left a comment

chrisjrn commented Aug 17, 2021

Use interpreter constraints to verify lockfile environment, rather than invalidation inputs #12566

Use interpreter constraints to verify lockfile environment, rather than invalidation inputs #12566

Conversation

chrisjrn commented Aug 13, 2021

Eric-Arellano left a comment

Choose a reason for hiding this comment

chrisjrn commented Aug 13, 2021

chrisjrn commented Aug 16, 2021

Eric-Arellano left a comment

Choose a reason for hiding this comment

chrisjrn commented Aug 17, 2021