fix(protos): honor PROTOC for protobuf codegen - #2370
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79e62c3100
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Binary Size Analysis (Agent Data Plane)Baseline: bfbd168 · Comparison: b1bc60b · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
There was a problem hiding this comment.
When PROTOC points to a compiler outside PATH, the helper sets its path but does not select the protoc parser. The build script fails before it invokes the compiler, so the hermetic-build case remains broken.
🤖 Datadog Autotest · Commit 79e62c3 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
## Summary Use the conventional `PROTOC` environment variable for the `protobuf-codegen` invocations in `datadog-protos` when it is set. Continue searching `PATH` when it is unset so existing Cargo builds keep working. Also declare `PROTOC` as a build-script input so Cargo reruns code generation when it changes. This allows hermetic build systems such as Bazel to provide an explicitly declared compiler instead of requiring `protoc` on the host `PATH`. ## Change Type - [x] Bug fix - [ ] New feature - [ ] Non-functional (chore, refactoring, docs) - [ ] Performance ## How did you test this PR? Ran the `datadog-protos` check both with an explicit compiler and with the existing `PATH` fallback: ```console PROTOC=/opt/homebrew/bin/protoc cargo check -p datadog-protos env -u PROTOC cargo check -p datadog-protos ``` Co-authored-by: ilya.rubnich <ilya.rubnich@datadoghq.com> d7046b3
Summary
Use the conventional
PROTOCenvironment variable for theprotobuf-codegeninvocations indatadog-protoswhen it is set. Continue searchingPATHwhen it is unset so existing Cargo builds keep working.Also declare
PROTOCas a build-script input so Cargo reruns code generation when it changes.This allows hermetic build systems such as Bazel to provide an explicitly declared compiler instead of requiring
protocon the hostPATH.Change Type
How did you test this PR?
Ran the
datadog-protoscheck both with an explicit compiler and with the existingPATHfallback: