Move protoc generation to binary crate, unpin prost/tonic build (#3876) #3927
Conversation
| path::Path, | ||
| }; | ||
|
|
||
| fn main() -> Result<(), Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
This is just the code from build.rs, but with the conditionals on path existence removed (as we want it to error if it can't find the protobuf definitions)
alamb
changed the title
arrow-flight/CONTRIBUTING.md
Outdated
| ./regen.sh | ||
| ``` | ||
|
|
||
| The above script will run the Rust binary located in [gen](./gen) |
There was a problem hiding this comment.
I think it would be useful to put a note here on the context of why this doesn't use the standard protoc process (i.e. so that the code can be vendored and not automatically change if people have different versions of protoc)
It might be helpful to explicitly note that now any changes to protobuf files will require running these scripts manually
.github/workflows/arrow_flight.yml
Outdated
| - name: Setup Rust toolchain | ||
| uses: ./.github/actions/setup-builder | ||
| - name: Run gen | ||
| run: ./arrow-flight/regen.sh | ||
| - name: Verify workspace clean |
There was a problem hiding this comment.
| - name: Verify workspace clean | |
| - name: Verify workspace clean (if this fails, run ./arrow-flight/regen.sh and check in results) |
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| [package] |
There was a problem hiding this comment.
It might be worth pointing out here that this crate is not meant to be distributed
| @@ -64,13 +64,6 @@ tempfile = "3.3" | |||
| tokio-stream = { version = "0.1", features = ["net"] } | |||
| tower = "0.4.13" | |||
|
|
|||
| [build-dependencies] | |||
| # Pin specific version of the tonic-build dependencies to avoid auto-generated | |||
github-actions
bot
added
arrow
|
FYI I've filled a corresponding ticket in DataFusion - apache/datafusion#5718 |
Which issue does this PR close?
Closes #3876
Rationale for this change
Inspired by @sunng87 's work in #3926
What changes are included in this PR?
Are there any user-facing changes?