Skip to content

Commit

Permalink
buildsys: remove package variant sensitivity
Browse files Browse the repository at this point in the history
Remove the logic by which packages are differentially built based on
which variant they are for.
Requires bottlerocket-os/bottlerocket#4038
  • Loading branch information
webern committed Jun 11, 2024
1 parent 4406591 commit 8bfbe21
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 94 deletions.
17 changes: 1 addition & 16 deletions tools/buildsys/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,6 @@ impl CommonBuildArgs {
}

struct PackageBuildArgs {
/// The package might need to know what the `image_features` are going to be for the variant
/// it is going to be used in downstream. This is because certain packages will be built
/// differently based on certain image features such as cgroupsv1 vs cgroupsv2. During a
/// package build, these are determined by looking at the variant's Cargo.toml file based on
/// what was found in `BUILDSYS_VARIANT`.
image_features: HashSet<ImageFeature>,
package: String,
package_dependencies: Vec<String>,
kit_dependencies: Vec<String>,
Expand Down Expand Up @@ -170,10 +164,6 @@ impl crate::builder::PackageBuildArgs {
args.build_arg("VARIANT_FLAVOR", &self.variant_flavor);
args.build_arg("VARIANT_PLATFORM", &self.variant_platform);
args.build_arg("VARIANT_RUNTIME", &self.variant_runtime);
for image_feature in &self.image_features {
args.build_arg(format!("{}", image_feature), "1");
}

args
}
}
Expand Down Expand Up @@ -313,11 +303,7 @@ pub(crate) struct DockerBuild {

impl DockerBuild {
/// Create a new `DockerBuild` that can build a package.
pub(crate) fn new_package(
args: BuildPackageArgs,
manifest: &Manifest,
image_features: HashSet<ImageFeature>,
) -> Result<Self> {
pub(crate) fn new_package(args: BuildPackageArgs, manifest: &Manifest) -> Result<Self> {
let package = manifest.info().package_name();
let per_package_dir = format!("{}/{}", args.packages_dir.display(), package).into();
let old_package_dir = format!("{}", args.packages_dir.display()).into();
Expand Down Expand Up @@ -345,7 +331,6 @@ impl DockerBuild {
OutputCleanup::BeforeBuild,
),
target_build_args: TargetBuildArgs::Package(PackageBuildArgs {
image_features,
package: package.to_string(),
package_dependencies: manifest.package_dependencies().context(error::GraphSnafu)?,
kit_dependencies: manifest.kit_dependencies().context(error::GraphSnafu)?,
Expand Down
82 changes: 4 additions & 78 deletions tools/buildsys/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ use crate::args::{
BuildKitArgs, BuildPackageArgs, BuildVariantArgs, Buildsys, Command, RepackVariantArgs,
};
use crate::builder::DockerBuild;
use buildsys::manifest::{BundleModule, ImageFeature, Manifest, ManifestInfo, SupportedArch};
use buildsys::manifest::{BundleModule, Manifest, ManifestInfo, SupportedArch};
use buildsys_config::EXTERNAL_KIT_METADATA;
use cache::LookasideCache;
use clap::Parser;
use gomod::GoMod;
use project::ProjectInfo;
use snafu::{ensure, ResultExt};
use spec::SpecInfo;
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::process;

Expand Down Expand Up @@ -130,18 +129,8 @@ fn build_package(args: BuildPackageArgs) -> Result<()> {
let manifest = Manifest::new(&manifest_path, &args.common.cargo_metadata_path)
.context(error::ManifestParseSnafu)?;

let image_features = if std::env::var("BUILDSYS_DEPRECATED_FEATURE_VARIANT_SENSITIVITY").is_ok()
{
get_package_features_and_emit_cargo_watches_for_variant_sensitivity(
&manifest,
&args.common.root_dir,
&args.variant,
args.common.arch,
)?
} else {
ensure_package_is_not_variant_sensitive(&manifest, &manifest_path)?;
HashSet::default()
};
// Check for a deprecated key and error if it is detected.
ensure_package_is_not_variant_sensitive(&manifest, &manifest_path)?;

if let Some(files) = manifest.info().external_files() {
let lookaside_cache = LookasideCache::new(
Expand Down Expand Up @@ -198,7 +187,7 @@ fn build_package(args: BuildPackageArgs) -> Result<()> {
println!("cargo:rerun-if-changed={}", f.display());
}

DockerBuild::new_package(args, &manifest, image_features)
DockerBuild::new_package(args, &manifest)
.context(error::BuilderInstantiationSnafu)?
.build()
.context(error::BuildAttemptSnafu)
Expand Down Expand Up @@ -274,69 +263,6 @@ fn supported_arch(manifest: &ManifestInfo, arch: SupportedArch) -> Result<()> {
Ok(())
}

fn get_package_features_and_emit_cargo_watches_for_variant_sensitivity(
manifest: &Manifest,
root_dir: &Path,
variant: &str,
arch: SupportedArch,
) -> Result<HashSet<ImageFeature>> {
let package_features = manifest.info().package_features();

// Load the Variant manifest to find image features that may affect the package build.
let variant_manifest_path = root_dir.join("variants").join(variant).join("Cargo.toml");

let variant_manifest =
ManifestInfo::new(variant_manifest_path).context(error::ManifestParseSnafu)?;
supported_arch(&variant_manifest, arch)?;
let mut image_features = variant_manifest.image_features();

// For any package feature specified in the package manifest, track the corresponding
// environment variable for changes to the ambient set of image features for the current
// variant.
if let Some(package_features) = &package_features {
for package_feature in package_features {
println!(
"cargo:rerun-if-env-changed=BUILDSYS_VARIANT_IMAGE_FEATURE_{}",
package_feature
);
}
}

// Keep only the image features that the package has indicated that it tracks, if any.
if let Some(image_features) = &mut image_features {
match package_features {
Some(package_features) => image_features.retain(|k| package_features.contains(k)),
None => image_features.clear(),
}
}

// If manifest has package.metadata.build-package.variant-sensitive set, then track the
// appropriate environment variable for changes.
if let Some(sensitivity) = manifest.info().variant_sensitive() {
use buildsys::manifest::{SensitivityType::*, VariantSensitivity::*};
fn emit_variant_env(suffix: Option<&str>) {
if let Some(suffix) = suffix {
println!(
"cargo:rerun-if-env-changed=BUILDSYS_VARIANT_{}",
suffix.to_uppercase()
);
} else {
println!("cargo:rerun-if-env-changed=BUILDSYS_VARIANT");
}
}
match sensitivity {
Any(false) => (),
Any(true) => emit_variant_env(None),
Specific(Platform) => emit_variant_env(Some("platform")),
Specific(Runtime) => emit_variant_env(Some("runtime")),
Specific(Family) => emit_variant_env(Some("family")),
Specific(Flavor) => emit_variant_env(Some("flavor")),
}
}

Ok(image_features.unwrap_or_default())
}

/// Prior to the release of Kits as a build feature, packages could, and did, declare themselves
/// sensitive to various Variant features so that they could be conditionally compiled based on
/// what variant was being built. This is no longer the case, so we enforce that these keys are no
Expand Down

0 comments on commit 8bfbe21

Please sign in to comment.