Skip to content

Build profiles #3246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
itowlson wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Build profiles #3246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 90 additions & 7 deletions crates/build/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,18 @@ use subprocess::{Exec, Redirection};

use crate::manifest::component_build_configs;

const LAST_BUILD_PROFILE_FILE: &str = "last-build.txt";
const LAST_BUILD_ANON_VALUE: &str = "<anonymous>";
Copy link
Collaborator

@lann lann Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚲🏠

Suggested change
const LAST_BUILD_ANON_VALUE: &str = "<anonymous>";
const LAST_BUILD_NONE_VALUE: &str = "<none>";

Copy link
Collaborator Author

@itowlson itowlson Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessarily "none" (which to me implies no last build). It's usually "the anonymous profile."

Copy link
Collaborator

@lann lann Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok that makes sense.


/// If present, run the build command of each component.
pub async fn build(
manifest_file: &Path,
profile: Option<&str>,
component_ids: &[String],
target_checks: TargetChecking,
cache_root: Option<PathBuf>,
) -> Result<()> {
let build_info = component_build_configs(manifest_file)
let build_info = component_build_configs(manifest_file, profile)
.await
.with_context(|| {
format!(
Expand Down Expand Up @@ -54,6 +58,10 @@ pub async fn build(
// If the build failed, exit with an error at this point.
build_result?;

if let Err(e) = save_last_build_profile(&app_dir, profile) {
tracing::warn!("Failed to save build profile: {e:?}");
}

let Some(manifest) = build_info.manifest() else {
// We can't proceed to checking (because that needs a full healthy manifest), and we've
// already emitted any necessary warning, so quit.
Expand Down Expand Up @@ -90,8 +98,19 @@ pub async fn build(
/// Run all component build commands, using the default options (build all
/// components, perform target checking). We run a "default build" in several
/// places and this centralises the logic of what such a "default build" means.
pub async fn build_default(manifest_file: &Path, cache_root: Option<PathBuf>) -> Result<()> {
build(manifest_file, &[], TargetChecking::Check, cache_root).await
pub async fn build_default(
manifest_file: &Path,
profile: Option<&str>,
cache_root: Option<PathBuf>,
) -> Result<()> {
build(
manifest_file,
profile,
&[],
TargetChecking::Check,
cache_root,
)
.await
}

fn build_components(
Expand Down Expand Up @@ -316,6 +335,69 @@ fn sort(components: Vec<ComponentBuildInfo>) -> (Vec<ComponentBuildInfo>, bool)
}
}

/// Saves the build profile to the "last build profile" file.
pub fn save_last_build_profile(app_dir: &Path, profile: Option<&str>) -> anyhow::Result<()> {
let app_stash_dir = app_dir.join(".spin");
let last_build_profile_file = app_stash_dir.join(LAST_BUILD_PROFILE_FILE);

// This way, if the user never uses build profiles, they won't see a
// weird savefile that they have no idea what it is.
if profile.is_none() && !last_build_profile_file.exists() {
return Ok(());
}

std::fs::create_dir_all(&app_stash_dir)?;
std::fs::write(
&last_build_profile_file,
profile.unwrap_or(LAST_BUILD_ANON_VALUE),
)?;

Ok(())
}

/// Reads the last build profile from the "last build profile" file.
pub fn read_last_build_profile(app_dir: &Path) -> anyhow::Result<Option<String>> {
let app_stash_dir = app_dir.join(".spin");
let last_build_profile_file = app_stash_dir.join(LAST_BUILD_PROFILE_FILE);
if !last_build_profile_file.exists() {
return Ok(None);
}

let last_build_str = std::fs::read_to_string(&last_build_profile_file)?;

if last_build_str == LAST_BUILD_ANON_VALUE {
Ok(None)
} else {
Ok(Some(last_build_str))
}
Comment on lines 368 to 372
Copy link
Collaborator

@lann lann Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not inarguably better, but another option:

Suggested change
if last_build_str == LAST_BUILD_ANON_VALUE {
None
} else {
Some(last_build_str)
}
(last_build_str != LAST_BUILD_ANON_VALUE).then_some(last_build_str)

Copy link
Collaborator Author

@itowlson itowlson Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's too much Go programming but I find this a bit obscure and would prefer to keep spelling it out. Appreciate the education though - just give me a couple of years to internalise it!

}

/// Prints a warning to stderr if the given profile is not the same
/// as the most recent build in the given application directory.
pub fn warn_if_not_latest_build(manifest_path: &Path, profile: Option<&str>) {
let Some(app_dir) = manifest_path.parent() else {
return;
};

let latest_build = match read_last_build_profile(app_dir) {
Ok(profile) => profile,
Err(e) => {
tracing::warn!(
"Failed to read last build profile: using anonymous profile. Error was {e:?}"
);
None
}
};

if profile != latest_build.as_deref() {
let profile_opt = match profile {
Some(p) => format!(" --profile {p}"),
None => "".to_string(),
};
terminal::warn!("You built a different profile more recently than the one you are running. If the app appears to be behaving like an older version then run `spin up --build{profile_opt}`.");
}
}

/// Specifies target environment checking behaviour
pub enum TargetChecking {
/// The build should check that all components are compatible with all target environments.
Expand Down Expand Up @@ -343,23 +425,23 @@ mod tests {
#[tokio::test]
async fn can_load_even_if_trigger_invalid() {
let bad_trigger_file = test_data_root().join("bad_trigger.toml");
build(&bad_trigger_file, &[], TargetChecking::Skip, None)
build(&bad_trigger_file, None, &[], TargetChecking::Skip, None)
.await
.unwrap();
}

#[tokio::test]
async fn succeeds_if_target_env_matches() {
let manifest_path = test_data_root().join("good_target_env.toml");
build(&manifest_path, &[], TargetChecking::Check, None)
build(&manifest_path, None, &[], TargetChecking::Check, None)
.await
.unwrap();
}

#[tokio::test]
async fn fails_if_target_env_does_not_match() {
let manifest_path = test_data_root().join("bad_target_env.toml");
let err = build(&manifest_path, &[], TargetChecking::Check, None)
let err = build(&manifest_path, None, &[], TargetChecking::Check, None)
.await
.expect_err("should have failed")
.to_string();
Expand All @@ -374,7 +456,8 @@ mod tests {
#[tokio::test]
async fn has_meaningful_error_if_target_env_does_not_match() {
let manifest_file = test_data_root().join("bad_target_env.toml");
let manifest = spin_manifest::manifest_from_file(&manifest_file).unwrap();
let mut manifest = spin_manifest::manifest_from_file(&manifest_file).unwrap();
spin_manifest::normalize::normalize_manifest(&mut manifest, None).unwrap();
let application = spin_environments::ApplicationToValidate::new(
manifest.clone(),
manifest_file.parent().unwrap(),
Expand Down
9 changes: 7 additions & 2 deletions crates/build/src/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,16 @@ impl ManifestBuildInfo {
/// given (v1 or v2) manifest path. If the manifest cannot be loaded, the
/// function attempts fallback: if fallback succeeds, result is Ok but the load error
/// is also returned via the second part of the return value tuple.
pub async fn component_build_configs(manifest_file: impl AsRef<Path>) -> Result<ManifestBuildInfo> {
pub async fn component_build_configs(
manifest_file: impl AsRef<Path>,
profile: Option<&str>,
) -> Result<ManifestBuildInfo> {
let manifest = spin_manifest::manifest_from_file(&manifest_file);
match manifest {
Ok(mut manifest) => {
spin_manifest::normalize::normalize_manifest(&mut manifest)?;
manifest.ensure_profile(profile)?;

spin_manifest::normalize::normalize_manifest(&mut manifest, profile)?;
let components = build_configs_from_manifest(&manifest);
let deployment_targets = deployment_targets_from_manifest(&manifest);
Ok(ManifestBuildInfo::Loadable {
Expand Down
2 changes: 1 addition & 1 deletion crates/factors-test/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,5 @@ pub async fn build_locked_app(manifest: &toml::Table) -> anyhow::Result<LockedAp
let dir = tempfile::tempdir().context("failed creating tempdir")?;
let path = dir.path().join("spin.toml");
std::fs::write(&path, toml_str).context("failed writing manifest")?;
spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await
spin_loader::from_file(&path, FilesMountStrategy::Direct, None, None).await
}
7 changes: 4 additions & 3 deletions crates/loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,21 @@ pub(crate) const MAX_FILE_LOADING_CONCURRENCY: usize = 16;
pub async fn from_file(
manifest_path: impl AsRef<Path>,
files_mount_strategy: FilesMountStrategy,
profile: Option<&str>,
cache_root: Option<PathBuf>,
) -> Result<LockedApp> {
let path = manifest_path.as_ref();
let app_root = parent_dir(path).context("manifest path has no parent directory")?;
let loader = LocalLoader::new(&app_root, files_mount_strategy, cache_root).await?;
let loader = LocalLoader::new(&app_root, files_mount_strategy, profile, cache_root).await?;
loader.load_file(path).await
}

/// Load a Spin locked app from a standalone Wasm file.
pub async fn from_wasm_file(wasm_path: impl AsRef<Path>) -> Result<LockedApp> {
let app_root = std::env::current_dir()?;
let manifest = single_file_manifest(wasm_path)?;
let loader = LocalLoader::new(&app_root, FilesMountStrategy::Direct, None).await?;
loader.load_manifest(manifest).await
let loader = LocalLoader::new(&app_root, FilesMountStrategy::Direct, None, None).await?;
loader.load_manifest(manifest, None).await
}

/// The strategy to use for mounting WASI files into a guest.
Expand Down
25 changes: 22 additions & 3 deletions crates/loader/src/local.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ pub struct LocalLoader {
files_mount_strategy: FilesMountStrategy,
file_loading_permits: std::sync::Arc<Semaphore>,
wasm_loader: WasmLoader,
profile: Option<String>,
}

impl LocalLoader {
pub async fn new(
app_root: &Path,
files_mount_strategy: FilesMountStrategy,
profile: Option<&str>,
cache_root: Option<PathBuf>,
) -> Result<Self> {
let app_root = safe_canonicalize(app_root)
Expand All @@ -44,6 +46,7 @@ impl LocalLoader {
// Limit concurrency to avoid hitting system resource limits
file_loading_permits: file_loading_permits.clone(),
wasm_loader: WasmLoader::new(app_root, cache_root, Some(file_loading_permits)).await?,
profile: profile.map(|s| s.to_owned()),
})
}

Expand All @@ -59,7 +62,7 @@ impl LocalLoader {
)
})?;
let mut locked = self
.load_manifest(manifest)
.load_manifest(manifest, self.profile())
.await
.with_context(|| format!("Failed to load Spin app from {}", quoted_path(path)))?;

Expand All @@ -68,12 +71,23 @@ impl LocalLoader {
.metadata
.insert("origin".into(), file_url(path)?.into());

// Set build profile metadata
if let Some(profile) = self.profile.as_ref() {
locked
.metadata
.insert("profile".into(), profile.as_str().into());
}

Ok(locked)
}

// Load the given manifest into a LockedApp, ready for execution.
pub(crate) async fn load_manifest(&self, mut manifest: AppManifest) -> Result<LockedApp> {
spin_manifest::normalize::normalize_manifest(&mut manifest)?;
pub(crate) async fn load_manifest(
&self,
mut manifest: AppManifest,
profile: Option<&str>,
) -> Result<LockedApp> {
spin_manifest::normalize::normalize_manifest(&mut manifest, profile)?;

manifest.validate_dependencies()?;

Expand Down Expand Up @@ -538,6 +552,10 @@ impl LocalLoader {
path: dest.into(),
})
}

fn profile(&self) -> Option<&str> {
self.profile.as_deref()
}
}

fn explain_file_mount_source_error(e: anyhow::Error, src: &Path) -> anyhow::Error {
Expand Down Expand Up @@ -928,6 +946,7 @@ mod test {
&app_root,
FilesMountStrategy::Copy(wd.path().to_owned()),
None,
None,
)
.await?;
let err = loader
Expand Down
1 change: 1 addition & 0 deletions crates/loader/tests/ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ fn run_test(input: &Path, normalizer: &mut Normalizer) -> Result<String, Failed>
input,
spin_loader::FilesMountStrategy::Copy(files_mount_root),
None,
None,
)
.await
.map_err(|err| format!("{err:?}"))?;
Expand Down
1 change: 1 addition & 0 deletions crates/manifest/src/compat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ pub fn v1_to_v2_app(manifest: v1::AppManifestV1) -> Result<v2::AppManifest, Erro
allowed_http_hosts: Vec::new(),
dependencies_inherit_configuration: false,
dependencies: Default::default(),
profile: Default::default(),
},
);
triggers
Expand Down
48 changes: 44 additions & 4 deletions crates/manifest/src/normalize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ use anyhow::Context;
/// - Inline components in trigger configs are moved into top-level
/// components and replaced with a reference.
/// - Any triggers without an ID are assigned a generated ID.
pub fn normalize_manifest(manifest: &mut AppManifest) -> anyhow::Result<()> {
pub fn normalize_manifest(manifest: &mut AppManifest, profile: Option<&str>) -> anyhow::Result<()> {
normalize_trigger_ids(manifest);
normalize_inline_components(manifest);
apply_profile_overrides(manifest, profile);
normalize_dependency_component_refs(manifest)?;
Ok(())
}
Expand Down Expand Up @@ -107,6 +108,44 @@ fn normalize_trigger_ids(manifest: &mut AppManifest) {
}
}

fn apply_profile_overrides(manifest: &mut AppManifest, profile: Option<&str>) {
let Some(profile) = profile else {
return;
};

for (_, component) in &mut manifest.components {
let Some(overrides) = component.profile.get(profile) else {
continue;
};

if let Some(profile_build) = overrides.build.as_ref() {
match component.build.as_mut() {
None => {
component.build = Some(crate::schema::v2::ComponentBuildConfig {
command: profile_build.command.clone(),
workdir: None,
watch: vec![],
})
}
Some(build) => {
build.command = profile_build.command.clone();
}
}
}

if let Some(source) = overrides.source.as_ref() {
component.source = source.clone();
}

component.environment.extend(overrides.environment.clone());

component
.dependencies
.inner
.extend(overrides.dependencies.inner.clone());
}
}

use crate::schema::v2::{Component, ComponentDependency, ComponentSource};

fn normalize_dependency_component_refs(manifest: &mut AppManifest) -> anyhow::Result<()> {
Expand Down Expand Up @@ -191,6 +230,7 @@ fn ensure_is_acceptable_dependency(
tool: _,
dependencies_inherit_configuration: _,
dependencies,
profile: _,
} = component;

if !ai_models.is_empty() {
Expand Down Expand Up @@ -261,7 +301,7 @@ mod test {
})
.unwrap();

normalize_manifest(&mut manifest).unwrap();
normalize_manifest(&mut manifest, None).unwrap();

let dep = manifest
.components
Expand Down Expand Up @@ -301,7 +341,7 @@ mod test {
})
.unwrap();

normalize_manifest(&mut manifest).unwrap();
normalize_manifest(&mut manifest, None).unwrap();

let dep = manifest
.components
Expand Down Expand Up @@ -347,7 +387,7 @@ mod test {
})
.unwrap();

normalize_manifest(&mut manifest).unwrap();
normalize_manifest(&mut manifest, None).unwrap();

let dep = manifest
.components
Expand Down
Loading