Refactor our build.rs files

crates/re_analytics/build.rs

@@ -1,4 +1,3 @@
 fn main() {
-    re_build_tools::rebuild_if_crate_changed("re_analytics");
-    re_build_tools::export_env_vars();
+    re_build_tools::export_build_info_vars_for_crate("re_analytics");
 }

crates/re_build_tools/src/lib.rs

@@ -1,8 +1,6 @@
 #![allow(clippy::unwrap_used)]
 
 //! This crate is to be used from `build.rs` build scripts.
-//!
-//! Use this crate together with the `re_build_info` crate.
 
 use anyhow::Context as _;
 
@@ -36,31 +34,85 @@ pub(crate) fn should_output_cargo_build_instructions() -> bool {
     OUTPUT_CARGO_BUILD_INSTRUCTIONS.load(Ordering::Relaxed)
 }
 
-// Situations to consider
-// ----------------------
-//
-// # Using the published crate
-//
-// The published crate carries its version around, which in turns gives us the git tag, which makes
-// the commit hash irrelevant.
-// We still need to compute _something_ so that we can actually build, but that value will be
-// ignored when the crate is built by the end user anyhow.
-//
-// # Working directly within the workspace
-//
-// When working within the workspace, we can simply try and call `git` and we're done.
-//
-// # Using an unpublished crate (e.g. `path = "…"` or `git = "…"` or `[patch.crates-io]`)
-//
-// In these cases we may or may not have access to the workspace (e.g. a `path = …` import likely
-// will, while a crate patch won't).
-//
-// This is not an issue however, as we can simply try and see what we get.
-// If we manage to compute a commit hash, great, otherwise we still have the crate version to
-// fallback on.
+/// Where is this `build.rs` build script running?
+pub enum Environment {
+    /// We are running `cargo publish` (via `scripts/ci/crates.py`); _probably_ on CI.
+    PublishingCrates,
+
+    /// We are running on CI, but NOT publishing crates
+    CI,
+
+    /// Are we a developer running inside the workspace of <https://github.com/rerun-io/rerun> ?
+    DeveloperInWorkspace,
+
+    /// We are not on CI, and not in the Rerun workspace.
+    ///
+    /// This is _most likely_ a Rerun user who is compiling a `re_` crate
+    /// because they depend on it either directly or indirectly in their `Cargo.toml`,
+    /// or they running `cargo install rerun-cli` or other tool that depend on a `re_` crate.
+    ///
+    /// In these cases we should do as little shenanigans in the `build.rs` as possible.
+    UsedAsDependency,
+}
+
+impl Environment {
+    /// Detect what environment we are running in.
+    pub fn detect() -> Self {
+        if is_tracked_env_var_set("RERUN_IS_PUBLISHING") {
+            // "RERUN_IS_PUBLISHING" is set by `scripts/ci/crates.py`
+            eprintln!("Environment: env-var RERUN_IS_PUBLISHING is set");
+            Self::PublishingCrates
+        } else if is_on_ci() {
+            // `CI` is an env-var set by GitHub actions.
+            eprintln!("Environment: env-var CI is set");
+            Self::CI
+        } else if is_tracked_env_var_set("IS_IN_RERUN_WORKSPACE") {
+            // IS_IN_RERUN_WORKSPACE is set by `.cargo/config.toml` and also in the Rust-analyzer settings in `.vscode/settings.json`
+            eprintln!("Environment: env-var IS_IN_RERUN_WORKSPACE is set");
+            Self::DeveloperInWorkspace
+        } else {
+            eprintln!("Environment: Not on CI anmd not in workspace");
+            Self::UsedAsDependency
+        }
+    }
+}
+
+/// Are we running on a CI machine?
+pub fn is_on_ci() -> bool {
+    // `CI` is an env-var set by GitHub actions.
+    std::env::var("CI").is_ok()
+}
 
 /// Call from the `build.rs` file of any crate you want to generate build info for.
-pub fn export_env_vars() {
+///
+/// Use this crate together with the `re_build_info` crate.
+pub fn export_build_info_vars_for_crate(crate_name: &str) {
+    rebuild_if_crate_changed(crate_name);
+    export_build_info_env_vars();
+}
+
+/// # Situations to consider regarding git
+///
+/// ## Using the published crate
+///
+/// The published crate carries its version around, which in turns gives us the git tag, which makes
+/// the commit hash irrelevant.
+/// We still need to compute _something_ so that we can actually build, but that value will be
+/// ignored when the crate is built by the end user anyhow.
+///
+/// ## Working directly within the workspace
+///
+/// When working within the workspace, we can simply try and call `git` and we're done.
+///
+/// ## Using an unpublished crate (e.g. `path = "…"` or `git = "…"` or `[patch.crates-io]`)
+///
+/// In these cases we may or may not have access to the workspace (e.g. a `path = …` import likely
+/// will, while a crate patch won't).
+///
+/// This is not an issue however, as we can simply try and see what we get.
+/// If we manage to compute a commit hash, great, otherwise we still have the crate version to
+/// fallback on.
+fn export_build_info_env_vars() {
     // target triple
     set_env("RE_BUILD_TARGET_TRIPLE", &std::env::var("TARGET").unwrap());
     set_env("RE_BUILD_GIT_HASH", &git_hash().unwrap_or_default());
@@ -74,9 +126,8 @@ pub fn export_env_vars() {
     // We need to check `IS_IN_RERUN_WORKSPACE` in the build-script (here),
     // because otherwise it won't show up when compiling through maturin.
     // We must also make an exception for when we build actual wheels (on CI) for release.
-    if std::env::var("CI").is_ok() {
-        // Probably building wheels on CI.
-        // `CI` is an env-var set by GitHub actions.
+    if is_on_ci() {
+        // e.g. building wheels on CI.
         set_env("RE_BUILD_IS_IN_RERUN_WORKSPACE", "no");
     } else {
         set_env(

crates/re_build_tools/src/rebuild_detector.rs

@@ -9,19 +9,33 @@ use cargo_metadata::{CargoOpt, Metadata, MetadataCommand, Package, PackageId};
 
 use crate::should_output_cargo_build_instructions;
 
+fn should_run() -> bool {
+    #![allow(clippy::match_same_arms)]
+    use super::Environment;
+
+    match Environment::detect() {
+        // We cannot run this during publishing,
+        // we don't need to,
+        // and it can also can cause a Cargo.lock file to be generated.
+        Environment::PublishingCrates => false,
+
+        // Dependencies shouldn't change on CI, but who knows 🤷‍♂️
+        Environment::CI => true,
+
+        Environment::DeveloperInWorkspace => true,
+
+        // Definitely not
+        Environment::UsedAsDependency => false,
+    }
+}
+
 /// Call from `build.rs` to trigger a rebuild whenever any source file of the given package
 /// _or any of its dependencies_ changes, recursively.
 ///
 /// This will work even if the package depends on crates that are outside of the workspace,
 /// included with `path = …`
 pub fn rebuild_if_crate_changed(pkg_name: &str) {
-    if !is_tracked_env_var_set("IS_IN_RERUN_WORKSPACE") {
-        // Only run if we are in the rerun workspace, not on users machines.
-        return;
-    }
-    if is_tracked_env_var_set("RERUN_IS_PUBLISHING") {
-        // We cannot run this during publishing.
-        // We don't need to, and it can also can cause a Cargo.lock file to be generated.
+    if !should_run() {
         return;
     }
 
@@ -40,19 +54,28 @@ pub fn rebuild_if_crate_changed(pkg_name: &str) {
     }
 }
 
-/// Call from `build.rs` to trigger a rebuild whenever an environment variable changes.
+/// Read the environment variable and trigger a rebuild whenever the environment variable changes.
 pub fn get_and_track_env_var(env_var_name: &str) -> Result<String, std::env::VarError> {
     if should_output_cargo_build_instructions() {
         println!("cargo:rerun-if-env-changed={env_var_name}");
     }
     std::env::var(env_var_name)
 }
 
-/// Call from `build.rs` to trigger a rebuild whenever an environment variable changes, and returns
-/// true if that variable has been set to a truthy value.
+/// Read the environment variable and trigger a rebuild whenever the environment variable changes.
+///
+/// Returns `true` if that variable has been set to a truthy value.
 pub fn is_tracked_env_var_set(env_var_name: &str) -> bool {
-    let var = get_and_track_env_var(env_var_name).map(|v| v.to_lowercase());
-    var == Ok("1".to_owned()) || var == Ok("yes".to_owned()) || var == Ok("true".to_owned())
+    match get_and_track_env_var(env_var_name) {
+        Err(_) => false,
+        Ok(value) => match value.to_lowercase().as_str() {
+            "1" | "yes" | "true" => true,
+            "0" | "no" | "false" => false,
+            _ => {
+                panic!("Failed to understand boolean env-var {env_var_name}={value}");
+            }
+        },
+    }
 }
 
 /// Call from `build.rs` to trigger a rebuild whenever the file at `path` changes.

crates/re_data_source/build.rs

@@ -1,4 +1,3 @@
 fn main() {
-    re_build_tools::rebuild_if_crate_changed("re_data_source");
-    re_build_tools::export_env_vars();
+    re_build_tools::export_build_info_vars_for_crate("re_data_source");
 }

crates/re_renderer/build.rs

@@ -19,7 +19,7 @@ use std::path::{Path, PathBuf};
 use anyhow::{bail, ensure, Context as _};
 use walkdir::{DirEntry, WalkDir};
 
-use re_build_tools::{is_tracked_env_var_set, rerun_if_changed, write_file_if_necessary};
+use re_build_tools::{get_and_track_env_var, rerun_if_changed, write_file_if_necessary};
 
 // ---
 
@@ -101,21 +101,25 @@ fn check_hermeticity(root_path: impl AsRef<Path>, file_path: impl AsRef<Path>) {
 
 // ---
 
-fn main() {
-    if std::env::var("CI").is_ok() {
-        // Don't run on CI!
-        //
-        // The code we're generating here is actual source code that gets committed into the
-        // repository.
-        return;
-    }
-    if !is_tracked_env_var_set("IS_IN_RERUN_WORKSPACE") {
-        // Only run if we are in the rerun workspace, not on users machines.
-        return;
+fn should_run() -> bool {
+    #![allow(clippy::match_same_arms)]
+    use re_build_tools::Environment;
+
+    match Environment::detect() {
+        // we should have been run before publishing
+        Environment::PublishingCrates => false,
+
+        // The code we're generating here is actual source code that gets committed into the repository.
+        Environment::CI => false,
+
+        Environment::DeveloperInWorkspace => true,
+
+        Environment::UsedAsDependency => false,
     }
-    if is_tracked_env_var_set("RERUN_IS_PUBLISHING") {
-        // We don't need to rebuild - we should have done so beforehand!
-        // See `RELEASES.md`
+}
+
+fn main() {
+    if !should_run() {
         return;
     }
 
@@ -124,7 +128,7 @@ fn main() {
     // We're packing at that level rather than at the workspace level because we lose all workspace
     // layout information when publishing the crates.
     // This means all the shaders we pack must live under `re_renderer/shader` for now.
-    let manifest_path = Path::new(&std::env::var("CARGO_MANIFEST_DIR").unwrap()).to_owned();
+    let manifest_path = Path::new(&get_and_track_env_var("CARGO_MANIFEST_DIR").unwrap()).to_owned();
     let shader_dir = manifest_path.join("shader");
 
     // On windows at least, it's been shown that the paths we get out of these env-vars can
@@ -201,7 +205,7 @@ pub fn init() {
 
         let is_release = cfg!(not(debug_assertions));
         // DO NOT USE `cfg!` for this, that would give you the host's platform!
-        let targets_wasm = std::env::var("CARGO_CFG_TARGET_FAMILY").unwrap() == "wasm";
+        let targets_wasm = get_and_track_env_var("CARGO_CFG_TARGET_FAMILY").unwrap() == "wasm";
 
         // Make sure we're not referencing anything outside of the workspace!
         //

crates/re_sdk/build.rs

@@ -1,4 +1,3 @@
 fn main() {
-    re_build_tools::rebuild_if_crate_changed("re_sdk");
-    re_build_tools::export_env_vars();
+    re_build_tools::export_build_info_vars_for_crate("re_sdk");
 }

crates/re_types/build.rs

@@ -2,9 +2,7 @@
 
 use std::path::Path;
 
-use re_build_tools::{
-    is_tracked_env_var_set, iter_dir, read_versioning_hash, rerun_if_changed, write_versioning_hash,
-};
+use re_build_tools::{iter_dir, read_versioning_hash, rerun_if_changed, write_versioning_hash};
 use re_types_builder::{compute_re_types_hash, SourceLocations};
 
 // ---
@@ -31,19 +29,31 @@ macro_rules! join {
     }}
 }
 
-fn main() {
+fn should_run() -> bool {
+    #![allow(clippy::match_same_arms)]
+    use re_build_tools::Environment;
+
     if cfg!(target_os = "windows") {
-        // TODO(#2591): Codegen is temporarily disabled on Windows due to hashing issues.
-        return;
+        // TODO(#2591): Codegen is currently disabled on Windows due to hashing issues, likely because of `\r` in files
+        return false;
     }
 
-    if !is_tracked_env_var_set("IS_IN_RERUN_WORKSPACE") {
-        // Only run if we are in the rerun workspace, not on users machines.
-        return;
+    match Environment::detect() {
+        // we should have been run before publishing
+        Environment::PublishingCrates => false,
+
+        // YES! We run it to verify that the generated code is up-to-date.
+        Environment::CI => true,
+
+        Environment::DeveloperInWorkspace => true,
+
+        // We ship pre-built source files for users
+        Environment::UsedAsDependency => false,
     }
-    if is_tracked_env_var_set("RERUN_IS_PUBLISHING") {
-        // We don't need to rebuild - we should have done so beforehand!
-        // See `RELEASES.md`
+}
+
+fn main() {
+    if !should_run() {
         return;
     }
 
@@ -80,7 +90,7 @@ fn main() {
     // Detect desyncs between definitions and generated when running on CI, and
     // crash the build accordingly.
     #[allow(clippy::manual_assert)]
-    if std::env::var("CI").is_ok() {
+    if re_build_tools::is_on_ci() {
         panic!("re_types' fbs definitions and generated code are out-of-sync!");
     }