Skip to content

chore(CI): Cache well-known-type protos with protoc binary#2683

Closed
arjan-bal wants to merge 1 commit into
grpc:masterfrom
arjan-bal:ci-one-protoc
Closed

chore(CI): Cache well-known-type protos with protoc binary#2683
arjan-bal wants to merge 1 commit into
grpc:masterfrom
arjan-bal:ci-one-protoc

Conversation

@arjan-bal

Copy link
Copy Markdown
Contributor

Background: In #2665, the well-known-type protos were included in the output of the protoc-gen-rust-grpc crate, but CI only cached protoc and protoc-gen-rust-grpc.

Changes in this PR:

  • Caches the included protos to easily reference them in the upcoming Google creds example (examples(grpc): Add example for Google Cloud Platform client #2680).
  • Removes the redundant protoc installation via the taiki-e/install-action@protoc action, since prost-build now works with the cached binary.
  • Ensures the protoc plugin is rebuilt whenever the workflow file changes, even if the source code remains untouched.

@ejona86

ejona86 commented Jun 15, 2026

Copy link
Copy Markdown
Member

Ah, so you were looking at this as well. I just sent out #2685

I think with my change it'd be okay to not include .github/workflows/CI.yml in the cache key, since there is so little logic in CI.yml with my change. I didn't notice/remove taiki-e/install-action@protoc. Is that installed protoc earlier in the PATH, so we're only using the cached protoc-gen-rust-grpc today?

@arjan-bal

Copy link
Copy Markdown
Contributor Author

Ah, so you were looking at this as well. I just sent out #2685

I think with my change it'd be okay to not include .github/workflows/CI.yml in the cache key, since there is so little logic in CI.yml with my change. I didn't notice/remove taiki-e/install-action@protoc. Is that installed protoc earlier in the PATH, so we're only using the cached protoc-gen-rust-grpc today?

I had asked Doug to remove it in an earlier PR, but tonic's prost codegen relied on well-known type protos that were missing in the compiled protoc. Now that we're including those, we don't need the install action.

@arjan-bal

Copy link
Copy Markdown
Contributor Author

Closing in favour of #2685.

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