[internal] Make `generate-lockfiles` goal generic #14122

Eric-Arellano · 2022-01-08T00:46:15Z

The goal was originally hardcoded for Python. This allows us to now use it for other languages like Java.

We do this through some elaborate usage of unions + newtyping so that distinct types look the same.

Remaining TODOs:

Re-enable ambiguity check, and check for ambiguity between user lockfiles, like Python vs. Java.
Update help for the goal to be generic.

…thonLockfile` to `Lockfile` These will be common types used across multiple languages like Python and Java. # 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]

This also uses `WrappedLockfileRequest` so that we can make our tool lockfile code completely generic. We now have a way to dynamically compute which tools to generate lockfiles for, regardless of language. The only thing not yet generic is user lockfiles. # 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]

No changes made. Just moved around. # 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]

This allows us to move that Python specific functionality out of the `generate-lockfiles` goal. We use a `@rule` so that the warning doesn't render more than one time. # 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]

This is pretty involved. We need to first use a union to discover all "known" resolves per language. We then need another union to convert all _requested_ resolves for that language into concrete LockfileRequest subclasses # 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

I strongly recommend reviewing per-commit. I tried to add explanation to each commit message also.

# 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

Thanks a lot!

The individual commits are all clear, despite the size.

stuhood · 2022-01-08T00:55:00Z

src/python/pants/core/goals/generate_lockfiles.py

+@dataclass(frozen=True)
+class WrappedLockfileRequest:
+    request: LockfileRequest


Relates to #11354: ... subtyping in the return position of @rules would avoid the need to wrap here.

Agreed. Although TBH I found it useful needing the wrapper type - it made more clear to me what was what.

stuhood · 2022-01-08T01:05:51Z

src/python/pants/core/goals/generate_lockfiles.py

+    `PythonLockfileRequest`.
+    """
+
+


I'll be honest: I don't fully understand why two @unions are necessary... but I suppose that it's because some bit of logic must be async, and so needs to be provided by a @rule rather than a method of the union class?

It's because we need to first compute the entire universe of known resolves (the first union), then validate for ambiguity + filter to what was specified with --resolve, and then finally convert what was specified into actual LockfileRequest objects (the second union).

tdyas · 2022-01-08T11:17:22Z

src/python/pants/core/goals/generate_lockfiles_test.py

-        determine_resolves_to_generate(
-            {"ambiguous": "lockfile.txt"}, [AmbiguousTool], ["ambiguous"]
-        )
+    # class AmbiguousTool(ToolLockfileSentinel):


Eric-Arellano · 2022-01-08T23:42:54Z

@stuhood thoughts on how to fix this rule graph issue? If the Python backend isn't activated, the engine complains that there is no way to satisfy this: Get(WrappedLockfileRequest, ToolLockfileSentinel, sentinel()). Note that the rules for ToolLockfileSentinel. ToolLockfileSentinel is a union and rules should return something like PythonLockfileRequest, and then there's a rule to go from PythonLockfileRequest -> WrappedLockfileRequest.

https://github.com/pantsbuild/pants/runs/4745393095?check_suite_focus=true#step:14:1151

stuhood · 2022-01-11T00:17:18Z

@stuhood thoughts on how to fix this rule graph issue? If the Python backend isn't activated, the engine complains that there is no way to satisfy this: Get(WrappedLockfileRequest, ToolLockfileSentinel, sentinel()). Note that the rules for ToolLockfileSentinel. ToolLockfileSentinel is a union and rules should return something like PythonLockfileRequest, and then there's a rule to go from PythonLockfileRequest -> WrappedLockfileRequest.

All of the Python lint backends register instances of ToolLockfileSentinel, but they were not exporting the (generic) @rule that actually satisfies the @union: https://github.com/pantsbuild/pants/pull/14122/files#diff-e15283e30aacb25ae1fa37f0b33fab110d281d2322c064fd2f61c6ba9f40984dL171-R102

Re-exporting that @rule fixes this: will push.

…rs a `ToolLockfileSentinel`. # 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]

Internal changes: * upgrade to Rust v1.58.0 ([pantsbuild#14174](pantsbuild#14174)) * [internal] fix typos in codegen registration ([pantsbuild#14172](pantsbuild#14172)) * Remove per-language indirection for formatter plugins. ([pantsbuild#14166](pantsbuild#14166)) * Pulls `Coordinate` and `ArtifactRequirements` out into a separate file to avoid a circuilar import later ([pantsbuild#14164](pantsbuild#14164)) * Factors lockfile metadata code into class that supports multiple languages as well as versions ([pantsbuild#14116](pantsbuild#14116)) * Adds `.env` file to make vscode auto-imports work correctly out of the box ([pantsbuild#14130](pantsbuild#14130)) * [internal] Rename classes for `generate_lockfiles.py` for clarity ([pantsbuild#14218](pantsbuild#14218)) * [internal] Fix bad Find+Replace ([pantsbuild#14213](pantsbuild#14213)) * [internal] Bump libc from 0.2.106 to 0.2.112 in /src/rust/engine ([pantsbuild#14206](pantsbuild#14206)) * [internal] Bump tempfile from 3.2.0 to 3.3.0 in /src/rust/engine ([pantsbuild#14202](pantsbuild#14202)) * [internal] Bump criterion from 0.3.3 to 0.3.5 in /src/rust/engine ([pantsbuild#14205](pantsbuild#14205)) * [internal] Bump walkdir from 2.3.1 to 2.3.2 in /src/rust/engine ([pantsbuild#14204](pantsbuild#14204)) * [internal] Bump generic-array from 0.14.4 to 0.14.5 in /src/rust/engine ([pantsbuild#14203](pantsbuild#14203)) * [internal] Remove the minimum bucket size of batching to improve stability. ([pantsbuild#14210](pantsbuild#14210)) * [internal] Make `JvmLockfileRequest` generic ([pantsbuild#14201](pantsbuild#14201)) * [internal] add comment re clippy issue ([pantsbuild#14188](pantsbuild#14188)) * [internal] rename some codegen scopes to put language first ([pantsbuild#14187](pantsbuild#14187)) * [internal] Check for ambiguity when running `generate-lockfiles` ([pantsbuild#14178](pantsbuild#14178)) * [internal] Change JVM lockfile format ([pantsbuild#14175](pantsbuild#14175)) * [internal] go: rewrite third party package analysis to not use `go list` ([pantsbuild#14157](pantsbuild#14157)) * [internal] Hotfix `fmt` crashing on non-formattable targets ([pantsbuild#14168](pantsbuild#14168)) * [internal] Simplify `core/goals/package.py` filesystem API usage ([pantsbuild#14162](pantsbuild#14162)) * [internal] java/thrift: register union that was not registered ([pantsbuild#14159](pantsbuild#14159)) * [internal] Bump arc-swap from 1.3.0 to 1.5.0 in /src/rust/engine ([pantsbuild#14148](pantsbuild#14148)) * [internals] Bump tokio-rustls from 0.22.0 to 0.23.2 in /src/rust/engine ([pantsbuild#14149](pantsbuild#14149)) * [internal] Bump num_cpus from 1.13.0 to 1.13.1 in /src/rust/engine ([pantsbuild#14150](pantsbuild#14150)) * [internal] Bump tokio-util from 0.6.7 to 0.6.9 in /src/rust/engine ([pantsbuild#14151](pantsbuild#14151)) * [internal] Bump os_pipe from 0.9.2 to 1.0.0 in /src/rust/engine ([pantsbuild#14152](pantsbuild#14152)) * [internal] Introduce new `BuiltinGoal` subsystem type. ([pantsbuild#13991](pantsbuild#13991)) * [internal] Upgrade python type stubs packages ([pantsbuild#14143](pantsbuild#14143)) * [internal] Bump strum_macros from 0.20.1 to 0.23.1 in /src/rust/engine ([pantsbuild#14141](pantsbuild#14141)) * [internal] Make `generate-lockfiles` goal generic ([pantsbuild#14122](pantsbuild#14122)) * [internal] Bump errno from 0.2.7 to 0.2.8 in /src/rust/engine ([pantsbuild#14137](pantsbuild#14137)) * [internal] Bump colored from 1.9.3 to 2.0.0 in /src/rust/engine ([pantsbuild#14138](pantsbuild#14138)) * [internal] Bump crossbeam-channel from 0.4.4 to 0.5.0 in /src/rust/engine ([pantsbuild#14139](pantsbuild#14139)) * [internal] Bump reqwest from 0.11.4 to 0.11.9 in /src/rust/engine ([pantsbuild#14135](pantsbuild#14135)) * [internal] Further tweak dependabot config ([pantsbuild#14132](pantsbuild#14132)) * [internal] python: use immutable_input_digests for protobuf codegen ([pantsbuild#13885](pantsbuild#13885)) * [internal] python: use immutable_input_digests for protobuf codegen ([pantsbuild#13885](pantsbuild#13885)) * [internal] No need for two f-strings/two strings. ([pantsbuild#14119](pantsbuild#14119))

Internal changes: * upgrade to Rust v1.58.0 ([pantsbuild#14174](pantsbuild#14174)) * [internal] fix typos in codegen registration ([pantsbuild#14172](pantsbuild#14172)) * Pulls `Coordinate` and `ArtifactRequirements` out into a separate file to avoid a circuilar import later ([pantsbuild#14164](pantsbuild#14164)) * Factors lockfile metadata code into class that supports multiple languages as well as versions ([pantsbuild#14116](pantsbuild#14116)) * Adds `.env` file to make vscode auto-imports work correctly out of the box ([pantsbuild#14130](pantsbuild#14130)) * [internal] Rename classes for `generate_lockfiles.py` for clarity ([pantsbuild#14218](pantsbuild#14218)) * [internal] Fix bad Find+Replace ([pantsbuild#14213](pantsbuild#14213)) * [internal] Bump libc from 0.2.106 to 0.2.112 in /src/rust/engine ([pantsbuild#14206](pantsbuild#14206)) * [internal] Bump tempfile from 3.2.0 to 3.3.0 in /src/rust/engine ([pantsbuild#14202](pantsbuild#14202)) * [internal] Bump criterion from 0.3.3 to 0.3.5 in /src/rust/engine ([pantsbuild#14205](pantsbuild#14205)) * [internal] Bump walkdir from 2.3.1 to 2.3.2 in /src/rust/engine ([pantsbuild#14204](pantsbuild#14204)) * [internal] Bump generic-array from 0.14.4 to 0.14.5 in /src/rust/engine ([pantsbuild#14203](pantsbuild#14203)) * [internal] Remove the minimum bucket size of batching to improve stability. ([pantsbuild#14210](pantsbuild#14210)) * [internal] Make `JvmLockfileRequest` generic ([pantsbuild#14201](pantsbuild#14201)) * [internal] add comment re clippy issue ([pantsbuild#14188](pantsbuild#14188)) * [internal] rename some codegen scopes to put language first ([pantsbuild#14187](pantsbuild#14187)) * [internal] Check for ambiguity when running `generate-lockfiles` ([pantsbuild#14178](pantsbuild#14178)) * [internal] Change JVM lockfile format ([pantsbuild#14175](pantsbuild#14175)) * [internal] go: rewrite third party package analysis to not use `go list` ([pantsbuild#14157](pantsbuild#14157)) * [internal] Hotfix `fmt` crashing on non-formattable targets ([pantsbuild#14168](pantsbuild#14168)) * [internal] Simplify `core/goals/package.py` filesystem API usage ([pantsbuild#14162](pantsbuild#14162)) * [internal] java/thrift: register union that was not registered ([pantsbuild#14159](pantsbuild#14159)) * [internal] Bump arc-swap from 1.3.0 to 1.5.0 in /src/rust/engine ([pantsbuild#14148](pantsbuild#14148)) * [internals] Bump tokio-rustls from 0.22.0 to 0.23.2 in /src/rust/engine ([pantsbuild#14149](pantsbuild#14149)) * [internal] Bump num_cpus from 1.13.0 to 1.13.1 in /src/rust/engine ([pantsbuild#14150](pantsbuild#14150)) * [internal] Bump tokio-util from 0.6.7 to 0.6.9 in /src/rust/engine ([pantsbuild#14151](pantsbuild#14151)) * [internal] Bump os_pipe from 0.9.2 to 1.0.0 in /src/rust/engine ([pantsbuild#14152](pantsbuild#14152)) * [internal] Introduce new `BuiltinGoal` subsystem type. ([pantsbuild#13991](pantsbuild#13991)) * [internal] Upgrade python type stubs packages ([pantsbuild#14143](pantsbuild#14143)) * [internal] Bump strum_macros from 0.20.1 to 0.23.1 in /src/rust/engine ([pantsbuild#14141](pantsbuild#14141)) * [internal] Make `generate-lockfiles` goal generic ([pantsbuild#14122](pantsbuild#14122)) * [internal] Bump errno from 0.2.7 to 0.2.8 in /src/rust/engine ([pantsbuild#14137](pantsbuild#14137)) * [internal] Bump colored from 1.9.3 to 2.0.0 in /src/rust/engine ([pantsbuild#14138](pantsbuild#14138)) * [internal] Bump crossbeam-channel from 0.4.4 to 0.5.0 in /src/rust/engine ([pantsbuild#14139](pantsbuild#14139)) * [internal] Bump reqwest from 0.11.4 to 0.11.9 in /src/rust/engine ([pantsbuild#14135](pantsbuild#14135)) * [internal] Further tweak dependabot config ([pantsbuild#14132](pantsbuild#14132)) * [internal] python: use immutable_input_digests for protobuf codegen ([pantsbuild#13885](pantsbuild#13885)) * [internal] python: use immutable_input_digests for protobuf codegen ([pantsbuild#13885](pantsbuild#13885)) * [internal] No need for two f-strings/two strings. ([pantsbuild#14119](pantsbuild#14119)) * `LockfileMetadata` - Replace flaky `_header_dict()` mechanism with more robust `header_attrs` mechanism ([pantsbuild#14229](pantsbuild#14229))

Internal changes: * upgrade to Rust v1.58.0 ([#14174](#14174)) * [internal] fix typos in codegen registration ([#14172](#14172)) * Pulls `Coordinate` and `ArtifactRequirements` out into a separate file to avoid a circuilar import later ([#14164](#14164)) * Factors lockfile metadata code into class that supports multiple languages as well as versions ([#14116](#14116)) * Adds `.env` file to make vscode auto-imports work correctly out of the box ([#14130](#14130)) * [internal] Rename classes for `generate_lockfiles.py` for clarity ([#14218](#14218)) * [internal] Fix bad Find+Replace ([#14213](#14213)) * [internal] Bump libc from 0.2.106 to 0.2.112 in /src/rust/engine ([#14206](#14206)) * [internal] Bump tempfile from 3.2.0 to 3.3.0 in /src/rust/engine ([#14202](#14202)) * [internal] Bump criterion from 0.3.3 to 0.3.5 in /src/rust/engine ([#14205](#14205)) * [internal] Bump walkdir from 2.3.1 to 2.3.2 in /src/rust/engine ([#14204](#14204)) * [internal] Bump generic-array from 0.14.4 to 0.14.5 in /src/rust/engine ([#14203](#14203)) * [internal] Remove the minimum bucket size of batching to improve stability. ([#14210](#14210)) * [internal] Make `JvmLockfileRequest` generic ([#14201](#14201)) * [internal] add comment re clippy issue ([#14188](#14188)) * [internal] rename some codegen scopes to put language first ([#14187](#14187)) * [internal] Check for ambiguity when running `generate-lockfiles` ([#14178](#14178)) * [internal] Change JVM lockfile format ([#14175](#14175)) * [internal] go: rewrite third party package analysis to not use `go list` ([#14157](#14157)) * [internal] Hotfix `fmt` crashing on non-formattable targets ([#14168](#14168)) * [internal] Simplify `core/goals/package.py` filesystem API usage ([#14162](#14162)) * [internal] java/thrift: register union that was not registered ([#14159](#14159)) * [internal] Bump arc-swap from 1.3.0 to 1.5.0 in /src/rust/engine ([#14148](#14148)) * [internals] Bump tokio-rustls from 0.22.0 to 0.23.2 in /src/rust/engine ([#14149](#14149)) * [internal] Bump num_cpus from 1.13.0 to 1.13.1 in /src/rust/engine ([#14150](#14150)) * [internal] Bump tokio-util from 0.6.7 to 0.6.9 in /src/rust/engine ([#14151](#14151)) * [internal] Bump os_pipe from 0.9.2 to 1.0.0 in /src/rust/engine ([#14152](#14152)) * [internal] Introduce new `BuiltinGoal` subsystem type. ([#13991](#13991)) * [internal] Upgrade python type stubs packages ([#14143](#14143)) * [internal] Bump strum_macros from 0.20.1 to 0.23.1 in /src/rust/engine ([#14141](#14141)) * [internal] Make `generate-lockfiles` goal generic ([#14122](#14122)) * [internal] Bump errno from 0.2.7 to 0.2.8 in /src/rust/engine ([#14137](#14137)) * [internal] Bump colored from 1.9.3 to 2.0.0 in /src/rust/engine ([#14138](#14138)) * [internal] Bump crossbeam-channel from 0.4.4 to 0.5.0 in /src/rust/engine ([#14139](#14139)) * [internal] Bump reqwest from 0.11.4 to 0.11.9 in /src/rust/engine ([#14135](#14135)) * [internal] Further tweak dependabot config ([#14132](#14132)) * [internal] python: use immutable_input_digests for protobuf codegen ([#13885](#13885)) * [internal] python: use immutable_input_digests for protobuf codegen ([#13885](#13885)) * [internal] No need for two f-strings/two strings. ([#14119](#14119)) * `LockfileMetadata` - Replace flaky `_header_dict()` mechanism with more robust `header_attrs` mechanism ([#14229](#14229))

Eric-Arellano added 5 commits January 7, 2022 14:05

Move some lockfile helpers to core/goals/generate_lockfiles.py

73458a5

No changes made. Just moved around. # 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 requested review from stuhood, chrisjrn and tdyas January 8, 2022 00:46

Eric-Arellano commented Jan 8, 2022

View reviewed changes

Move generate-lockfiles goal to core/goals

9790b27

# 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 force-pushed the merge-generate-lockfiles branch from b7d2ca3 to 9790b27 Compare January 8, 2022 00:53

stuhood approved these changes Jan 8, 2022

View reviewed changes

tdyas approved these changes Jan 8, 2022

View reviewed changes

Re-export the Python lockfile @rules from each backend that registe…

b577d11

…rs a `ToolLockfileSentinel`. # 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 enabled auto-merge (squash) January 11, 2022 00:24

stuhood merged commit e9946d3 into pantsbuild:main Jan 11, 2022

illicitonion mentioned this pull request Jan 22, 2022

Prep for 2.10.0.dev2 #14236

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[internal] Make `generate-lockfiles` goal generic #14122

[internal] Make `generate-lockfiles` goal generic #14122

Eric-Arellano commented Jan 8, 2022

Eric-Arellano left a comment

stuhood left a comment

stuhood Jan 8, 2022

Eric-Arellano Jan 8, 2022

stuhood Jan 8, 2022

Eric-Arellano Jan 8, 2022 •

edited

Loading

tdyas Jan 8, 2022

Eric-Arellano commented Jan 8, 2022 •

edited

Loading

stuhood commented Jan 11, 2022 •

edited

Loading

[internal] Make generate-lockfiles goal generic #14122

[internal] Make generate-lockfiles goal generic #14122

Conversation

Eric-Arellano commented Jan 8, 2022

Eric-Arellano left a comment

Choose a reason for hiding this comment

stuhood left a comment

Choose a reason for hiding this comment

stuhood Jan 8, 2022

Choose a reason for hiding this comment

Eric-Arellano Jan 8, 2022

Choose a reason for hiding this comment

stuhood Jan 8, 2022

Choose a reason for hiding this comment

Eric-Arellano Jan 8, 2022 • edited Loading

Choose a reason for hiding this comment

tdyas Jan 8, 2022

Choose a reason for hiding this comment

Eric-Arellano commented Jan 8, 2022 • edited Loading

stuhood commented Jan 11, 2022 • edited Loading

[internal] Make `generate-lockfiles` goal generic #14122

[internal] Make `generate-lockfiles` goal generic #14122

Eric-Arellano Jan 8, 2022 •

edited

Loading

Eric-Arellano commented Jan 8, 2022 •

edited

Loading

stuhood commented Jan 11, 2022 •

edited

Loading