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.

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

Add python_resolve field to protobuf_source and thrift_source to support multiple resolves with codegen (Cherry-pick of #14693) #14698

Merged
merged 1 commit into from
Mar 4, 2022

Conversation

Eric-Arellano
Copy link
Contributor

Closes #14484.

Problem

Generated code needs associated runtime libraries, e.g. protobuf. Those must come from a python_requirement which by definition have a single resolve.

Before this PR, it was impossible to have >1 python_requirement target used for each runtime library and every protobuf_source target would depend on the same python_requirement. Practically, this means codegen could only work with code belonging to a single resolve.

Solution

Add a python_resolve field to codegen targets so that they can indicate which python_requirement they should be using. Our new dependency inference from #14691 wires this up properly.

If you want the same generated code to work with multiple resolves, you will need to use parametrize. That gets tricky when you consider a codegen target generating for multiple languages, as explained at the bottom of #14484, but there is a decent workaround via configurations. That will be tackled in a followup.

FYI: lazy validation of runtime library

#14691 asserts that there is exactly 1 runtime library in the project; now we assert that for each resolve.

Key nuance: this check is lazy. If you have 2 resolves, but your codegen targets only use 1 of the 2, then we will never end up checking that there is a runtime library defined for the 2nd resolve. That is important so that, for example, if you have a pants-plugins resolve we don't force you to add protobuf unnecessarily to it.

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

…o support multiple resolves with codegen (pantsbuild#14693)

Closes pantsbuild#14484.

## Problem

Generated code needs associated runtime libraries, e.g. `protobuf`. Those must come from a `python_requirement` which by definition have a single `resolve`.

Before this PR, it was impossible to have >1 `python_requirement` target used for each runtime library and every `protobuf_source` target would depend on the same `python_requirement`. Practically, this means codegen could only work with code belonging to a single resolve.

## Solution

Add a `python_resolve` field to codegen targets so that they can indicate which `python_requirement` they should be using. Our new dependency inference from pantsbuild#14691 wires this up properly.

If you want the same generated code to work with multiple resolves, you will need to use `parametrize`. That gets tricky when you consider a codegen target generating for multiple languages, as explained at the bottom of pantsbuild#14484, but there is a decent workaround via configurations. That will be tackled in a followup.

### FYI: lazy validation of runtime library

pantsbuild#14691 asserts that there is exactly 1 runtime library in the project; now we assert that for each resolve. 

Key nuance: this check is lazy. If you have 2 resolves, but your codegen targets only use 1 of the 2, then we will never end up checking that there is a runtime library defined for the 2nd resolve. That is important so that, for example, if you have a `pants-plugins` resolve we don't force you to add `protobuf` unnecessarily to it.

[ci skip-rust]
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano requested a review from tdyas March 4, 2022 00:51
@Eric-Arellano
Copy link
Contributor Author

Note that this is cherry-picking from 2.10 to main.

@Eric-Arellano Eric-Arellano merged commit 5db4b72 into pantsbuild:main Mar 4, 2022
@Eric-Arellano Eric-Arellano deleted the cp-codegen-resolve branch March 4, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Figure out codegen's runtime_dependencies and multiple resolves
2 participants