fix: move proto code generation output to OUT_DIR, out of tree#1703
fix: move proto code generation output to OUT_DIR, out of tree#1703
OUT_DIR, out of tree#1703Conversation
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
I think is better and minimizes the review diffs when proto changes are involved.
It also makes sense to me to drop the generated/mod.rs codegen but as you say that probably makes more sense as a separate PR. This file can be created manually.
This looks correct; but PR is still in draft and description looks semi-done so I'm not approving yet.
34b453c to
d5326f2
Compare
|
Added CI and Makefile changes, which were the last missing pieces. |
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
I would hold off on merging this until @bobbinth agrees with this in principle
| - uses: ./.github/actions/install-rocksdb | ||
| - uses: ./.github/actions/install-protobuf-compiler |
There was a problem hiding this comment.
Given that these are now married at the hip, should we have a single install-system-prerequisites?
There was a problem hiding this comment.
One CTRL-K too much. Fixed
I considered merging them, but wasn't sure if won't get more changes in rocksdb when I migrate the LargeSmtForest backend (!= LargeSmt backend).
Motivation
Committing generated protobuf code is discouraged and creates review churn. Moving artifacts to
OUT_DIRremoves ~10k LOC from the repo and avoids frequent diffs when protobuf definitions change.rust-analyzerresolvesinclude!paths, so location is no longer a blocker when it comes to symbol resolution / editor support.Implementation
We keep the existing integration wiring by continuing to use i.e.
generated/mod.rsandinclude!(concat!(env!("OUT_DIR"), "/generated/…")). This minimizes import-path changes across the codebase and keeps the diff concise.Side effects
docs.rs-specificBUILD_PROTO=1special casing.Risks