Merge pull request #345 from bcressey/cicd-hack

buildsys: add CI/CD hack, fix mtime for external files

tools/buildsys/Cargo.toml

@@ -13,6 +13,7 @@ bottlerocket-variant.workspace = true
 buildsys-config.workspace = true
 clap = { workspace = true, features = ["derive", "env"] }
 duct.workspace = true
+filetime.workspace = true
 guppy.workspace = true
 hex.workspace = true
 lazy_static.workspace = true

tools/buildsys/src/args.rs

@@ -92,6 +92,14 @@ pub(crate) struct Common {
 
     #[arg(long, env = "TWOLITER_TOOLS_DIR")]
     pub(crate) tools_dir: PathBuf,
+
+    /// cicd_hack is used to suppress builds from running after all the cargo-related metadata is
+    /// emitted. This allows cargo to create a fresh crate, and assumes that the corresponding
+    /// build artifacts are already present. It is intended for use in a CI/CD scenario where some
+    /// other process populates the build directory from a cache. Other uses may lead to unexpected
+    /// build failures that are difficult to troubleshoot.
+    #[arg(long, env = "BUILDSYS_CICD_HACK")]
+    pub(crate) cicd_hack: bool,
 }
 
 /// Build RPMs from a spec file and sources.

tools/buildsys/src/cache.rs

@@ -14,6 +14,7 @@ pub(crate) mod error;
 use error::Result;
 
 use buildsys::manifest;
+use filetime::{set_file_mtime, FileTime};
 use reqwest::header::{HeaderMap, HeaderValue, USER_AGENT};
 use sha2::{Digest, Sha512};
 use snafu::{ensure, OptionExt, ResultExt};
@@ -48,7 +49,7 @@ impl LookasideCache {
     }
 
     /// Fetch files stored out-of-tree and ensure they match the stored hash.
-    pub(crate) fn fetch(&self, files: &[manifest::ExternalFile]) -> Result<()> {
+    pub(crate) fn fetch(&self, files: &[manifest::ExternalFile], mtime: FileTime) -> Result<()> {
         for f in files {
             let url_file_name = Self::extract_file_name(&f.url)?;
             let path = &f.path.as_ref().unwrap_or(&url_file_name);
@@ -86,6 +87,7 @@ impl LookasideCache {
                 Ok(_) => {
                     fs::rename(&tmp, path)
                         .context(error::ExternalFileRenameSnafu { path: &tmp })?;
+                    set_file_mtime(path, mtime).context(error::SetMtimeSnafu { path })?;
                     continue;
                 }
                 Err(e) => {
@@ -96,6 +98,7 @@ impl LookasideCache {
                         self.fetch_file(&f.url, &tmp, hash)?;
                         fs::rename(&tmp, path)
                             .context(error::ExternalFileRenameSnafu { path: &tmp })?;
+                        set_file_mtime(path, mtime).context(error::SetMtimeSnafu { path })?;
                     } else {
                         // we failed to fetch from the lookaside cache, and we cannot fall back to
                         // upstream sources, so we should not continue, we need to return the error

tools/buildsys/src/cache/error.rs

@@ -45,6 +45,9 @@ pub(crate) enum Error {
     #[snafu(display("Failed to delete file '{}': {}", path.display(), source))]
     ExternalFileDelete { path: PathBuf, source: io::Error },
 
+    #[snafu(display("Failed to set modification time for file '{}': {}", path.display(), source))]
+    SetMtime { path: PathBuf, source: io::Error },
+
     #[snafu(display("Failed to get path segments from URL '{}'", url))]
     UrlPathSegments { url: String },
 }

tools/buildsys/src/gomod.rs

@@ -20,6 +20,7 @@ pub(crate) mod error;
 use buildsys::manifest;
 use duct::cmd;
 use error::Result;
+use filetime::{set_file_mtime, FileTime};
 use snafu::{ensure, OptionExt, ResultExt};
 use std::io::Write;
 use std::os::unix::fs::PermissionsExt;
@@ -76,6 +77,7 @@ impl GoMod {
         package_dir: &Path,
         external_file: &manifest::ExternalFile,
         sdk: &str,
+        mtime: FileTime,
     ) -> Result<()> {
         let url_file_name = extract_file_name(&external_file.url)?;
         let local_file_name = &external_file.path.as_ref().unwrap_or(&url_file_name);
@@ -144,6 +146,13 @@ impl GoMod {
 
         let res = docker_go(&args);
         fs::remove_file(&script_path).context(error::RemoveFileSnafu { path: &script_path })?;
+
+        if res.is_ok() {
+            set_file_mtime(output_path_arg, mtime).context(error::SetMtimeSnafu {
+                path: output_path_arg,
+            })?;
+        }
+
         res
     }
 }

tools/buildsys/src/gomod/error.rs

@@ -41,6 +41,12 @@ pub(crate) enum Error {
         source: std::io::Error,
     },
 
+    #[snafu(display("Failed to set modification time for file '{}': {}", path.display(), source))]
+    SetMtime {
+        path: PathBuf,
+        source: std::io::Error,
+    },
+
     #[snafu(display("Failed to write contents to '{}': {}", path.display(), source))]
     WriteFile {
         path: PathBuf,

tools/buildsys/src/main.rs

@@ -23,6 +23,7 @@ use buildsys::manifest::{BundleModule, Manifest, ManifestInfo, SupportedArch};
 use buildsys_config::EXTERNAL_KIT_METADATA;
 use cache::LookasideCache;
 use clap::Parser;
+use filetime::FileTime;
 use gomod::GoMod;
 use project::ProjectInfo;
 use snafu::{ensure, ResultExt};
@@ -47,6 +48,12 @@ mod error {
         #[snafu(display("{source}"))]
         ExternalFileFetch { source: super::cache::error::Error },
 
+        #[snafu(display("Failed to get metadata for '{}': {}", path.display(), source))]
+        FileMetadata {
+            path: PathBuf,
+            source: std::io::Error,
+        },
+
         #[snafu(display("{source}"))]
         GoMod { source: super::gomod::error::Error },
 
@@ -136,14 +143,24 @@ fn build_package(args: BuildPackageArgs) -> Result<()> {
     ensure_package_is_not_variant_sensitive(&manifest, &manifest_path)?;
 
     if let Some(files) = manifest.info().external_files() {
+        // We need the modification time for any external files or bundled modules to be no later
+        // than the manifest's modification time, to avoid triggering spurious rebuilds.
+        let metadata =
+            std::fs::metadata(manifest_path.clone()).context(error::FileMetadataSnafu {
+                path: manifest_path,
+            })?;
+        let mtime = FileTime::from_last_modification_time(&metadata);
+
         let lookaside_cache = LookasideCache::new(
             &args.common.version_full,
             args.lookaside_cache.clone(),
             args.upstream_source_fallback == "true",
         );
+
         lookaside_cache
-            .fetch(files)
+            .fetch(files, mtime)
             .context(error::ExternalFileFetchSnafu)?;
+
         for f in files {
             if f.bundle_modules.is_none() {
                 continue;
@@ -156,6 +173,7 @@ fn build_package(args: BuildPackageArgs) -> Result<()> {
                         &args.common.cargo_manifest_dir,
                         f,
                         &args.common.sdk_image,
+                        mtime,
                     )
                     .context(error::GoModSnafu)?,
                 }
@@ -190,6 +208,10 @@ fn build_package(args: BuildPackageArgs) -> Result<()> {
         println!("cargo:rerun-if-changed={}", f.display());
     }
 
+    if args.common.cicd_hack {
+        return Ok(());
+    }
+
     DockerBuild::new_package(args, &manifest)
         .context(error::BuilderInstantiationSnafu)?
         .build()
@@ -210,6 +232,10 @@ fn build_kit(args: BuildKitArgs) -> Result<()> {
     )
     .context(error::ManifestParseSnafu)?;
 
+    if args.common.cicd_hack {
+        return Ok(());
+    }
+
     DockerBuild::new_kit(args, &manifest)
         .context(error::BuilderInstantiationSnafu)?
         .build()
@@ -232,6 +258,10 @@ fn build_variant(args: BuildVariantArgs) -> Result<()> {
 
     supported_arch(manifest.info(), args.common.arch)?;
 
+    if args.common.cicd_hack {
+        return Ok(());
+    }
+
     DockerBuild::new_variant(args, &manifest)
         .context(error::BuilderInstantiationSnafu)?
         .build()
@@ -249,6 +279,10 @@ fn repack_variant(args: RepackVariantArgs) -> Result<()> {
 
     supported_arch(manifest.info(), args.common.arch)?;
 
+    if args.common.cicd_hack {
+        return Ok(());
+    }
+
     DockerBuild::repack_variant(args, &manifest)
         .context(error::BuilderInstantiationSnafu)?
         .build()