diff --git a/crates/spk-build/src/build/binary.rs b/crates/spk-build/src/build/binary.rs index 50d0a9eb0..2e2c7e03d 100644 --- a/crates/spk-build/src/build/binary.rs +++ b/crates/spk-build/src/build/binary.rs @@ -23,7 +23,6 @@ use spk_schema::foundation::format::FormatIdent; use spk_schema::foundation::ident_build::Build; use spk_schema::foundation::ident_component::Component; use spk_schema::foundation::option_map::OptionMap; -use spk_schema::foundation::version::VERSION_SEP; use spk_schema::ident::{PkgRequest, PreReleasePolicy, RangeIdent, RequestedBy, VersionIdent}; use spk_schema::variant::Override; use spk_schema::{ @@ -638,7 +637,7 @@ where let mut cmd = cmd.into_std(); cmd.envs(self.environment.drain()); cmd.envs(options.as_ref().to_environment()); - cmd.envs(get_package_build_env(package)); + cmd.envs(package.get_build_env()); cmd.env("PREFIX", &self.prefix); // force the base environment to be setup using bash, so that the // spfs startup and build environment are predictable and consistent @@ -721,43 +720,6 @@ where } } -/// Return the environment variables to be set for a build of the given package spec. -pub fn get_package_build_env

(spec: &P) -> HashMap -where - P: Package, -{ - let mut env = HashMap::with_capacity(8); - env.insert("SPK_PKG".to_string(), spec.ident().to_string()); - env.insert("SPK_PKG_NAME".to_string(), spec.name().to_string()); - env.insert("SPK_PKG_VERSION".to_string(), spec.version().to_string()); - env.insert( - "SPK_PKG_BUILD".to_string(), - spec.ident().build().to_string(), - ); - env.insert( - "SPK_PKG_VERSION_MAJOR".to_string(), - spec.version().major().to_string(), - ); - env.insert( - "SPK_PKG_VERSION_MINOR".to_string(), - spec.version().minor().to_string(), - ); - env.insert( - "SPK_PKG_VERSION_PATCH".to_string(), - spec.version().patch().to_string(), - ); - env.insert( - "SPK_PKG_VERSION_BASE".to_string(), - spec.version() - .parts - .iter() - .map(u32::to_string) - .collect::>() - .join(VERSION_SEP), - ); - env -} - /// Commit changes discovered in the runtime as a package. /// /// Only the changes also present in `filter` will be committed. It is diff --git a/crates/spk-build/src/build/mod.rs b/crates/spk-build/src/build/mod.rs index be29e897c..53979ac55 100644 --- a/crates/spk-build/src/build/mod.rs +++ b/crates/spk-build/src/build/mod.rs @@ -11,7 +11,6 @@ pub use binary::{ build_spec_path, commit_component_layers, component_marker_path, - get_package_build_env, source_package_path, BinaryPackageBuilder, BuildError, diff --git a/crates/spk-build/src/build/sources.rs b/crates/spk-build/src/build/sources.rs index 032a5e39e..692186d35 100644 --- a/crates/spk-build/src/build/sources.rs +++ b/crates/spk-build/src/build/sources.rs @@ -136,7 +136,7 @@ where std::fs::create_dir_all(source_dir) .map_err(|err| Error::DirectoryCreateError(source_dir.to_owned(), err))?; - let env = super::binary::get_package_build_env(spec); + let env = spec.get_build_env(); for source in spec.sources().iter() { let target_dir = match source.subdir() { Some(subdir) => subdir.to_path(source_dir), diff --git a/crates/spk-build/src/lib.rs b/crates/spk-build/src/lib.rs index 271333a68..0b7549d1b 100644 --- a/crates/spk-build/src/lib.rs +++ b/crates/spk-build/src/lib.rs @@ -17,7 +17,6 @@ pub use build::{ build_spec_path, commit_component_layers, component_marker_path, - get_package_build_env, source_package_path, validate_source_changeset, BinaryPackageBuilder, diff --git a/crates/spk-cli/cmd-build/src/cmd_build_test/environment.rs b/crates/spk-cli/cmd-build/src/cmd_build_test/environment.rs new file mode 100644 index 000000000..eae297d2a --- /dev/null +++ b/crates/spk-cli/cmd-build/src/cmd_build_test/environment.rs @@ -0,0 +1,99 @@ +// Copyright (c) Sony Pictures Imageworks, et al. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/imageworks/spk + +use rstest::rstest; +use spfs::storage::{LayerStorage, ManifestStorage, PayloadStorage}; +use spk_cli_common::BuildArtifact; +use spk_schema::foundation::fixtures::*; +use spk_schema::ident_component::Component; +use spk_storage::fixtures::*; + +use crate::try_build_package; + +#[rstest] +#[case::simple( + "- { set: TEST_VAR, value: \"${SPK_PKG_VERSION_MAJOR}\" }", + "export TEST_VAR=\"1\"\n" +)] +#[case::unexpanded_and_expanded( + "- { set: TEST_VAR, value: \"$$HOME/${SPK_PKG_VERSION_MAJOR}.${SPK_PKG_VERSION_MINOR}\" }", + "export TEST_VAR=\"$HOME/1.2\"\n" +)] +#[tokio::test] +async fn basic_environment_generation( + tmpdir: tempfile::TempDir, + #[case] env_spec: &str, + #[case] expected: &str, +) { + let rt = spfs_runtime().await; + + let (_, result) = try_build_package!( + tmpdir, + "test.spk.yaml", + format!( + r#" +pkg: test/1.2.3 +api: v0/package +build: + script: + - true +install: + environment: + {env_spec} + "# + ), + ); + + let mut result = result.expect("Expected build to succeed"); + + // Only care about binary builds (not source builds) + result + .created_builds + .artifacts + .retain(|(_, artifact)| matches!(artifact, BuildArtifact::Binary(_, _, _))); + + assert_eq!( + result.created_builds.artifacts.len(), + 1, + "Expected one build to be created" + ); + + // Check the generated activation script + + let BuildArtifact::Binary(build, _, _) = &result.created_builds.artifacts[0].1 else { + panic!("Expected binary build"); + }; + + let digest = *rt + .tmprepo + .read_components(build) + .await + .unwrap() + .get(&Component::Run) + .unwrap(); + + let spk_storage::RepositoryHandle::SPFS(repo) = &*rt.tmprepo else { + panic!("Expected SPFS repo"); + }; + + let layer = repo.read_layer(digest).await.unwrap(); + + let manifest = repo + .read_manifest( + *layer + .manifest() + .expect("Layer should have a manifest in this test"), + ) + .await + .unwrap() + .to_tracking_manifest(); + + let entry = manifest.get_path("etc/spfs/startup.d/spk_test.sh").unwrap(); + + let (mut payload, _filename) = repo.open_payload(entry.object).await.unwrap(); + let mut writer: Vec = vec![]; + tokio::io::copy(&mut payload, &mut writer).await.unwrap(); + let contents = String::from_utf8(writer).unwrap(); + assert_eq!(contents, expected); +} diff --git a/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs b/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs index 8e803bbfc..1ceee6019 100644 --- a/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs +++ b/crates/spk-cli/cmd-build/src/cmd_build_test/mod.rs @@ -2,9 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/imageworks/spk -use std::fs::File; -use std::io::Write; - use clap::Parser; use rstest::rstest; use spfs::storage::prelude::*; @@ -18,6 +15,7 @@ use spk_storage::fixtures::*; use super::Build; use crate::{build_package, try_build_package}; +mod environment; mod variant_filter; #[derive(Parser)] diff --git a/crates/spk-cli/cmd-build/src/macros.rs b/crates/spk-cli/cmd-build/src/macros.rs index df88f0d01..813396ec2 100644 --- a/crates/spk-cli/cmd-build/src/macros.rs +++ b/crates/spk-cli/cmd-build/src/macros.rs @@ -53,7 +53,8 @@ macro_rules! try_build_package { // Leak `filename` for convenience. let filename = Box::leak(Box::new($tmpdir.path().join($filename))); { - let mut file = File::create(&filename).unwrap(); + let mut file = std::fs::File::create(&filename).unwrap(); + use std::io::Write; file.write_all($recipe.as_bytes()).unwrap(); } diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index 0dc9aefcc..f8e4bd6be 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -111,9 +111,12 @@ impl EnvOp { /// Returns the EnvOp object with expanded env var, if any pub fn to_expanded(&self, env_vars: &HashMap) -> Self { - let value = self - .value() - .map(|val| shellexpand::env_with_context_no_errors(val, |s: &str| env_vars.get(s))); + let value = self.value().map(|val| { + shellexpand::env_with_context_no_errors(val, |s: &str| { + dbg!(env_vars); + env_vars.get(dbg!(s)) + }) + }); match value { Some(val) => self.update_value(val.into_owned()), diff --git a/crates/spk-schema/src/package.rs b/crates/spk-schema/src/package.rs index bb477e6b2..4e9866b05 100644 --- a/crates/spk-schema/src/package.rs +++ b/crates/spk-schema/src/package.rs @@ -3,9 +3,11 @@ // https://github.com/imageworks/spk use std::borrow::Cow; +use std::collections::HashMap; use spk_schema_foundation::ident_build::Build; use spk_schema_foundation::spec_ops::{Named, Versioned}; +use spk_schema_foundation::version::VERSION_SEP; use spk_schema_ident::BuildIdent; use super::RequirementsList; @@ -61,6 +63,40 @@ pub trait Package: /// Identify the requirements for a build of this package. fn get_build_requirements(&self) -> crate::Result>; + /// Return the environment variables to be set for a build of the given package spec. + fn get_build_env(&self) -> HashMap { + let mut env = HashMap::with_capacity(8); + env.insert("SPK_PKG".to_string(), self.ident().to_string()); + env.insert("SPK_PKG_NAME".to_string(), self.name().to_string()); + env.insert("SPK_PKG_VERSION".to_string(), self.version().to_string()); + env.insert( + "SPK_PKG_BUILD".to_string(), + self.ident().build().to_string(), + ); + env.insert( + "SPK_PKG_VERSION_MAJOR".to_string(), + self.version().major().to_string(), + ); + env.insert( + "SPK_PKG_VERSION_MINOR".to_string(), + self.version().minor().to_string(), + ); + env.insert( + "SPK_PKG_VERSION_PATCH".to_string(), + self.version().patch().to_string(), + ); + env.insert( + "SPK_PKG_VERSION_BASE".to_string(), + self.version() + .parts + .iter() + .map(u32::to_string) + .collect::>() + .join(VERSION_SEP), + ); + env + } + /// Requests that must be met to use this package fn runtime_requirements(&self) -> Cow<'_, RequirementsList>; diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index e111b2e37..88b474843 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -569,16 +569,6 @@ impl Recipe for Spec { updated.meta.update_metadata(&config.metadata)?; - // Expand env variables from EnvOp. - let mut updated_ops = Vec::new(); - let build_env_vars = build_env.env_vars(); - for op in updated.install.environment.iter() { - updated_ops.push(op.to_expanded(&build_env_vars)); - } - - updated.install.environment.clear(); - updated.install.environment.append(&mut updated_ops); - let mut missing_build_requirements = HashMap::new(); let mut missing_runtime_requirements: HashMap)> = HashMap::new(); @@ -649,7 +639,18 @@ impl Recipe for Spec { // by `build_env`. The digest is expected to be based solely on the // input options and recipe. let digest = self.build_digest(variant.input_variant())?; - Ok(updated.map_ident(|i| i.into_build(Build::BuildId(digest)))) + let mut build = updated.map_ident(|i| i.into_build(Build::BuildId(digest))); + + // Expand env variables from EnvOp. + let mut updated_ops = Vec::new(); + let mut build_env_vars = build_env.env_vars(); + build_env_vars.extend(build.get_build_env()); + for op in build.install.environment.iter() { + updated_ops.push(op.to_expanded(&build_env_vars)); + } + build.install.environment = updated_ops; + + Ok(build) } fn metadata(&self) -> &Meta {