From 65bca262c61a9c451826cbcb861e82fb02932be2 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Mon, 8 Nov 2021 10:30:30 -0800 Subject: [PATCH] model: set mtime of links to epoch to prevent spurious rebuilds We want to rebuild the model package when changing variants, but before this change, we rebuild every time. This happens because cargo detects that the timestamps of the variant-sensitive links we create (and that we tell it to watch) don't match its output file. We don't control creation of the output file, so they'll never match. Each rebuild changes the links again, causing another rebuild. This change sets the mtime of the links to epoch, effectively telling cargo that they haven't changed. We still rebuild if the variant changes, or if someone changes the links behind our back, which would result in a different mtime. --- sources/Cargo.lock | 1 + sources/models/Cargo.toml | 1 + sources/models/build.rs | 43 +++++++++++++++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/sources/Cargo.lock b/sources/Cargo.lock index 5714ffc5003..e54d79ba4b0 100644 --- a/sources/Cargo.lock +++ b/sources/Cargo.lock @@ -1904,6 +1904,7 @@ dependencies = [ "base64", "bottlerocket-release", "cargo-readme", + "filetime", "lazy_static", "model-derive", "rand", diff --git a/sources/models/Cargo.toml b/sources/models/Cargo.toml index 66ce1e6657b..67ab098413a 100644 --- a/sources/models/Cargo.toml +++ b/sources/models/Cargo.toml @@ -25,6 +25,7 @@ url = "2.1" [build-dependencies] cargo-readme = "3.1" +filetime = "0.2" rand = "0.8" [lib] diff --git a/sources/models/build.rs b/sources/models/build.rs index 295b9c749fe..15c5b42763d 100644 --- a/sources/models/build.rs +++ b/sources/models/build.rs @@ -4,6 +4,7 @@ // // See README.md to understand the symlink setup. +use filetime::{set_symlink_file_times, FileTime}; use rand::{distributions::Alphanumeric, thread_rng, Rng}; use std::env; use std::fs::{self, File}; @@ -12,9 +13,21 @@ use std::os::unix::fs::symlink; use std::path::{Path, PathBuf}; use std::process; +/// The name of the environment variable that tells us the current variant; we need to rebuild if +/// this changes. +const VARIANT_ENV: &str = "VARIANT"; + +/// We create a link from 'current' to the variant selected by the environment variable above. const VARIANT_LINK: &str = "src/variant/current"; + +/// We create a link for the 'variant' module's mod.rs; this can't be checked into the repo because +/// the `src/variant` directory is a cache mount created by Docker before building a package. +/// This isn't variant-specific, so we can have a fixed link target. The file has top-level +/// definitions that apply to all models, and defines a 'current' submodule (that Rust will be able +/// to find through the 'current' link mentioned above) and re-exports everything in 'current' so +/// that consumers of the model don't have to care what the current variant is. const MOD_LINK: &str = "src/variant/mod.rs"; -const VARIANT_ENV: &str = "VARIANT"; +const MOD_LINK_TARGET: &str = "../variant_mod.rs"; fn main() { // Tell cargo when we have to rerun; we always want variant links to be correct, especially @@ -54,11 +67,33 @@ fn link_current_variant() { // Also create the link for mod.rs so Rust can import source from the "current" link // created above. - let mod_target = "../variant_mod.rs"; - symlink_safe(&mod_target, MOD_LINK).unwrap_or_else(|e| { - eprintln!("Failed to create symlink at '{}' pointing to '{}' - we need this to build a Rust module structure through the `current` link. Error: {}", MOD_LINK, mod_target, e); + symlink_safe(MOD_LINK_TARGET, MOD_LINK).unwrap_or_else(|e| { + eprintln!("Failed to create symlink at '{}' pointing to '{}' - we need this to build a Rust module structure through the `current` link. Error: {}", MOD_LINK, MOD_LINK_TARGET, e); process::exit(1); }); + + // Set the mtime of the links to a fixed time, the epoch. This is because cargo decides + // whether to rerun build.rs based on the "rerun-if-changed" statements printed above and the + // mtime of the files they reference. If the mtime of the file doesn't match the mtime of the + // "output" file in the build directory (which contains the output of the rerun-if prints) then + // it rebuilds. Those times won't match because we don't control when they happen, meaning + // we'd rebuild every time. Setting to a consistent time means we only rebuild when the other + // rerun-if statements apply, the important one being the variant changing. + // + // Note that we still use rerun-if-changed for these links in case someone changes them outside + // of this build.rs. If they really want to get around our system, they'd also need to set the + // mtime to epoch, and then hopefully they know what they're doing. + for link in &[VARIANT_LINK, MOD_LINK] { + // Do our best, but if we fail, rebuilding isn't the end of the world. + // Note: set_symlink_file_times is the only method that operates on the symlink rather than + // its target, and it also updates atime, which we don't care about but isn't harmful. + if let Err(e) = set_symlink_file_times(link, FileTime::zero(), FileTime::zero()) { + eprintln!( + "Warning: unable to set mtime on {}; crate may rebuild unnecessarily: {}", + link, e + ); + } + } } fn generate_readme() {