Skip to content

Conversation

iczero
Copy link

@iczero iczero commented Jul 5, 2025

Updating prost in xds-api causes a catch-22 situation where the crate needs to be built in order to run generate_sources (which is a test), but the crate cannot be built since the new prost is not API-compatible with the old generated code. I have added a generate_only internal feature to be used when prost-build needs to be run for a breaking prost upgrade. This effectively disables pretty much the entire crate.

XDS_API_SKIP_GEN_SRC_DIRTY_CHECK=1 PROTOC="/path/to/protoc-31.1/bin/protoc" \
  cargo test --features generate_only -- generate_sources

Additionally, opencensus was removed because it is gone from upstream: envoyproxy/data-plane-api@040c2d8, and generate_sources now checks the PROTOC env var.

Please note that I have not tested this except for cargo test. It may be best to not merge yet.

@blinsay
Copy link
Member

blinsay commented Jul 7, 2025

Thanks for taking a look!

This is definitely something we've also run into but haven't had a reason to come clean up yet. I'm not inclined to merge this, since I think we can do better than the feature - I've been thinking about moving codegen back out of the tests and either back into build.rs or into a separate xtask crate.

Before I do any of that, are you also actively interested in the protoc upgrade or was that just a nice-to-have while upgrading everything else?

@blinsay
Copy link
Member

blinsay commented Jul 7, 2025

I should also ask: are you interested in just the protobufs or in the tonic api as well? I've been thinking about whether or not it's useful to split those into separate features.

@iczero
Copy link
Author

iczero commented Jul 7, 2025

The protoc upgrade was indeed just a nice to have. I personally am interested in using the tonic APIs.

In git history I saw that the generation step was previously in an xtask. Is there a reason it was moved into a test? I assumed it wasn't in build.rs since that would run generation on install and hence require the correct protoc version to be present.

@blinsay
Copy link
Member

blinsay commented Jul 7, 2025

In git history I saw that the generation step was previously in an xtask. Is there a reason it was moved into a test?

I don't remember why we decided not to do an xtask at the time, but do remember borrowing the test approach from tokio-console. I'll put up a PR in the next couple days with the generation moved back into an xtask and go from there.

Thanks for opening the PR!

@blinsay
Copy link
Member

blinsay commented Aug 18, 2025

Sorry about the delay - should have time to start landing all of this shortly.

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.

2 participants