Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Less automatic build.rs shenanigans #3814

Merged
merged 12 commits into from
Oct 11, 2023
10 changes: 8 additions & 2 deletions crates/re_build_info/src/build_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ pub struct BuildInfo {
/// ISO 8601 / RFC 3339 build time.
///
/// Example: `"2023-02-23T19:33:26Z"`
///
/// Empty if unknown.
pub datetime: &'static str,
}

Expand Down Expand Up @@ -83,7 +85,9 @@ impl std::fmt::Display for BuildInfo {
write!(f, "]")?;
}

write!(f, " {target_triple}")?;
if !target_triple.is_empty() {
write!(f, " {target_triple}")?;
}

if !git_branch.is_empty() {
write!(f, " {git_branch}")?;
Expand All @@ -93,7 +97,9 @@ impl std::fmt::Display for BuildInfo {
write!(f, " {git_hash}")?;
}

write!(f, ", built {datetime}")?;
if !datetime.is_empty() {
write!(f, ", built {datetime}")?;
}
Comment on lines +100 to +102
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be more helpful to write something like built <unknown>?

Copy link
Member Author

@emilk emilk Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it? Maybe "built at an unknown time", but then we could also add "and at an unknown place"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the output you get from rerun --version btw


Ok(())
}
Expand Down
65 changes: 65 additions & 0 deletions crates/re_build_tools/src/git.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//! # Situations to consider regarding git
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is old code that has moved to its own file

//!
//! ## 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.

use std::path::PathBuf;

use crate::{rerun_if_changed, run_command};

pub fn rebuild_if_branch_or_commit_changes() {
if let Ok(head_path) = git_path("HEAD") {
rerun_if_changed(&head_path); // Track changes to branch
if let Ok(head) = std::fs::read_to_string(&head_path) {
if let Some(git_file) = head.strip_prefix("ref: ") {
if let Ok(path) = git_path(git_file) {
if path.exists() {
rerun_if_changed(path); // Track changes to commit hash
} else {
// Weird that it doesn't exist. Maybe we will miss a git hash change,
// but that is better that tracking a non-existing files (which leads to constant rebuilds).
// See https://github.com/rerun-io/rerun/issues/2380 for more
}
}
}
}
}
}

pub fn commit_hash() -> anyhow::Result<String> {
let git_hash = run_command("git", &["rev-parse", "HEAD"])?;
if git_hash.is_empty() {
anyhow::bail!("empty commit hash");
}
Ok(git_hash)
}

pub fn branch() -> anyhow::Result<String> {
run_command("git", &["symbolic-ref", "--short", "HEAD"])
}

/// From <https://git-scm.com/docs/git-rev-parse>:
///
/// Resolve `$GIT_DIR/<path>` and takes other path relocation variables such as `$GIT_OBJECT_DIRECTORY`, `$GIT_INDEX_FILE…​` into account.
/// For example, if `$GIT_OBJECT_DIRECTORY` is set to /foo/bar then `git rev-parse --git-path objects/abc` returns `/foo/bar/abc`.
fn git_path(path: &str) -> anyhow::Result<PathBuf> {
let path = run_command("git", &["rev-parse", "--git-path", path])?;
Ok(path.into())
}
184 changes: 95 additions & 89 deletions crates/re_build_tools/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#![allow(clippy::unwrap_used)]
#![warn(missing_docs)]

//! This crate is to be used from `build.rs` build scripts.

use anyhow::Context as _;

use std::process::Command;
use std::sync::atomic::{AtomicBool, Ordering};
use std::{path::PathBuf, process::Command};

mod git;
mod hashing;
mod rebuild_detector;

Expand All @@ -21,6 +23,24 @@ pub use self::rebuild_detector::{
rerun_if_changed_glob, rerun_if_changed_or_doesnt_exist, write_file_if_necessary,
};

// ------------------

/// Should we export the build datetime for developers in the workspace?
///
/// It will be visible in analytics, in the viewer's about-menu, and with `rerun --version`.
///
/// To do so accurately may incur unnecessary recompiles, so only turn this on if you really need it.
const EXPORT_BUILD_TIME_FOR_DEVELOPERS: bool = false;

/// Should we export the current git hash/branch for developers in the workspace?
///
/// It will be visible in analytics, in the viewer's about-menu, and with `rerun --version`.
///
/// To do so accurately may incur unnecessary recompiles, so only turn this on if you really need it.
const EXPORT_GIT_FOR_DEVELOPERS: bool = false;
Comment on lines +33 to +40
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided not to make these into environment variables right now. We can very easily do so now if we change our minds.


// ------------------

/// Atomic bool indicating whether or not to print the cargo build instructions
pub(crate) static OUTPUT_CARGO_BUILD_INSTRUCTIONS: AtomicBool = AtomicBool::new(true);

Expand All @@ -34,6 +54,8 @@ pub(crate) fn should_output_cargo_build_instructions() -> bool {
OUTPUT_CARGO_BUILD_INSTRUCTIONS.load(Ordering::Relaxed)
}

// ------------------

/// Where is this `build.rs` build script running?
pub enum Environment {
/// We are running `cargo publish` (via `scripts/ci/crates.py`); _probably_ on CI.
Expand Down Expand Up @@ -87,79 +109,84 @@ pub fn is_on_ci() -> bool {
///
/// 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();
}
let environment = Environment::detect();

/// # 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());
set_env("RE_BUILD_GIT_BRANCH", &git_branch().unwrap_or_default());

// rust version
let (rustc, llvm) = rust_version().unwrap_or_default();
set_env("RE_BUILD_RUSTC_VERSION", &rustc);
set_env("RE_BUILD_LLVM_VERSION", &llvm);

// 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 is_on_ci() {
// e.g. building wheels on CI.
set_env("RE_BUILD_IS_IN_RERUN_WORKSPACE", "no");
let export_datetime = match environment {
Environment::PublishingCrates | Environment::CI => true,

Environment::DeveloperInWorkspace => EXPORT_BUILD_TIME_FOR_DEVELOPERS,

// Datetime won't always be accurate unless we rebuild as soon as a dependency changes,
// and we don't want to add that burden to our users.
Environment::UsedAsDependency => false,
};

let export_git_info = match environment {
Environment::PublishingCrates | Environment::CI => true,

Environment::DeveloperInWorkspace => EXPORT_GIT_FOR_DEVELOPERS,

// We shouldn't show the users git hash/branch in the rerun viewer.
Environment::UsedAsDependency => false,
};

if export_datetime {
set_env("RE_BUILD_DATETIME", &date_time());

// The only way to make sure the build datetime is up-to-date is to run
// `build.rs` on every build, and there is really no good way of doing
// so except to manually check if any files have changed:
rebuild_if_crate_changed(crate_name);
} else {
set_env("RE_BUILD_DATETIME", "");
}

if export_git_info {
set_env("RE_BUILD_GIT_HASH", &git::commit_hash().unwrap_or_default());
set_env("RE_BUILD_GIT_BRANCH", &git::branch().unwrap_or_default());

// Make sure the above are up-to-date
git::rebuild_if_branch_or_commit_changes();
} else {
set_env(
"RE_BUILD_IS_IN_RERUN_WORKSPACE",
&std::env::var("IS_IN_RERUN_WORKSPACE").unwrap_or_default(),
);
set_env("RE_BUILD_GIT_HASH", "");
set_env("RE_BUILD_GIT_BRANCH", "");
}

// Stuff that doesn't change, so doesn't need rebuilding:
{
// target triple
set_env("RE_BUILD_TARGET_TRIPLE", &std::env::var("TARGET").unwrap());

// rust version
let (rustc, llvm) = rust_llvm_versions().unwrap_or_default();
set_env("RE_BUILD_RUSTC_VERSION", &rustc);
set_env("RE_BUILD_LLVM_VERSION", &llvm);

// 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 is_on_ci() {
// e.g. building wheels on CI.
set_env("RE_BUILD_IS_IN_RERUN_WORKSPACE", "no");
} else {
set_env(
"RE_BUILD_IS_IN_RERUN_WORKSPACE",
&std::env::var("IS_IN_RERUN_WORKSPACE").unwrap_or_default(),
);
}
}
}

/// ISO 8601 / RFC 3339 build time.
///
/// Example: `"2023-02-23T19:33:26Z"`
fn date_time() -> String {
let time_format =
time::format_description::parse("[year]-[month]-[day]T[hour]:[minute]:[second]Z").unwrap();
let date_time = time::OffsetDateTime::now_utc()

time::OffsetDateTime::now_utc()
.format(&time_format)
.unwrap();
set_env("RE_BUILD_DATETIME", &date_time);

// Make sure we re-run the build script if the branch or commit changes:
if let Ok(head_path) = git_path("HEAD") {
rerun_if_changed(&head_path); // Track changes to branch
if let Ok(head) = std::fs::read_to_string(&head_path) {
if let Some(git_file) = head.strip_prefix("ref: ") {
if let Ok(path) = git_path(git_file) {
if path.exists() {
rerun_if_changed(path); // Track changes to commit hash
} else {
// Weird that it doesn't exist. Maybe we will miss a git hash change,
// but that is better that tracking a non-existing files (which leads to constant rebuilds).
// See https://github.com/rerun-io/rerun/issues/2380 for more
}
}
}
}
}
.unwrap()
}

fn set_env(name: &str, value: &str) {
Expand All @@ -184,32 +211,11 @@ fn run_command(cmd: &str, args: &[&str]) -> anyhow::Result<String> {
Ok(String::from_utf8(output.stdout)?.trim().to_owned())
}

fn git_hash() -> anyhow::Result<String> {
let git_hash = run_command("git", &["rev-parse", "HEAD"])?;
if git_hash.is_empty() {
anyhow::bail!("empty commit hash");
}
Ok(git_hash)
}

fn git_branch() -> anyhow::Result<String> {
run_command("git", &["symbolic-ref", "--short", "HEAD"])
}

/// From <https://git-scm.com/docs/git-rev-parse>:
///
/// Resolve `$GIT_DIR/<path>` and takes other path relocation variables such as `$GIT_OBJECT_DIRECTORY`, `$GIT_INDEX_FILE…​` into account.
/// For example, if `$GIT_OBJECT_DIRECTORY` is set to /foo/bar then `git rev-parse --git-path objects/abc` returns `/foo/bar/abc`.
fn git_path(path: &str) -> anyhow::Result<PathBuf> {
let path = run_command("git", &["rev-parse", "--git-path", path])?;
Ok(path.into())
}

/// Returns `(rustc, LLVM)` versions.
///
/// Defaults to `"unknown"` if, for whatever reason, the output from `rustc -vV` did not contain
/// version information and/or the output format underwent breaking changes.
fn rust_version() -> anyhow::Result<(String, String)> {
fn rust_llvm_versions() -> anyhow::Result<(String, String)> {
let cmd = std::env::var("RUSTC").unwrap_or("rustc".into());
let args = &["-vV"];

Expand Down
4 changes: 4 additions & 0 deletions crates/re_build_tools/src/rebuild_detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ fn should_run() -> bool {
// Dependencies shouldn't change on CI, but who knows 🤷‍♂️
Environment::CI => true,

// Yes - this is what we want tracking for.
Environment::DeveloperInWorkspace => true,

// Definitely not
Expand All @@ -34,6 +35,9 @@ fn should_run() -> bool {
///
/// This will work even if the package depends on crates that are outside of the workspace,
/// included with `path = …`
///
/// However, this is a complex beast, and may have bugs in it.
/// Maybe it is even causing spurious re-compiles (<https://github.com/rerun-io/rerun/issues/3266>).
pub fn rebuild_if_crate_changed(pkg_name: &str) {
if !should_run() {
return;
Expand Down
8 changes: 8 additions & 0 deletions crates/re_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ 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 = []
emilk marked this conversation as resolved.
Show resolved Hide resolved


[dependencies]
# Rerun
re_error.workspace = true
Expand Down
Loading
Loading