Skip to content

Commit

Permalink
Do not run codegen in build.rs scripts (#3874)
Browse files Browse the repository at this point in the history
### What
Running codegen from our `build.rs` was a problem, because it required
specific versions of specific tools to be installed (`clang-format`,
`black`, etc). It is better to run the codegen with `just codegen`,
which will ensure the correct tools are installed via `pixi`.

With this PR you now have to run `just codegen` manually each time you
change `re_types_builder` or any `.fbs`. If you forget to do so, the CI
will catch it for you.

This PR removes `crates/re_types/build.rs`, but does NOT touch
`crates/re_types_builder/build.rs`.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3874) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/3874)
- [Docs
preview](https://rerun.io/preview/fdfd159d5b2d8f7a089d3b69d05b2a6c33c24a2b/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/fdfd159d5b2d8f7a089d3b69d05b2a6c33c24a2b/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
  • Loading branch information
emilk authored Oct 16, 2023
1 parent 9a4d833 commit f292a5c
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 164 deletions.
1 change: 1 addition & 0 deletions .github/workflows/contrib_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: codegen
args: --force

- name: No Diffs From Running Codegen
run: |
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/reusable_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: codegen
args: --force

- name: No Diffs From Running Codegen
run: |
Expand Down
7 changes: 0 additions & 7 deletions crates/re_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,6 @@ serde = ["dep:serde", "re_string_interner/serde"]
## Only useful for testing purposes.
testing = []

## Special hack!
##
## If you configure rust-analyzer to set `--all-features,
## then this feature will be set and that will ensure that
## `rust-analyzer` won't run the (expensive) `build.rs`!
__opt_out_of_auto_rebuild = []


[dependencies]
# Rerun
Expand Down
2 changes: 2 additions & 0 deletions crates/re_types/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ Part of the [`rerun`](https://github.com/rerun-io/rerun) family of crates.
The standard Rerun data types, component types, and archetypes.

This crate includes both the language-agnostic definitions (flatbuffers IDL) as well as the generated code.

Generated the code with `just codegen`.
135 changes: 0 additions & 135 deletions crates/re_types/build.rs

This file was deleted.

3 changes: 2 additions & 1 deletion crates/re_types_builder/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ Part of the [`rerun`](https://github.com/rerun-io/rerun) family of crates.
This crate implements Rerun's code generation tools.

These tools translate language-agnostic IDL definitions (flatbuffers) into code.
They are invoked from `re_types`' build script (`build.rs`).

You can generate the code with `just codegen`.
50 changes: 35 additions & 15 deletions crates/re_types_builder/src/bin/build_re_types.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
//! Helper binary for running the codegen manually. Useful during development!
//! This binary runs the codegen manually.
//!
//! It is easiest to call this using `just codegen`,
//! which will set up the necessary tools.
use camino::Utf8Path;
use re_build_tools::{
read_versioning_hash, set_output_cargo_build_instructions, write_versioning_hash,
};
use re_types_builder::{compute_re_types_builder_hash, compute_re_types_hash, SourceLocations};
use re_types_builder::{compute_re_types_hash, SourceLocations};

use camino::Utf8Path;

const RE_TYPES_BUILDER_SOURCE_HASH_PATH: &str = "crates/re_types_builder/source_hash.txt";
const RE_TYPES_SOURCE_HASH_PATH: &str = "crates/re_types/source_hash.txt";
const DEFINITIONS_DIR_PATH: &str = "crates/re_types/definitions";
const ENTRYPOINT_PATH: &str = "crates/re_types/definitions/rerun/archetypes.fbs";
Expand All @@ -31,6 +34,7 @@ macro_rules! join {

fn main() {
re_log::setup_native_logging();

// This isn't a build.rs script, so opt out of cargo build instrctinsr
set_output_cargo_build_instructions(false);

Expand All @@ -40,13 +44,17 @@ fn main() {
.unwrap();

let mut profiler = re_tracing::Profiler::default();
let mut always_run = false;

for arg in std::env::args().skip(1) {
match arg.as_str() {
"--help" => {
println!("Usage: [--help] [--profile]");
println!("Usage: [--help] [--force] [--profile]");
return;
}
"--force" => {
always_run = true;
}
"--profile" => profiler.start(),
_ => {
eprintln!("Unknown argument: {arg:?}");
Expand All @@ -61,28 +69,41 @@ fn main() {
.unwrap();

let re_types_source_hash_path = workspace_dir.join(RE_TYPES_SOURCE_HASH_PATH);
let re_types_builder_source_hash_path = workspace_dir.join(RE_TYPES_BUILDER_SOURCE_HASH_PATH);
let definitions_dir_path = workspace_dir.join(DEFINITIONS_DIR_PATH);
let entrypoint_path = workspace_dir.join(ENTRYPOINT_PATH);
let doc_examples_dir_path = workspace_dir.join(DOC_EXAMPLES_DIR_PATH);
let cpp_output_dir_path = workspace_dir.join(CPP_OUTPUT_DIR_PATH);
let rust_output_dir_path = workspace_dir.join(RUST_OUTPUT_DIR_PATH);
let python_output_dir_path = workspace_dir.join(PYTHON_OUTPUT_DIR_PATH);
let python_testing_output_dir_path = workspace_dir.join(PYTHON_TESTING_OUTPUT_DIR_PATH);
let docs_content_dir_path = workspace_dir.join(DOCS_CONTENT_DIR_PATH);

let cur_hash = read_versioning_hash(&re_types_source_hash_path);
eprintln!("cur_hash: {cur_hash:?}");

let builder_hash = compute_re_types_builder_hash();
re_log::debug!("cur_hash: {cur_hash:?}");

let new_hash = compute_re_types_hash(&SourceLocations {
definitions_dir: definitions_dir_path.as_str(),
doc_examples_dir: doc_examples_dir_path.as_str(),
python_output_dir: python_output_dir_path.as_str(),
cpp_output_dir: cpp_output_dir_path.as_str(),
definitions_dir: DEFINITIONS_DIR_PATH,
doc_examples_dir: DOC_EXAMPLES_DIR_PATH,
python_output_dir: PYTHON_OUTPUT_DIR_PATH,
cpp_output_dir: CPP_OUTPUT_DIR_PATH,
});

if let Some(cur_hash) = cur_hash {
if cur_hash == new_hash {
if always_run {
re_log::info!(
"The hash hasn't changed, but --force was passed, so we'll run anyway."
);
} else {
re_log::info!("Returning early: no changes detected (and --force wasn't set).");
return;
}
} else {
re_log::info!("Change detected");
}
} else {
re_log::info!("Missing {re_types_source_hash_path:?} (first time running codegen)");
}

re_log::info!("Running codegen…");
let (report, reporter) = re_types_builder::report::init();

Expand Down Expand Up @@ -121,7 +142,6 @@ fn main() {
report.finalize();

write_versioning_hash(re_types_source_hash_path, new_hash);
write_versioning_hash(re_types_builder_source_hash_path, builder_hash);

re_log::info!("Done.");
}
12 changes: 6 additions & 6 deletions crates/re_types_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,12 @@ pub fn compute_re_types_hash(locations: &SourceLocations<'_>) -> String {
&cpp_extensions_hash,
]);

eprintln!("re_types_builder_hash: {re_types_builder_hash:?}");
eprintln!("definitions_hash: {definitions_hash:?}");
eprintln!("doc_examples_hash: {doc_examples_hash:?}");
eprintln!("python_extensions_hash: {python_extensions_hash:?}");
eprintln!("cpp_extensions_hash: {cpp_extensions_hash:?}");
eprintln!("new_hash: {new_hash:?}");
re_log::debug!("re_types_builder_hash: {re_types_builder_hash:?}");
re_log::debug!("definitions_hash: {definitions_hash:?}");
re_log::debug!("doc_examples_hash: {doc_examples_hash:?}");
re_log::debug!("python_extensions_hash: {python_extensions_hash:?}");
re_log::debug!("cpp_extensions_hash: {cpp_extensions_hash:?}");
re_log::debug!("new_hash: {new_hash:?}");

new_hash
}
Expand Down

0 comments on commit f292a5c

Please sign in to comment.