diff --git a/Cargo.lock b/Cargo.lock index 3b0788d72..7a7c0756a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -760,6 +760,7 @@ dependencies = [ "buildsys-config", "clap", "duct", + "filetime", "guppy", "hex", "lazy_static", diff --git a/tools/buildsys/Cargo.toml b/tools/buildsys/Cargo.toml index 1c64ecb52..665ed5510 100644 --- a/tools/buildsys/Cargo.toml +++ b/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 diff --git a/tools/buildsys/src/args.rs b/tools/buildsys/src/args.rs index 6fbcb51e7..87f83e822 100644 --- a/tools/buildsys/src/args.rs +++ b/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. diff --git a/tools/buildsys/src/cache.rs b/tools/buildsys/src/cache.rs index 925d215ad..ee2c19b28 100644 --- a/tools/buildsys/src/cache.rs +++ b/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 diff --git a/tools/buildsys/src/cache/error.rs b/tools/buildsys/src/cache/error.rs index 86cef8a22..3c5f7885b 100644 --- a/tools/buildsys/src/cache/error.rs +++ b/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 }, } diff --git a/tools/buildsys/src/gomod.rs b/tools/buildsys/src/gomod.rs index 277ab1ae6..402628f19 100644 --- a/tools/buildsys/src/gomod.rs +++ b/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 } } diff --git a/tools/buildsys/src/gomod/error.rs b/tools/buildsys/src/gomod/error.rs index 64d736d31..1189b6c67 100644 --- a/tools/buildsys/src/gomod/error.rs +++ b/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, diff --git a/tools/buildsys/src/main.rs b/tools/buildsys/src/main.rs index bbdad7212..36ee698d1 100644 --- a/tools/buildsys/src/main.rs +++ b/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()