From 4d7239742c8ba3a9d3c092fab400769c8c8f23f0 Mon Sep 17 00:00:00 2001 From: Beniamin Sandu Date: Wed, 8 Jul 2026 11:36:26 +0300 Subject: [PATCH 1/3] honor target-: overrides for both runtime/standalone builds Signed-off-by: Beniamin Sandu --- src/commands/ext/build.rs | 27 ++++++++++++++------------- src/commands/ext/image.rs | 29 ++++++++++++++++------------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/commands/ext/build.rs b/src/commands/ext/build.rs index e3c1ed7..912299c 100644 --- a/src/commands/ext/build.rs +++ b/src/commands/ext/build.rs @@ -383,19 +383,20 @@ impl ExtBuildCommand { // Then apply target-specific overrides manually // Use find_ext_in_mapping to handle template keys like "avocado-bsp-{{ avocado.target }}" let ext_section = find_ext_in_mapping(parsed, &self.extension, &target); - if let Some(ext_val) = ext_section { - let base_ext = ext_val.clone(); - // Check for target-specific override within this extension - let target_override = ext_val.get(&target).cloned(); - if let Some(override_val) = target_override { - // Merge target override into base, filtering out other target sections - Some(config.merge_target_override(base_ext, override_val, &target)) - } else { - Some(base_ext) - } - } else { - None - } + // Resolve target-: / kernel-: / legacy bare : + // overrides from the composed extension value so the preferred + // `target-:` form is honored in the runtime-build path too + // (previously only the standalone `Local` path resolved it, so a + // path-sourced ext in `avocado build` got the base config: no + // per-target overlay). + ext_section.map(|ext_val| { + config.resolve_overrides_in_value( + ext_val.clone(), + &target, + None, + &format!("extensions.{}", self.extension), + ) + }) } ExtensionLocation::Local { config_path, .. } => { // For local extensions, read from the file with proper target merging diff --git a/src/commands/ext/image.rs b/src/commands/ext/image.rs index 46f7d92..3637644 100644 --- a/src/commands/ext/image.rs +++ b/src/commands/ext/image.rs @@ -450,19 +450,22 @@ impl ExtImageCommand { } } - if let Some(ext_val) = ext_section { - let base_ext = ext_val.clone(); - // Check for target-specific override within this extension - let target_override = ext_val.get(&target).cloned(); - if let Some(override_val) = target_override { - // Merge target override into base, filtering out other target sections - Some(config.merge_target_override(base_ext, override_val, &target)) - } else { - Some(base_ext) - } - } else { - None - } + // Resolve target-: / kernel-: / legacy bare : + // overrides from the composed extension value (find_ext_in_mapping + // returns it raw). Using resolve_overrides_in_value here — rather + // than only checking a bare `:` key — means the preferred + // `target-:` form is honored in the runtime-build path too + // (previously only the standalone `Local` path resolved it, so a + // path-sourced ext in `avocado build` got the base config: no + // per-target overlay and the base `--tag`). + ext_section.map(|ext_val| { + config.resolve_overrides_in_value( + ext_val.clone(), + &target, + None, + &format!("extensions.{}", self.extension), + ) + }) } ExtensionLocation::Local { config_path, .. } => { // For local extensions, read from the file with proper target merging From 9dfadb5df47c5646d8938085abc71060963886f7 Mon Sep 17 00:00:00 2001 From: Beniamin Sandu Date: Wed, 8 Jul 2026 12:11:12 +0300 Subject: [PATCH 2/3] add focused regression test for target- Signed-off-by: Beniamin Sandu --- src/commands/ext/build.rs | 24 ++------- src/commands/ext/image.rs | 50 +++---------------- src/commands/ext/mod.rs | 102 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 61 deletions(-) diff --git a/src/commands/ext/build.rs b/src/commands/ext/build.rs index 912299c..99051fb 100644 --- a/src/commands/ext/build.rs +++ b/src/commands/ext/build.rs @@ -2,7 +2,6 @@ use anyhow::{Context, Result}; use std::collections::HashMap; use std::sync::Arc; -use super::find_ext_in_mapping; use crate::commands::sdk::SdkCompileCommand; use crate::utils::config::{ComposedConfig, Config, ExtensionLocation}; use crate::utils::container::{RunConfig, SdkContainer, TuiContext}; @@ -379,24 +378,11 @@ impl ExtBuildCommand { // For local extensions, this uses get_merged_ext_config which reads from the file let ext_config = match &extension_location { ExtensionLocation::Remote { .. } => { - // Use the already-merged config from `parsed` which contains remote extension configs - // Then apply target-specific overrides manually - // Use find_ext_in_mapping to handle template keys like "avocado-bsp-{{ avocado.target }}" - let ext_section = find_ext_in_mapping(parsed, &self.extension, &target); - // Resolve target-: / kernel-: / legacy bare : - // overrides from the composed extension value so the preferred - // `target-:` form is honored in the runtime-build path too - // (previously only the standalone `Local` path resolved it, so a - // path-sourced ext in `avocado build` got the base config: no - // per-target overlay). - ext_section.map(|ext_val| { - config.resolve_overrides_in_value( - ext_val.clone(), - &target, - None, - &format!("extensions.{}", self.extension), - ) - }) + // Resolve the remote/path-sourced ext's config from the composed + // value, honoring its `target-:` overrides (overlay + `--tag`) + // — the same result the Local path gets via get_merged_ext_config. + // Shared with `ext image`. + super::resolve_remote_ext_config(config, parsed, &self.extension, &target) } ExtensionLocation::Local { config_path, .. } => { // For local extensions, read from the file with proper target merging diff --git a/src/commands/ext/image.rs b/src/commands/ext/image.rs index 3637644..8982d2b 100644 --- a/src/commands/ext/image.rs +++ b/src/commands/ext/image.rs @@ -421,51 +421,17 @@ impl ExtImageCommand { // For local extensions, this uses get_merged_ext_config which reads from the file let ext_config = match &extension_location { ExtensionLocation::Remote { .. } => { - // Use the already-merged config from `parsed` which contains remote extension configs - // Then apply target-specific overrides manually - // Use find_ext_in_mapping to handle template keys like "avocado-bsp-{{ avocado.target }}" - let ext_section = find_ext_in_mapping(parsed, &self.extension, &target); - if self.verbose { - if let Some(all_ext) = parsed.get("extensions") { - if let Some(ext_map) = all_ext.as_mapping() { - let ext_names: Vec<_> = - ext_map.keys().filter_map(|k| k.as_str()).collect(); - eprintln!( - "[DEBUG] Available extensions in composed config: {ext_names:?}" - ); - } - } - eprintln!( - "[DEBUG] Looking for extension '{}' in composed config, found: {}", - self.extension, - ext_section.is_some() - ); - if let Some(ext_val) = &ext_section { - eprintln!( - "[DEBUG] Extension '{}' config:\n{}", - self.extension, - serde_yaml::to_string(ext_val).unwrap_or_default() - ); + if let Some(ext_map) = parsed.get("extensions").and_then(|e| e.as_mapping()) { + let ext_names: Vec<_> = ext_map.keys().filter_map(|k| k.as_str()).collect(); + eprintln!("[DEBUG] Available extensions in composed config: {ext_names:?}"); } } - - // Resolve target-: / kernel-: / legacy bare : - // overrides from the composed extension value (find_ext_in_mapping - // returns it raw). Using resolve_overrides_in_value here — rather - // than only checking a bare `:` key — means the preferred - // `target-:` form is honored in the runtime-build path too - // (previously only the standalone `Local` path resolved it, so a - // path-sourced ext in `avocado build` got the base config: no - // per-target overlay and the base `--tag`). - ext_section.map(|ext_val| { - config.resolve_overrides_in_value( - ext_val.clone(), - &target, - None, - &format!("extensions.{}", self.extension), - ) - }) + // Resolve the remote/path-sourced ext's config from the composed + // value, honoring its `target-:` overrides (overlay + `--tag`) + // — the same result the Local path gets via get_merged_ext_config. + // Shared with `ext build`. + super::resolve_remote_ext_config(config, parsed, &self.extension, &target) } ExtensionLocation::Local { config_path, .. } => { // For local extensions, read from the file with proper target merging diff --git a/src/commands/ext/mod.rs b/src/commands/ext/mod.rs index 4ba9dec..e26b99f 100644 --- a/src/commands/ext/mod.rs +++ b/src/commands/ext/mod.rs @@ -60,6 +60,36 @@ pub(crate) fn find_ext_in_mapping<'a>( None } +/// Resolve a remote / path-sourced extension's effective config from the +/// composed value, applying its `target-:` (and legacy bare `:`) +/// per-target overrides. Any `kernel-:` override keys are stripped, not +/// applied — the kernel version isn't known at ext build/image time, so we pass +/// `resolved_kver: None` (the same as `get_merged_ext_config`, keeping the +/// `Local` and remote paths consistent). +/// +/// Shared by `ext build` and `ext image`. The composed config holds the base +/// extension keys plus its `target-:` sub-sections (as +/// `merge_installed_remote_extensions` produces for a path/remote source); +/// running it through `resolve_overrides_in_value` makes the runtime-build path +/// honor the same overrides the standalone `Local` path gets via +/// `get_merged_ext_config` — otherwise only a bare `:` key matched and +/// the preferred `target-:` form (overlay + `--tag`) was silently ignored. +/// Returns `None` when the extension isn't present in the composed config. +pub(crate) fn resolve_remote_ext_config( + config: &crate::utils::config::Config, + parsed: &serde_yaml::Value, + extension_name: &str, + target: &str, +) -> Option { + let ext_val = find_ext_in_mapping(parsed, extension_name, target)?; + Some(config.resolve_overrides_in_value( + ext_val.clone(), + target, + None, + &format!("extensions.{extension_name}"), + )) +} + #[cfg(test)] mod tests { use super::*; @@ -68,6 +98,78 @@ mod tests { serde_yaml::from_str(yaml).unwrap() } + /// Regression: a remote / path-sourced extension in a runtime build has the + /// base keys plus `target-:` sub-sections in the composed value. The + /// shared resolver (used by `ext build` + `ext image`) must honor the + /// preferred `target-:` overlay + `--tag`, not just a bare `:` + /// key, and must strip the override sub-keys rather than leak them. + #[test] + fn test_resolve_remote_ext_config_target_prefix_override() { + let yaml = r#" +supported_targets: + - raspberrypi4 + - qemux86-64 +sdk: + image: "docker.io/avocadolinux/sdk:apollo-edge" +extensions: + kos-layer-boardconf: + version: 2026.7.0 + image: + type: kab + args: '-b -t kos.layer -v 2026.7.0 --tag {{ avocado.target }}' + target-raspberrypi4: + overlay: overlay/raspberrypi4 + target-qemux86-64: + overlay: overlay/qemu-x64 + image: + args: '-b -t kos.layer -v 2026.7.0 --tag qemu-x64' +"#; + let config = crate::utils::config::Config::load_from_yaml_str(yaml).unwrap(); + let parsed = make_config(yaml); + + // qemux86-64: target overlay + tag win, base image.type kept, overrides stripped. + let q = resolve_remote_ext_config(&config, &parsed, "kos-layer-boardconf", "qemux86-64") + .expect("extension present"); + assert_eq!( + q.get("overlay").and_then(|v| v.as_str()), + Some("overlay/qemu-x64") + ); + let q_args = q + .get("image") + .and_then(|i| i.get("args")) + .and_then(|v| v.as_str()) + .unwrap(); + assert!(q_args.contains("--tag qemu-x64"), "got: {q_args}"); + assert_eq!( + q.get("image") + .and_then(|i| i.get("type")) + .and_then(|v| v.as_str()), + Some("kab") + ); + assert!(q.get("target-qemux86-64").is_none()); + assert!(q.get("target-raspberrypi4").is_none()); + + // raspberrypi4: its overlay wins (no image override → base args remain). + let r = resolve_remote_ext_config(&config, &parsed, "kos-layer-boardconf", "raspberrypi4") + .expect("extension present"); + assert_eq!( + r.get("overlay").and_then(|v| v.as_str()), + Some("overlay/raspberrypi4") + ); + + // A target with no matching override gets the base only — no overlay + // leaks from a sibling target-* section. + let o = resolve_remote_ext_config(&config, &parsed, "kos-layer-boardconf", "qemuarm64") + .expect("extension present"); + assert!(o.get("overlay").is_none()); + assert!(o.get("target-qemux86-64").is_none()); + + // Absent extension → None. + assert!( + resolve_remote_ext_config(&config, &parsed, "does-not-exist", "qemux86-64").is_none() + ); + } + #[test] fn test_find_ext_direct_lookup() { let config = make_config( From 726c1c59207b7edf0971b7e94f7771bee5622141 Mon Sep 17 00:00:00 2001 From: Beniamin Sandu Date: Tue, 14 Jul 2026 09:53:18 +0300 Subject: [PATCH 3/3] support target- overrides for kernel- Signed-off-by: Beniamin Sandu --- src/utils/config.rs | 83 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/utils/config.rs b/src/utils/config.rs index 14d736d..fade232 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -3072,6 +3072,12 @@ impl Config { /// 3. `target-:` match /// 4. `kernel-:` matches (in source order) /// + /// A matched `target-:` (and legacy bare-target) block is resolved + /// RECURSIVELY with the same target + resolved kernel before being merged, + /// so override keys nested inside it — notably `kernel-:` — compose + /// (per-board × per-kernel). Without this a nested `kernel-:` would + /// leak through as a literal key instead of being applied/stripped. + /// /// `section_path` is for diagnostic context only. pub fn resolve_overrides_in_value( &self, @@ -3157,9 +3163,14 @@ impl Config { let mut merged = serde_yaml::Value::Mapping(map); if let Some(v) = legacy_target_match { + // Recurse so override keys nested inside the target block (e.g. + // kernel-:) resolve against the same target + kernel instead + // of leaking through as literal keys. + let v = self.resolve_overrides_in_value(v, current_target, resolved_kver, section_path); merged = self.merge_values(merged, v); } if let Some(v) = target_match { + let v = self.resolve_overrides_in_value(v, current_target, resolved_kver, section_path); merged = self.merge_values(merged, v); } for v in kernel_matches { @@ -8075,6 +8086,78 @@ extensions: std::fs::remove_file(temp_file).ok(); } + #[test] + fn test_nested_kernel_under_target_resolves() { + // A shared BSP-style extension: per-board packages live under + // target-, with kernel- package blocks NESTED inside the + // target block. The resolver must recurse into the matched target + // block and apply the kernel block matching the resolved kernel. + let config = + Config::load_from_str("supported_targets: [\"raspberrypi4\", \"raspberrypi5\"]\n") + .unwrap(); + + let ext: serde_yaml::Value = serde_yaml::from_str( + r#" +packages: + common-pkg: '*' +target-raspberrypi5: + packages: + board-pkg: '*' + kernel-6.6.*: + packages: + rpivid-hevc: '*' + kernel-6.12.*: + packages: + rpi-hevc-dec: '*' + drm-shmem-helper: '*' +"#, + ) + .unwrap(); + + // raspberrypi5 on a 6.12 kernel -> base + target + nested 6.12 block. + let r = config.resolve_overrides_in_value( + ext.clone(), + "raspberrypi5", + Some("6.12.25"), + "extensions.avocado-bsp", + ); + let pkgs = r + .get("packages") + .and_then(|p| p.as_mapping()) + .expect("packages"); + assert!(pkgs.contains_key("common-pkg")); + assert!(pkgs.contains_key("board-pkg")); + assert!(pkgs.contains_key("rpi-hevc-dec")); + assert!(pkgs.contains_key("drm-shmem-helper")); + assert!(!pkgs.contains_key("rpivid-hevc")); + // No override keys leak through, at the section level or into packages. + assert!(r.get("target-raspberrypi5").is_none()); + assert!(r.get("kernel-6.12.*").is_none()); + assert!(!pkgs.contains_key("kernel-6.12.*")); + assert!(!pkgs.contains_key("kernel-6.6.*")); + + // raspberrypi5 on a 6.6 kernel -> nested 6.6 block instead. + let r66 = config.resolve_overrides_in_value( + ext.clone(), + "raspberrypi5", + Some("6.6.90"), + "extensions.avocado-bsp", + ); + let p66 = r66.get("packages").and_then(|p| p.as_mapping()).unwrap(); + assert!(p66.contains_key("rpivid-hevc")); + assert!(!p66.contains_key("rpi-hevc-dec")); + + // Unknown kernel (build/image path, resolved_kver = None): nested + // kernel blocks are stripped, not applied and not leaked. + let rnone = + config.resolve_overrides_in_value(ext, "raspberrypi5", None, "extensions.avocado-bsp"); + let pnone = rnone.get("packages").and_then(|p| p.as_mapping()).unwrap(); + assert!(pnone.contains_key("board-pkg")); + assert!(!pnone.contains_key("rpi-hevc-dec")); + assert!(!pnone.contains_key("rpivid-hevc")); + assert!(!pnone.contains_key("kernel-6.12.*")); + } + #[test] fn test_edge_cases_and_error_conditions() { // Test configuration with only target-specific sections