buildsys: use clap for env vars

Required input in the form of environment variables was sprinkled
throughout the code. Here we aggregate the inputs into a CLI interface
using Clap, while still allowing all to specified via environment
variables.

tools/buildsys/Cargo.toml

@@ -10,6 +10,7 @@ exclude = ["README.md"]
 
 [dependencies]
 bottlerocket-variant = { version = "0.1", path = "../bottlerocket-variant" }
+clap = { version = "4", features = ["derive", "env"] }
 duct = "0.13"
 hex = "0.4"
 lazy_static = "1"

tools/buildsys/src/args.rs

@@ -0,0 +1,138 @@
+/*!
+
+These structs provide the CLI interface for buildsys which is called from Cargo.toml and accepts all
+of its input arguments from environment variables.
+
+!*/
+
+use clap::{Parser, Subcommand};
+use std::path::PathBuf;
+
+/// A tool for building Bottlerocket images and artifacts.
+#[derive(Debug, Parser)]
+pub(crate) struct Buildsys {
+    #[command(subcommand)]
+    pub(crate) command: Command,
+}
+
+#[derive(Subcommand, Debug)]
+pub(crate) enum Command {
+    BuildPackage(BuildPackageArgs),
+    BuildVariant(BuildVariantArgs),
+}
+
+/// Arguments common to all subcommands.
+#[derive(Debug, Parser)]
+pub(crate) struct Common {
+    #[arg(long, env = "BUILDSYS_ARCH")]
+    pub(crate) arch: String,
+
+    #[arg(long, env = "BUILDSYS_OUTPUT_DIR")]
+    pub(crate) output_dir: PathBuf,
+
+    #[arg(long, env = "BUILDSYS_ROOT_DIR")]
+    pub(crate) root_dir: PathBuf,
+
+    #[arg(long, env = "BUILDSYS_STATE_DIR")]
+    pub(crate) state_dir: PathBuf,
+
+    #[arg(long, env = "BUILDSYS_TIMESTAMP")]
+    pub(crate) timestamp: String,
+
+    #[arg(long, env = "CARGO_MANIFEST_DIR")]
+    pub(crate) cargo_manifest_dir: PathBuf,
+}
+
+impl Common {
+    pub(crate) fn rerun_for_envs() {
+        [
+            "BUILDSYS_ARCH",
+            "BUILDSYS_OUTPUT_DIR",
+            "BUILDSYS_ROOT_DIR",
+            "BUILDSYS_STATE_DIR",
+            "BUILDSYS_TIMESTAMP",
+            "BUILDSYS_VERSION_FULL",
+            "CARGO_MANIFEST_DIR",
+        ]
+        .iter()
+        .for_each(|&var| rerun_for_env(var));
+    }
+}
+
+/// Build RPMs from a spec file and sources.
+#[derive(Debug, Parser)]
+pub(crate) struct BuildPackageArgs {
+    #[arg(long, env = "BUILDSYS_PACKAGES_DIR")]
+    pub(crate) packages_dir: PathBuf,
+
+    #[arg(long, env = "BUILDSYS_VARIANT")]
+    pub(crate) variant: String,
+
+    #[arg(long, env = "BUILDSYS_SOURCES_DIR")]
+    pub(crate) sources_dir: PathBuf,
+
+    #[arg(long, env = "CARGO_PKG_NAME")]
+    pub(crate) cargo_package_name: String,
+
+    #[command(flatten)]
+    pub(crate) common: Common,
+}
+
+impl BuildPackageArgs {
+    pub(crate) fn rerun_for_envs() {
+        Common::rerun_for_envs();
+        [
+            "BUILDSYS_PACKAGES_DIR",
+            "BUILDSYS_SOURCES_DIR",
+            "BUILDSYS_VARIANT",
+            "CARGO_PKG_NAME",
+        ]
+        .iter()
+        .for_each(|&var| rerun_for_env(var));
+    }
+}
+
+/// Build filesystem and disk images from RPMs.
+#[derive(Debug, Parser)]
+pub(crate) struct BuildVariantArgs {
+    #[arg(long, env = "BUILDSYS_NAME")]
+    pub(crate) name: String,
+
+    #[arg(long, env = "BUILDSYS_PRETTY_NAME")]
+    pub(crate) pretty_name: String,
+
+    #[arg(long, env = "BUILDSYS_VARIANT")]
+    pub(crate) variant: String,
+
+    #[arg(long, env = "BUILDSYS_VERSION_BUILD")]
+    pub(crate) version_build: String,
+
+    #[arg(long, env = "BUILDSYS_VERSION_FULL")]
+    pub(crate) version_full: String,
+
+    #[arg(long, env = "BUILDSYS_VERSION_IMAGE")]
+    pub(crate) version_image: String,
+
+    #[command(flatten)]
+    pub(crate) common: Common,
+}
+
+impl BuildVariantArgs {
+    pub(crate) fn rerun_for_envs() {
+        Common::rerun_for_envs();
+        [
+            "BUILDSYS_NAME",
+            "BUILDSYS_PRETTY_NAME",
+            "BUILDSYS_VARIANT",
+            "BUILDSYS_VERSION_BUILD",
+            "BUILDSYS_VERSION_FULL",
+            "BUILDSYS_VERSION_IMAGE",
+        ]
+        .iter()
+        .for_each(|&var| rerun_for_env(var));
+    }
+}
+
+fn rerun_for_env(var: &str) {
+    println!("cargo:rerun-if-env-changed={}", var);
+}

tools/buildsys/src/main.rs

@@ -8,22 +8,23 @@ specified as a command line argument.
 The implementation is closely tied to the top-level Dockerfile.
 
 */
+mod args;
 mod builder;
 mod cache;
 mod constants;
 mod gomod;
 mod project;
 mod spec;
 
+use crate::args::{BuildPackageArgs, BuildVariantArgs, Buildsys, Command};
 use builder::{PackageBuilder, VariantBuilder};
 use buildsys::manifest::{BundleModule, ManifestInfo, SupportedArch};
 use cache::LookasideCache;
+use clap::Parser;
 use gomod::GoMod;
 use project::ProjectInfo;
-use serde::Deserialize;
 use snafu::{ensure, ResultExt};
 use spec::SpecInfo;
-use std::env;
 use std::path::PathBuf;
 use std::process;
 
@@ -83,63 +84,47 @@ mod error {
 
 type Result<T> = std::result::Result<T, error::Error>;
 
-#[derive(Debug, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-enum Command {
-    BuildPackage,
-    BuildVariant,
-}
-
-fn usage() -> ! {
-    eprintln!(
-        "\
-USAGE:
-    buildsys <SUBCOMMAND>
-
-SUBCOMMANDS:
-    build-package           Build RPMs from a spec file and sources.
-    build-variant           Build filesystem and disk images from RPMs."
-    );
-    process::exit(1)
-}
-
 // Returning a Result from main makes it print a Debug representation of the error, but with Snafu
 // we have nice Display representations of the error, so we wrap "main" (run) and print any error.
 // https://github.com/shepmaster/snafu/issues/110
 fn main() {
-    if let Err(e) = run() {
+    let args = Buildsys::parse();
+    if let Err(e) = run(args) {
         eprintln!("{}", e);
         process::exit(1);
     }
 }
 
-fn run() -> Result<()> {
-    // Not actually redundant for a diverging function.
-    #[allow(clippy::redundant_closure)]
-    let command_str = std::env::args().nth(1).unwrap_or_else(|| usage());
-    let command = serde_plain::from_str::<Command>(&command_str).unwrap_or_else(|_| usage());
-    match command {
-        Command::BuildPackage => build_package()?,
-        Command::BuildVariant => build_variant()?,
+fn run(args: Buildsys) -> Result<()> {
+    match args.command {
+        Command::BuildPackage(args) => {
+            BuildPackageArgs::rerun_for_envs();
+            build_package(args)
+        }
+        Command::BuildVariant(args) => {
+            BuildVariantArgs::rerun_for_envs();
+            build_variant(args)
+        }
     }
-    Ok(())
 }
 
-fn build_package() -> Result<()> {
+fn build_package(args: BuildPackageArgs) -> Result<()> {
     let manifest_file = "Cargo.toml";
     println!("cargo:rerun-if-changed={}", manifest_file);
 
-    let root_dir: PathBuf = getenv("BUILDSYS_ROOT_DIR")?.into();
-    let variant = getenv("BUILDSYS_VARIANT")?;
-    let variant_manifest_path = root_dir.join("variants").join(variant).join(manifest_file);
+    let variant_manifest_path = args
+        .common
+        .root_dir
+        .join("variants")
+        .join(args.variant)
+        .join(manifest_file);
     let variant_manifest =
         ManifestInfo::new(variant_manifest_path).context(error::ManifestParseSnafu)?;
-    supported_arch(&variant_manifest)?;
+    supported_arch(&variant_manifest, &args.common.arch)?;
     let mut image_features = variant_manifest.image_features();
 
-    let manifest_dir: PathBuf = getenv("CARGO_MANIFEST_DIR")?.into();
-    let manifest =
-        ManifestInfo::new(manifest_dir.join(manifest_file)).context(error::ManifestParseSnafu)?;
+    let manifest = ManifestInfo::new(args.common.cargo_manifest_dir.join(manifest_file))
+        .context(error::ManifestParseSnafu)?;
     let package_features = manifest.package_features();
 
     // For any package feature specified in the package manifest, track the corresponding
@@ -196,19 +181,19 @@ fn build_package() -> Result<()> {
             for b in f.bundle_modules.as_ref().unwrap() {
                 match b {
                     BundleModule::Go => {
-                        GoMod::vendor(&root_dir, &manifest_dir, f).context(error::GoModSnafu)?
+                        GoMod::vendor(&args.common.root_dir, &args.common.cargo_manifest_dir, f)
+                            .context(error::GoModSnafu)?
                     }
                 }
             }
         }
     }
 
     if let Some(groups) = manifest.source_groups() {
-        let var = "BUILDSYS_SOURCES_DIR";
-        let root: PathBuf = getenv(var)?.into();
-        println!("cargo:rerun-if-env-changed={}", var);
-
-        let dirs = groups.iter().map(|d| root.join(d)).collect::<Vec<_>>();
+        let dirs = groups
+            .iter()
+            .map(|d| args.sources_dir.join(d))
+            .collect::<Vec<_>>();
         let info = ProjectInfo::crawl(&dirs).context(error::ProjectCrawlSnafu)?;
         for f in info.files {
             println!("cargo:rerun-if-changed={}", f.display());
@@ -220,7 +205,7 @@ fn build_package() -> Result<()> {
     let package = if let Some(name_override) = manifest.package_name() {
         name_override.clone()
     } else {
-        getenv("CARGO_PKG_NAME")?
+        args.cargo_package_name.clone()
     };
     let spec = format!("{}.spec", package);
     println!("cargo:rerun-if-changed={}", spec);
@@ -240,15 +225,14 @@ fn build_package() -> Result<()> {
     Ok(())
 }
 
-fn build_variant() -> Result<()> {
-    let manifest_dir: PathBuf = getenv("CARGO_MANIFEST_DIR")?.into();
+fn build_variant(args: BuildVariantArgs) -> Result<()> {
     let manifest_file = "Cargo.toml";
     println!("cargo:rerun-if-changed={}", manifest_file);
 
-    let manifest =
-        ManifestInfo::new(manifest_dir.join(manifest_file)).context(error::ManifestParseSnafu)?;
+    let manifest = ManifestInfo::new(args.common.cargo_manifest_dir.join(manifest_file))
+        .context(error::ManifestParseSnafu)?;
 
-    supported_arch(&manifest)?;
+    supported_arch(&manifest, &args.common.arch)?;
 
     if let Some(packages) = manifest.included_packages() {
         let image_format = manifest.image_format();
@@ -271,16 +255,15 @@ fn build_variant() -> Result<()> {
 }
 
 /// Ensure that the current arch is supported by the current variant
-fn supported_arch(manifest: &ManifestInfo) -> Result<()> {
+fn supported_arch(manifest: &ManifestInfo, arch: &str) -> Result<()> {
     if let Some(supported_arches) = manifest.supported_arches() {
-        let arch = getenv("BUILDSYS_ARCH")?;
         let current_arch: SupportedArch =
-            serde_plain::from_str(&arch).context(error::UnknownArchSnafu { arch: &arch })?;
+            serde_plain::from_str(arch).context(error::UnknownArchSnafu { arch })?;
 
         ensure!(
             supported_arches.contains(&current_arch),
             error::UnsupportedArchSnafu {
-                arch: &arch,
+                arch,
                 supported_arches: supported_arches
                     .iter()
                     .map(|a| a.to_string())
@@ -290,8 +273,3 @@ fn supported_arch(manifest: &ManifestInfo) -> Result<()> {
     }
     Ok(())
 }
-
-/// Retrieve a variable that we expect to be set in the environment.
-fn getenv(var: &str) -> Result<String> {
-    env::var(var).context(error::EnvironmentSnafu { var })
-}