Source code generation policy

[Mirko-von-Leipzig]

We generate code in a few build.rs across our various repos. Our current policy has been to generate this code in-tree, allowing us to audit this as part of code reviews in PRs.

General guidelines

This policy does go against the common guideline that code generation should be done out-of-tree and never checked into source control. This is also the policy that the Rust reference suggests:

Build scripts may save any output files or intermediate artifacts in the directory specified in the OUT_DIR environment variable. Scripts should not modify any files outside of that directory.

Why revisit this

Our policy has bitten us recently. Additional code generation was added to build.rs, and the generated files were added to the rerun-if-changed pragma (which is perfectly valid). However this caused the build script to always trigger, causing a full recompile each time. This was patched in #991, but I have to admit I still don't fully understand why it was happening. Probably it didn't like a trigger of a generated file; though that feels like it should be allowed to prevent accidental mutation post-generation.

Pros/Cons of current policy

Positives:

• audit of generated code
• generated code is (more) static between computers ("works on my machine syndrome")

Negatives:

• requires additional CI checks to ensure checked-in code matches generated code
• does not follow guidelines, can lead to silent issues like the above and doubts
• PRs are noisier e.g. have to review proto files and generated proto code (not really, but its there); sort of the corollary of our pro :)

[Mirko-von-Leipzig]

Personally I prefer the out-of-tree approach. But I've been significantly jaded by CMake's handling of such things; where it regularly runs afoul of similar issues with rebuilding and miscompiling.

If we are concerned with specific generated code audits e.g. rust files that we manually write our in the build script, then we could experiment auditing this by moving the format string into its own separate file. This file can then be include! by the build script's write!(format!(...)) invocation. The benefit would be we could inspect the "file" in its non-string form, where the only missing parts would be the placeholder {} parts. This should still give syntax highlighting if we exclude the outer quotes.

^^ haven't actually tried this; but I don't see why it wouldn't simply work :)

[bobbinth]

I somewhat prefer the in-tree approach because of the reasons you've listed in "positives" - i.e., the code that gets checked into the repo is all you need to build the project. But I also understand the downsides - so, this is not a super strong preference.

I wonder if we could simplify the rule to be something like: running cargo build and similar command should not execute the build scripts at all (e.g,. by gating them with env variables) - they get executed only via the make commands only.

This way, if I want to re-build some parts of the source code, I can always run make build but cargo build always works with "static" code. This may address the second point from your "negatives" list - though, the other two remain as is.