Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[internal] Make JvmLockfileRequest generic #14201

Merged
merged 2 commits into from
Jan 19, 2022
Merged

[internal] Make JvmLockfileRequest generic #14201

merged 2 commits into from
Jan 19, 2022

Conversation

Eric-Arellano
Copy link
Contributor

It before assumed that we're working with JvmToolBase, which got in the way of unifying user lockfiles to use generate-lockfiles.

[ci skip-rust]

@Eric-Arellano
[internal] Make JvmLockfileRequest generic
2d95dee 
# 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 Eric-Arellano enabled auto-merge (squash) January 19, 2022 00:36
@Eric-Arellano
Work around test
513af6a 
This was always broken, but now we're more eagerly validating the address so it makes a difference.

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
Eric-Arellano
Eric-Arellano commented Jan 19, 2022
View reviewed changes
src/python/pants/backend/scala/compile/scalac_test.py
@@ -384,7 +384,8 @@ def test_compile_with_scalac_plugin(rule_runner: RuleRunner) -> None:

scalac_plugin(
name = "acyclic",
artifact = ":acyclic_lib",
# TODO: Support relative addresses.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was always broken, but now we're more eagerly validating the address so it makes a difference. I can try fixing in a followup? We need to use AddressInput.parse(relative_to)

@chrisjrn
Copy link
Contributor

This looks like a good change and I'll go through and review assuming it'll get accepted momentarily.

I'm noticing some similarities between this work and the work I did over in #14185 adding ValidatedJvmToolLockfileRequest, and wondering if there might be a way to streamline the process of going from a JvmLockfileRequest for a tool to validating and returning a CoursierResolvedLockfile.

Right now, it looks like there's possibly more indirection and genericness than is necessary and we may want to investigate a bit more streamlining once both of these PRs are landed.

chrisjrn
chrisjrn approved these changes Jan 19, 2022
View reviewed changes
Copy link
Contributor

@chrisjrn chrisjrn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine

src/python/pants/backend/scala/compile/scalac_plugins.py
ArtifactRequirements,
GatherJvmCoordinatesRequest(
FrozenOrderedSet(loaded_global_plugins.artifact_address_inputs),
f"[{scalac_plugins.options_scope}].plugins_global",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm starting to see a lot of GatherJvmCoordinatesRequests with that f-string, is there an opportunity to create a util here to make sure these requests are more easily reused?

src/python/pants/jvm/resolve/jvm_tool_test.py
@@ -83,16 +84,8 @@ def test_jvm_tool_base_extracts_correct_coordinates() -> None:
}
)
lockfile_request = rule_runner.request(JvmLockfileRequest, [MockJvmToolLockfileSentinel()])
assert sorted(lockfile_request.artifact_inputs) == [
"//:junit_junit",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One side-effect of this assert here is that we validated that both Maven-style and Address-style artifact specifications were tests. Are we doing this anywhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, I had on auto-merge. Will make sure the followup PR has tests for this.

@Eric-Arellano Eric-Arellano merged commit 12147c1 into pantsbuild:main Jan 19, 2022
@Eric-Arellano Eric-Arellano deleted the generify-jvm-lockfiles branch January 19, 2022 20:04
illicitonion added a commit to illicitonion/pants that referenced this pull request Jan 22, 2022
@illicitonion
Prep for 2.10.0.dev2
bcb22d6 
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))
@illicitonion illicitonion mentioned this pull request Jan 22, 2022
Merged
illicitonion added a commit to illicitonion/pants that referenced this pull request Jan 22, 2022
@illicitonion
Prep for 2.10.0.dev2
6071ee3 
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))
illicitonion added a commit to illicitonion/pants that referenced this pull request Jan 22, 2022
@illicitonion
Prep for 2.10.0.dev2
237e3e1 
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))
illicitonion added a commit that referenced this pull request Jan 22, 2022
@illicitonion
Prep for 2.10.0.dev2 (#14236)
4113608 
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))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisjrn chrisjrn chrisjrn approved these changes

@stuhood stuhood Awaiting requested review from stuhood

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Eric-Arellano @chrisjrn