Merge pull request #1329 from tjkirch/model-link

models: link current variant more carefully

sources/api/storewolf/Cargo.toml

@@ -23,8 +23,11 @@ toml = "0.5"
 [build-dependencies]
 cargo-readme = "3.1"
 merge-toml = { path = "merge-toml" }
-# model dep because we read default settings from the model directory; we also
-# reflect it with cargo:rerun-if-changed statements in build.rs.
+# We have a models build-dep because we read default settings from the models
+# directory and need its build.rs to run first; we also reflect the dependency
+# with cargo:rerun-if-changed statements in our build.rs.  The models build.rs
+# runs twice, once for the above dependency and once for this build-dependency;
+# there's a check in models build.rs to skip running the second time.
 models = { path = "../../models" }
 snafu = "0.6"
 toml = "0.5"

sources/models/Cargo.toml

@@ -24,6 +24,7 @@ url = "2.1"
 
 [build-dependencies]
 cargo-readme = "3.1"
+rand = "0.8"
 
 [lib]
 # We're picking the current *model* with build.rs, so users shouldn't think

sources/models/build.rs

@@ -4,62 +4,65 @@
 //
 // See README.md to understand the symlink setup.
 
+use rand::{distributions::Alphanumeric, thread_rng, Rng};
 use std::env;
 use std::fs::{self, File};
 use std::io::{self, Write};
 use std::os::unix::fs::symlink;
 use std::path::{Path, PathBuf};
 use std::process;
 
-fn symlink_force<P1, P2>(target: P1, link: P2) -> io::Result<()>
-where
-    P1: AsRef<Path>,
-    P2: AsRef<Path>,
-{
-    // Remove link if it already exists
-    if let Err(e) = fs::remove_file(&link) {
-        if e.kind() != io::ErrorKind::NotFound {
-            return Err(e);
-        }
+const VARIANT_LINK: &str = "src/variant/current";
+const MOD_LINK: &str = "src/variant/mod.rs";
+const VARIANT_ENV: &str = "VARIANT";
+
+fn main() {
+    // Tell cargo when we have to rerun, regardless of early-exit below.
+    println!("cargo:rerun-if-env-changed={}", VARIANT_ENV);
+    println!("cargo:rerun-if-changed={}", VARIANT_LINK);
+    println!("cargo:rerun-if-changed={}", MOD_LINK);
+
+    // This build.rs runs once as a build-dependency of storewolf, and again as a (regular)
+    // dependency of storewolf.  There's no reason to do this work twice.
+    if env::var("CARGO_CFG_TARGET_VENDOR").unwrap_or_else(|_| String::new()) == "bottlerocket" {
+        println!("cargo:warning=Already ran model build.rs for host, skipping for target");
+        process::exit(0);
     }
-    // Link to requested target
-    symlink(&target, &link)
+
+    generate_readme();
+    link_current_variant();
 }
 
 fn link_current_variant() {
     // The VARIANT variable is originally BUILDSYS_VARIANT, set in the top-level Makefile.toml,
     // and is passed through as VARIANT by the top-level Dockerfile.  It represents which OS
     // variant we're building, and therefore which API model to use.
-    let var = "VARIANT";
-    println!("cargo:rerun-if-env-changed={}", var);
-    let variant = env::var(var).unwrap_or_else(|_| {
-        eprintln!("For local builds, you must set the {} environment variable so we know which API model to build against.  Valid values are the directories in variants/, for example \"aws-k8s-1.17\".", var);
+    let variant = env::var(VARIANT_ENV).unwrap_or_else(|_| {
+        eprintln!("For local builds, you must set the {} environment variable so we know which API model to build against.  Valid values are the directories in variants/, for example \"aws-k8s-1.17\".", VARIANT_ENV);
         process::exit(1);
     });
 
     // Point to the source for the requested variant
-    let variant_link = "src/variant/current";
     let variant_target = format!("../{}", variant);
 
     // Make sure requested variant exists
     let variant_path = format!("src/{}", variant);
     if !Path::new(&variant_path).exists() {
-        eprintln!("The environment variable {} should refer to a directory under sources/models/src with an API model, but it's set to '{}' which doesn't exist", var, variant);
+        eprintln!("The environment variable {} should refer to a directory under sources/models/src with an API model, but it's set to '{}' which doesn't exist", VARIANT_ENV, variant);
         process::exit(1);
     }
 
     // Create the symlink for the following `cargo build` to use for its source code
-    symlink_force(&variant_target, variant_link).unwrap_or_else(|e| {
-        eprintln!("Failed to create symlink at '{}' pointing to '{}' - we need this to support different API models for different variants.  Error: {}", variant_link, variant_target, e);
+    symlink_safe(&variant_target, VARIANT_LINK).unwrap_or_else(|e| {
+        eprintln!("Failed to create symlink at '{}' pointing to '{}' - we need this to support different API models for different variants.  Error: {}", VARIANT_LINK, variant_target, e);
         process::exit(1);
     });
 
     // Also create the link for mod.rs so Rust can import source from the "current" link
     // created above.
-    let mod_link = "src/variant/mod.rs";
     let mod_target = "../variant_mod.rs";
-    symlink_force(&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_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);
         process::exit(1);
     });
 }
@@ -90,7 +93,32 @@ fn generate_readme() {
     readme.write_all(content.as_bytes()).unwrap();
 }
 
-fn main() {
-    generate_readme();
-    link_current_variant();
+// Creates the requested symlink through an atomic swap, so it doesn't matter if the link path
+// already exists or not; like --force but fewer worries about reentrancy and retries.
+fn symlink_safe<P1, P2>(target: P1, link: P2) -> io::Result<()>
+where
+    P1: AsRef<Path>,
+    P2: AsRef<Path>,
+{
+    // Create the link at a temporary path.
+    let temp_link = link.as_ref().with_file_name(format!(".{}", rando()));
+    symlink(&target, &temp_link)?;
+
+    // Swap the temporary link into the real location
+    if let Err(e) = fs::rename(&temp_link, &link) {
+        // If we couldn't, for whatever reason, clean up the temporary path and return the error.
+        let _ = fs::remove_file(&temp_link);
+        return Err(e);
+    }
+
+    Ok(())
+}
+
+// Generates a random ID, affectionately known as a 'rando'.
+fn rando() -> String {
+    thread_rng()
+        .sample_iter(&Alphanumeric)
+        .take(16)
+        .map(char::from)
+        .collect()
 }