From f682a4cfdd8d2367b78fe5ece820b427e4a41692 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Mon, 27 Jul 2026 14:44:08 -0400 Subject: [PATCH] fix(sdk/install): stop reinstalling rootfs and initramfs on every run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `avocado sdk install` redid the rootfs and initramfs installs in full on every invocation, even when nothing had changed. Two independent defects caused it. **1. False "packages removed" detection wiped both sysroots every run.** `detect_sysroot_package_removals` compared *config-declared* package names against *locked* names and returned true — meaning `rm -rf $AVOCADO_PREFIX/` plus a from-scratch reinstall — whenever the lock held names the config didn't. But the install auto-appends two packages the config never declares, then records them in the lock: - `packagegroup-avocado-{rootfs,initramfs}-modules-` - `kernel-image-` (rootfs only) From the second run onward those always read as removed. Because `get_rootfs_packages` / `get_initramfs_packages` default to the single meta-package, this fired on *every* project, not only ones overriding `rootfs.packages`. Confirmed against every real lockfile in the tree — for example `references/jetson-trt/avocado.lock`: | | locked names | config names | |---|---|---| | rootfs | `avocado-pkg-rootfs`, `kernel-image-6.8.12-…`, `packagegroup-avocado-rootfs-modules-6.8.12-…` | `avocado-pkg-rootfs` | | initramfs | `avocado-pkg-initramfs`, `packagegroup-avocado-initramfs-modules-6.8.12-…` | `avocado-pkg-initramfs` | **Second-order effect:** the same path called `remove_packages_from_sysroot`, dropping those packages' version pins from the in-memory lock. `build_package_spec_with_lock` then fell back to a bare package name, so dnf resolved the kernel image and module packagegroup to *newest available* instead of the pinned NVR. `avocado.lock` looked pinned on disk (it is rewritten post-install) but the pin never applied. **2. The install stamps were written but never read.** `install_sysroot` wrote `rootfs/install.stamp` and `initramfs/install.stamp`, but nothing consulted them to skip work — across the whole CLI, stamps were only prerequisite gates. So even with defect 1 fixed, each run still paid a kernel repoquery, off-kernel exclude computation, a repoquery existence probe, the dnf transaction, `query_installed_packages`, kernel-sysroot staging, and the stamp write. ## Solution An unchanged config now does zero container work for rootfs/initramfs; a changed one reinstalls exactly the sysroot(s) affected; `avocado.lock` pins actually bind. ## Key changes - **Compare removals against the effective set.** `install_sysroot` resolves the kernel and computes the auto-appended names *before* removal detection, and `detect_sysroot_package_removals` now takes the effective name set (config packages ∪ auto-appends) and returns the removed names rather than a bool. That set is built once and reused as the post-install lock query list, replacing a second, duplicate computation. A kver change falls out of the same mechanism, so the dedicated kernel-pin-change block became the *reason* for the clean rather than a second code path, and the two verbatim inline `rm -rf` RunConfig blocks collapse into one `clean_sysroot` helper built from the existing `clean_sysroot_command`. - **Skip the install when the stamp is current.** New `read_stamps_batch` / `StampBatch` in `utils/prerequisites.rs` wraps `generate_batch_read_stamps_script` + one container run + parsing. `sdk install` reads both sysroot stamps in a single invocation before the parallel phase and passes them in via `SysrootInstallParams`; the standalone `rootfs install` / `initramfs install` read their own through the same helper. `install_sysroot` short-circuits before any container call. Stamps live in the `/opt/_avocado` named volume rather than a host bind mount, so a container run is unavoidable — hence batching. `check_prerequisites` and five sites that hand-rolled the same RunConfig (`build.rs`, `sdk/{clean,package}.rs`, `ext/{clean,checkout}.rs`) are ported to the helper. - **Make the hash trustworthy enough to skip on.** The two per-sysroot hash functions collapse into one core that now hashes the *effective* package map (so an absent `rootfs.packages` and an explicit default hash identically), plus `sdk.repo_url`, `sdk.repo_release` — the hook that makes a snapshot bump or `avocado update` land — and `sdk.disable_weak_dependencies`. `StampInputs.package_list_hash` is populated with a digest of that sysroot's lockfile pins, so a hand-edited `avocado.lock` or an `avocado unlock` goes stale automatically. An empty pin set hashes to its own value rather than to `None`, because `Stamp::is_current` only compares that field when both sides carry one — otherwise a cleared lock would compare equal by omission. `STAMP_VERSION` is deliberately *not* bumped: that would invalidate every SDK/ext/runtime stamp. Changing only these two hash bodies makes existing rootfs/initramfs stamps mismatch naturally, costing one reinstall. - **Make `clean` invalidate its stamp.** `clean_sysroot_command` also removes `$AVOCADO_PREFIX/.stamps/`, mirroring `ext clean`. Without it, `rootfs clean && sdk install` would skip against a current stamp and leave an empty sysroot. - **Drop the mid-flight lockfile saves.** `install_sysroot` and `stage_kernel_sysroot_from_rootfs` saved their lockfile *clone* while the rootfs and initramfs tasks ran concurrently on separate clones — last-writer-wins, healed only by the caller's final save. Saving is now the caller's job throughout, which removes the race. Escape hatches are unchanged in spirit: `--no-stamps` disables the read along with the write, so it always reinstalls, and `avocado rootfs clean` remains the hard reset. ## Reviewer notes `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, and the full `cargo test` suite are green (1124 lib tests plus the integration suites, no failures). New unit tests: - `detect_sysroot_package_removals` reports nothing for the exact `references/jetson-trt/avocado.lock` shape — a default config with both auto-appended kernel packages locked. This is the regression test for the bug. - It still reports a genuinely config-dropped package, and reports the previous kver's auto-appends as stale after a repin. - The rootfs hash is stable across absent-vs-explicit-default `rootfs.packages`, and moves on an added package, on `repo_url`, `repo_release` and `disable_weak_dependencies`, and on a lock re-pin. - `package_list_hash` is order-independent (pins come out of a `HashMap`), and an `avocado unlock` invalidates `is_current`. - A rootfs-only config edit does not invalidate the initramfs hash. End-to-end verification against a live feed is in progress separately. --- CHANGELOG.md | 23 + src/commands/build.rs | 44 +- src/commands/ext/checkout.rs | 44 +- src/commands/ext/clean.rs | 45 +- src/commands/initramfs/install.rs | 31 +- src/commands/rootfs/clean.rs | 8 +- src/commands/rootfs/install.rs | 684 ++++++++++++++++++++---------- src/commands/sdk/clean.rs | 42 +- src/commands/sdk/install.rs | 39 +- src/commands/sdk/package.rs | 42 +- src/utils/prerequisites.rs | 106 ++++- src/utils/stamps.rs | 470 +++++++++++++++----- 12 files changed, 1125 insertions(+), 453 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72b752ba..f914c0a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **Rootfs and initramfs no longer reinstall on every run.** `avocado sdk + install` wiped and rebuilt both sysroots from scratch on every invocation, + even with nothing changed. Removal detection compared the lockfile against + *config-declared* packages only, so the per-kernel packages the install + auto-appends (`packagegroup-avocado-{rootfs,initramfs}-modules-` and + `kernel-image-`) read as "removed" from the second run onward and + forced a clean reinstall. It now compares against the effective set — + config packages plus those auto-appends. + As a second-order effect of that false positive, the same path dropped those + packages' version pins from the in-memory lockfile, so dnf resolved the + kernel image and module packagegroup to newest-available instead of the + locked NVR. `avocado.lock` looked pinned on disk but the pin never applied; + it now binds. +- **Unchanged sysroots are skipped outright.** The rootfs and initramfs install + stamps were written but never read, so each run still paid a kernel + repoquery, a dnf transaction, and a lockfile rewrite. Both stamps are now + read (batched into one container invocation) and a current stamp short- + circuits the install. The stamp hash covers the effective package set, + `sdk.repo_url`, `sdk.repo_release`, `sdk.disable_weak_dependencies`, and a + digest of the sysroot's lockfile pins, so a snapshot bump, a feed switch, or + an `avocado unlock` all invalidate it. `avocado {rootfs,initramfs} clean` now + removes the install stamp along with the sysroot, and `--no-stamps` still + forces a full reinstall. - **`--connect-sign` guidance.** The deploy help text and the Level 2 setup messages now reference `avocado connect trust promote-root --key ` with its required `--key` option, matching the CLI reference documentation. diff --git a/src/commands/build.rs b/src/commands/build.rs index 274b52f0..27816e84 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -175,10 +175,7 @@ impl BuildCommand { // task list. This gives the user a clear "run avocado install" message // instead of failing mid-build inside a TUI. if !self.no_stamps { - use crate::utils::stamps::{ - generate_batch_read_stamps_script, resolve_required_stamps, validate_stamps_batch, - StampCommand, StampComponent, - }; + use crate::utils::stamps::{resolve_required_stamps, StampCommand, StampComponent}; let container_image = config.get_sdk_image().ok_or_else(|| { anyhow::anyhow!("No container image specified in config under 'sdk.image'") @@ -192,27 +189,24 @@ impl BuildCommand { let required = resolve_required_stamps(StampCommand::Install, StampComponent::Runtime, None, &[]); - let batch_script = generate_batch_read_stamps_script(&required); - let run_config = crate::utils::container::RunConfig { - container_image: container_image.clone(), - target: target.clone(), - command: batch_script, - verbose: false, - source_environment: true, - interactive: false, - repo_url: config.get_sdk_repo_url(), - repo_release: config.get_sdk_repo_release(), - container_args: self.container_args.clone(), - sdk_arch: self.sdk_arch.clone(), - runs_on: self.runs_on.clone(), - nfs_port: self.nfs_port, - ..Default::default() - }; - - let output = container_helper - .run_in_container_with_output(run_config) - .await?; - let validation = validate_stamps_batch(&required, output.as_deref().unwrap_or(""), &[]); + let validation = crate::utils::prerequisites::read_stamps_batch( + &required, + &container_helper, + crate::utils::container::RunConfig { + container_image: container_image.clone(), + target: target.clone(), + repo_url: config.get_sdk_repo_url(), + repo_release: config.get_sdk_repo_release(), + container_args: self.container_args.clone(), + sdk_arch: self.sdk_arch.clone(), + runs_on: self.runs_on.clone(), + nfs_port: self.nfs_port, + ..Default::default() + }, + None, + ) + .await? + .validate(&required, &[]); if !validation.is_satisfied() { let error = diff --git a/src/commands/ext/checkout.rs b/src/commands/ext/checkout.rs index b121ff28..d1a9b728 100644 --- a/src/commands/ext/checkout.rs +++ b/src/commands/ext/checkout.rs @@ -7,9 +7,8 @@ use tokio::process::Command as AsyncCommand; use crate::utils::config::{ComposedConfig, Config}; use crate::utils::container::{RunConfig, SdkContainer}; use crate::utils::output::{print_error, print_info, print_success, OutputLevel}; -use crate::utils::stamps::{ - generate_batch_read_stamps_script, validate_stamps_batch, StampRequirement, -}; +use crate::utils::prerequisites::read_stamps_batch; +use crate::utils::stamps::StampRequirement; use crate::utils::target::resolve_target_required; use crate::utils::volume::VolumeManager; @@ -115,28 +114,23 @@ impl ExtCheckoutCommand { StampRequirement::ext_install(&self.extension), ]; - let batch_script = generate_batch_read_stamps_script(&requirements); - let run_config = RunConfig { - container_image: container_image.to_string(), - target: target.clone(), - command: batch_script, - verbose: false, - source_environment: true, - interactive: false, - repo_url: config.get_sdk_repo_url(), - repo_release: config.get_sdk_repo_release(), - container_args: config.merge_sdk_container_args(None), - sdk_arch: self.sdk_arch.clone(), - env_vars: self.runtime_env_vars(), - ..Default::default() - }; - - let output = container_helper - .run_in_container_with_output(run_config) - .await?; - - let validation = - validate_stamps_batch(&requirements, output.as_deref().unwrap_or(""), &[]); + let validation = read_stamps_batch( + &requirements, + &container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.clone(), + repo_url: config.get_sdk_repo_url(), + repo_release: config.get_sdk_repo_release(), + container_args: config.merge_sdk_container_args(None), + sdk_arch: self.sdk_arch.clone(), + env_vars: self.runtime_env_vars(), + ..Default::default() + }, + None, + ) + .await? + .validate(&requirements, &[]); if !validation.is_satisfied() { validation diff --git a/src/commands/ext/clean.rs b/src/commands/ext/clean.rs index 6fd13c7a..73ebada7 100644 --- a/src/commands/ext/clean.rs +++ b/src/commands/ext/clean.rs @@ -6,9 +6,8 @@ use super::find_ext_in_mapping; use crate::utils::config::{ComposedConfig, Config, ExtensionLocation}; use crate::utils::container::{RunConfig, SdkContainer}; use crate::utils::output::{print_error, print_info, print_success, OutputLevel}; -use crate::utils::stamps::{ - generate_batch_read_stamps_script, validate_stamps_batch, StampRequirement, -}; +use crate::utils::prerequisites::read_stamps_batch; +use crate::utils::stamps::StampRequirement; use crate::utils::target::resolve_target_required; pub struct ExtCleanCommand { @@ -214,28 +213,24 @@ impl ExtCleanCommand { // Validate SDK is installed before running clean scripts let requirements = vec![StampRequirement::sdk_install()]; - let batch_script = generate_batch_read_stamps_script(&requirements); - let run_config = RunConfig { - container_image: container_image.to_string(), - target: target.to_string(), - command: batch_script, - verbose: false, - source_environment: true, - interactive: false, - repo_url: repo_url.clone(), - repo_release: repo_release.clone(), - container_args: merged_container_args.clone(), - dnf_args: self.dnf_args.clone(), - sdk_arch: self.sdk_arch.clone(), - env_vars: self.runtime_env_vars(), - ..Default::default() - }; - - let output = container_helper - .run_in_container_with_output(run_config) - .await?; - - let validation = validate_stamps_batch(&requirements, output.as_deref().unwrap_or(""), &[]); + let validation = read_stamps_batch( + &requirements, + &container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.to_string(), + repo_url: repo_url.clone(), + repo_release: repo_release.clone(), + container_args: merged_container_args.clone(), + dnf_args: self.dnf_args.clone(), + sdk_arch: self.sdk_arch.clone(), + env_vars: self.runtime_env_vars(), + ..Default::default() + }, + None, + ) + .await? + .validate(&requirements, &[]); if !validation.is_satisfied() { validation diff --git a/src/commands/initramfs/install.rs b/src/commands/initramfs/install.rs index 5fdbfc09..a7b7d0d3 100644 --- a/src/commands/initramfs/install.rs +++ b/src/commands/initramfs/install.rs @@ -5,14 +5,16 @@ use std::sync::Arc; use crate::utils::{ config::{ComposedConfig, Config}, - container::SdkContainer, + container::{RunConfig, SdkContainer}, lockfile::{LockFile, SysrootType}, output::{print_error, OutputLevel}, runs_on::RunsOnContext, target::validate_and_log_target, }; -use crate::commands::rootfs::install::{install_sysroot, SysrootInstallParams}; +use crate::commands::rootfs::install::{ + install_sysroot, read_sysroot_install_stamp, SysrootInstallParams, +}; /// Implementation of the 'initramfs install' command. pub struct InitramfsInstallCommand { @@ -128,6 +130,24 @@ impl InitramfsInstallCommand { .unwrap_or(std::path::Path::new(".")); let mut lock_file = LockFile::load(src_dir)?; + let prefetched_stamp = read_sysroot_install_stamp( + &SysrootType::Initramfs, + self.no_stamps, + &container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.to_string(), + verbose: self.verbose, + repo_url: repo_url.clone(), + repo_release: repo_release.clone(), + container_args: merged_container_args.clone(), + sdk_arch: self.sdk_arch.clone(), + ..Default::default() + }, + runs_on_context.as_ref(), + ) + .await?; + let result = install_sysroot(&mut SysrootInstallParams { sysroot_type: SysrootType::Initramfs, config, @@ -147,10 +167,17 @@ impl InitramfsInstallCommand { sdk_arch: self.sdk_arch.as_ref(), no_stamps: self.no_stamps, parsed: Some(&composed.merged_value), + prefetched_stamp, tui_context: None, }) .await; + // Persist the lockfile the install updated — `install_sysroot` leaves + // saving to its caller. + if result.is_ok() { + lock_file.save(src_dir)?; + } + if let Some(ref mut context) = runs_on_context { if let Err(e) = context.teardown().await { print_error( diff --git a/src/commands/rootfs/clean.rs b/src/commands/rootfs/clean.rs index 4501591f..1d0b61a0 100644 --- a/src/commands/rootfs/clean.rs +++ b/src/commands/rootfs/clean.rs @@ -11,8 +11,14 @@ use crate::utils::{ }; /// Generate the shell command to clean a sysroot (rootfs or initramfs). +/// +/// Removes the install stamp along with the sysroot. Without this, `rootfs +/// clean && sdk install` would see a current stamp against an empty sysroot +/// and skip the reinstall (see the short-circuit in +/// [`install_sysroot`](super::install::install_sysroot)) — mirroring how +/// `ext clean` drops `.stamps/ext/`. pub fn clean_sysroot_command(sysroot_dir: &str) -> String { - format!(r#"rm -rf "$AVOCADO_PREFIX/{sysroot_dir}""#) + format!(r#"rm -rf "$AVOCADO_PREFIX/{sysroot_dir}" "$AVOCADO_PREFIX/.stamps/{sysroot_dir}""#) } /// Implementation of the 'rootfs clean' command. diff --git a/src/commands/rootfs/install.rs b/src/commands/rootfs/install.rs index 2c54d3fa..c287d50a 100644 --- a/src/commands/rootfs/install.rs +++ b/src/commands/rootfs/install.rs @@ -1,7 +1,7 @@ //! Rootfs sysroot install command and shared install logic for rootfs/initramfs. use anyhow::{Context, Result}; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::Path; use std::sync::Arc; @@ -75,14 +75,17 @@ use crate::utils::{ kernel_version::substitute_kernel_version, lockfile::{build_package_spec_with_lock, LockFile, SysrootType}, output::{print_error, print_info, print_success, OutputLevel}, + prerequisites::read_stamps_batch, runs_on::RunsOnContext, stamps::{ compute_initramfs_input_hash, compute_rootfs_input_hash, generate_write_stamp_script, - Stamp, StampOutputs, + Stamp, StampInputs, StampOutputs, StampRequirement, SysrootStampInputs, }, target::validate_and_log_target, }; +use super::clean::clean_sysroot_command; + /// Parameters for the shared sysroot install function. pub struct SysrootInstallParams<'a> { pub sysroot_type: SysrootType, @@ -102,14 +105,59 @@ pub struct SysrootInstallParams<'a> { pub force: bool, pub runs_on_context: Option<&'a RunsOnContext>, pub sdk_arch: Option<&'a String>, - /// Skip stamp writing when true. + /// Skip stamp reading and writing when true — the escape hatch that + /// forces a full reinstall. pub no_stamps: bool, /// Parsed (merged) YAML config — needed for stamp hash computation. pub parsed: Option<&'a serde_yaml::Value>, + /// This sysroot's install stamp, read by the caller. `avocado sdk install` + /// batches both sysroots' stamps into one container invocation; the + /// standalone commands read their own. `None` means no stamp was found + /// (or none was read), which always installs. + pub prefetched_stamp: Option, /// TUI context for output capture (if TUI is active). pub tui_context: Option, } +impl SysrootInstallParams<'_> { + /// The `SysrootType`-appropriate stamp requirement, or `None` for a + /// sysroot type that has no install stamp. + fn stamp_requirement(sysroot_type: &SysrootType) -> Option { + match sysroot_type { + SysrootType::Rootfs => Some(StampRequirement::rootfs_install()), + SysrootType::Initramfs => Some(StampRequirement::initramfs_install()), + _ => None, + } + } +} + +/// Read one sysroot's install stamp for the standalone `avocado rootfs +/// install` / `avocado initramfs install` entry points, which have no +/// sibling task to batch with. Returns `None` when stamps are disabled, the +/// stamp is absent, or it can't be parsed. +pub async fn read_sysroot_install_stamp( + sysroot_type: &SysrootType, + no_stamps: bool, + container_helper: &SdkContainer, + base_run_config: RunConfig, + runs_on_context: Option<&RunsOnContext>, +) -> Result> { + if no_stamps { + return Ok(None); + } + let Some(requirement) = SysrootInstallParams::stamp_requirement(sysroot_type) else { + return Ok(None); + }; + let batch = read_stamps_batch( + std::slice::from_ref(&requirement), + container_helper, + base_run_config, + runs_on_context, + ) + .await?; + Ok(batch.stamp_for(&requirement)) +} + /// Stage the kernel `Image` from the rootfs sysroot into the per-target /// content-addressed kernel sysroot at `$AVOCADO_PREFIX/kernel//`. /// @@ -134,7 +182,6 @@ async fn stage_kernel_sysroot_from_rootfs( rootfs_image_pkg_name: &str, rootfs_image_pkg_version: &str, lock_file: &mut LockFile, - src_dir: &Path, repo_url: Option<&str>, repo_release: Option<&str>, merged_container_args: Option>, @@ -230,7 +277,6 @@ fi ); let kernel_sysroot = SysrootType::Kernel(kver.to_string()); lock_file.update_sysroot_versions(target, &kernel_sysroot, versions); - lock_file.save(src_dir)?; print_success( &format!("Staged kernel sysroot at $AVOCADO_PREFIX/kernel/{kver}."), @@ -240,49 +286,116 @@ fi Ok(()) } -/// Detect package removals by comparing config packages against lock file. -/// Returns true if the sysroot needs to be cleaned and reinstalled from scratch. +/// Detect package removals by comparing the **effective** package set for +/// this sysroot against what the lockfile recorded. A non-empty result means +/// the sysroot must be cleaned and reinstalled from scratch, because dnf +/// install is additive-only and cannot remove packages. +/// +/// `effective_names` is the config-declared packages *plus* the packages +/// [`install_sysroot`] auto-appends: the per-kernel module packagegroup and, +/// for rootfs, `kernel-image-`. Deriving the reference set from config +/// alone — as this did before — reads those auto-appended lock entries as +/// removals on every run after the first, which wipes both sysroots and, +/// via `remove_packages_from_sysroot`, drops their version pins so dnf +/// resolves newest-available instead of the locked NVR. +/// +/// Returns the removed names sorted, or empty when the sysroot is consistent. fn detect_sysroot_package_removals( - config: &Config, + effective_names: &HashSet, sysroot_type: &SysrootType, target: &str, - lock_file: &mut LockFile, -) -> bool { + lock_file: &LockFile, +) -> Vec { let locked_names = lock_file.get_locked_package_names(target, sysroot_type); if locked_names.is_empty() { - return false; + return Vec::new(); } - let config_names: HashSet = match sysroot_type { - SysrootType::Rootfs => config.get_rootfs_packages().keys().cloned().collect(), - SysrootType::Initramfs => config.get_initramfs_packages().keys().cloned().collect(), - _ => return false, - }; + let mut removed: Vec = locked_names.difference(effective_names).cloned().collect(); + removed.sort(); + removed +} - let removed: Vec = locked_names.difference(&config_names).cloned().collect(); +/// `rm -rf` a sysroot (and its stamp) inside the SDK container. +/// +/// Shares [`clean_sysroot_command`] with `avocado rootfs clean` so the +/// mid-install clean and the user-facing clean can't drift. +/// +/// Best-effort by design, matching the inline copies this replaced: an +/// absent sysroot is the normal case on a first install and must not fail +/// the run. +async fn clean_sysroot(params: &SysrootInstallParams<'_>, sysroot_dir: &str) { + let clean_config = RunConfig { + container_image: params.container_image.to_string(), + target: params.target.to_string(), + command: clean_sysroot_command(sysroot_dir), + verbose: params.verbose, + source_environment: true, + interactive: false, + repo_url: params.repo_url.map(|s| s.to_string()), + repo_release: params.repo_release.map(|s| s.to_string()), + container_args: params.merged_container_args.clone(), + sdk_arch: params.sdk_arch.cloned(), + tui_context: params.tui_context.clone(), + ..Default::default() + }; - if removed.is_empty() { - return false; + if let Some(context) = params.runs_on_context { + params + .container_helper + .run_in_container_with_context(&clean_config, context) + .await + .ok(); + } else { + params + .container_helper + .run_in_container(clean_config) + .await + .ok(); } +} - let label = match sysroot_type { - SysrootType::Rootfs => "rootfs", - SysrootType::Initramfs => "initramfs", - _ => "sysroot", +/// Compute this sysroot's install-stamp inputs from the config, the SDK feed +/// identity, and the lockfile pins **as they stand at call time**. +/// +/// Called twice per install: once up front to compare against the stamp on +/// record, and once after a successful install to write the stamp the next +/// run will compare against. The second call has to see the post-install +/// lock (the install re-pins packages), which is why this reads the lock +/// each time instead of caching a single value. +/// +/// Returns `None` when there is no parsed config to hash, or for a sysroot +/// type that has no install stamp. +fn compute_install_stamp_inputs( + params: &SysrootInstallParams<'_>, + packages: &HashMap, +) -> Result> { + let Some(parsed) = params.parsed else { + return Ok(None); }; - print_info( - &format!( - "Packages removed from {label}: {}. Cleaning sysroot for fresh install.", - removed.join(", ") - ), - OutputLevel::Normal, - ); - // Remove only the stale entries, preserving version pins for remaining packages - lock_file.remove_packages_from_sysroot(target, sysroot_type, &removed); + let resolved = SysrootStampInputs { + packages, + repo_url: params.repo_url, + repo_release: params.repo_release, + disable_weak_dependencies: params.config.get_sdk_disable_weak_dependencies(), + locked_packages: params + .lock_file + .get_sysroot_versions(params.target, ¶ms.sysroot_type), + }; - true + let inputs = match params.sysroot_type { + SysrootType::Rootfs => { + compute_rootfs_input_hash(parsed, params.src_dir, params.target_board, &resolved)? + } + SysrootType::Initramfs => { + compute_initramfs_input_hash(parsed, params.src_dir, params.target_board, &resolved)? + } + _ => return Ok(None), + }; + + Ok(Some(inputs)) } /// Install a sysroot (rootfs or initramfs) via DNF into the SDK container volume. @@ -347,57 +460,38 @@ pub async fn install_sysroot(params: &mut SysrootInstallParams<'_>) -> Result<() _ => return Err(anyhow::anyhow!("Unsupported sysroot type for install")), }; - print_info(&format!("Installing {label} sysroot."), OutputLevel::Normal); - - // Detect package removals: compare current config packages with lock file. - // If packages were removed, we must clean the sysroot and reinstall from scratch - // because DNF install is additive-only and cannot remove packages. - let needs_clean_reinstall = detect_sysroot_package_removals( - params.config, - ¶ms.sysroot_type, - params.target, - params.lock_file, - ); - - if needs_clean_reinstall { - let clean_command = format!(r#"rm -rf "$AVOCADO_PREFIX/{sysroot_dir}""#); - let clean_config = RunConfig { - container_image: params.container_image.to_string(), - target: params.target.to_string(), - command: clean_command, - verbose: params.verbose, - source_environment: true, - interactive: false, - repo_url: params.repo_url.map(|s| s.to_string()), - repo_release: params.repo_release.map(|s| s.to_string()), - container_args: params.merged_container_args.clone(), - sdk_arch: params.sdk_arch.cloned(), - tui_context: params.tui_context.clone(), - ..Default::default() - }; - - if let Some(context) = params.runs_on_context { - params - .container_helper - .run_in_container_with_context(&clean_config, context) - .await - .ok(); - } else { - params - .container_helper - .run_in_container(clean_config) - .await - .ok(); - } - } - - // Get packages from config + // Get packages from config (the effective set — absent config yields the + // default meta-package). let packages = match params.sysroot_type { SysrootType::Rootfs => params.config.get_rootfs_packages(), SysrootType::Initramfs => params.config.get_initramfs_packages(), _ => unreachable!(), }; + // Short-circuit: nothing to do when the stamp on record still matches the + // current inputs. This is checked before any container call, so an + // unchanged project pays nothing here — no kernel repoquery, no dnf + // transaction, no lock rewrite. + // + // Escape hatches: `--no-stamps` skips the read (and the write) so it + // always reinstalls, and `avocado {rootfs,initramfs} clean` removes the + // stamp along with the sysroot, so a cleaned sysroot never skips. + if !params.no_stamps { + if let Some(stamp) = params.prefetched_stamp.as_ref() { + if let Some(inputs) = compute_install_stamp_inputs(params, &packages)? { + if stamp.is_current(&inputs) { + print_success( + &format!("{label} sysroot is up to date."), + OutputLevel::Normal, + ); + return Ok(()); + } + } + } + } + + print_info(&format!("Installing {label} sysroot."), OutputLevel::Normal); + // Resolve (or reuse a pinned) KERNEL_VERSION before building package specs // so kernel/kernel-module-*/kernel-devsrc-* names get suffixed to exactly // one kernel — avoiding dnf's virtual-provider tie-break picking @@ -436,77 +530,6 @@ pub async fn install_sysroot(params: &mut SysrootInstallParams<'_>) -> Result<() (kver, excludes) }; - // Detect kernel pin change vs lockfile. dnf install is additive, so if - // the resolved kver differs from what the lockfile recorded for this - // sysroot, a plain re-install would land the new kernel-image and - // module packagegroup *alongside* the prior pin's packages — leaving - // /lib/modules//, the old kernel-image, and stale module - // packages in the sysroot. Force a clean+reinstall so the new pin is - // the only thing present. - if let Some(new_kver) = resolved_kver.as_deref() { - if let Some(prev) = prev_pinned_kver { - if prev != new_kver { - print_info( - &format!( - "{label}: kernel pin changed ({prev} -> {new_kver}); cleaning sysroot for fresh install" - ), - OutputLevel::Normal, - ); - - let clean_command = format!(r#"rm -rf "$AVOCADO_PREFIX/{sysroot_dir}""#); - let clean_config = RunConfig { - container_image: params.container_image.to_string(), - target: params.target.to_string(), - command: clean_command, - verbose: params.verbose, - source_environment: true, - interactive: false, - repo_url: params.repo_url.map(|s| s.to_string()), - repo_release: params.repo_release.map(|s| s.to_string()), - container_args: params.merged_container_args.clone(), - sdk_arch: params.sdk_arch.cloned(), - tui_context: params.tui_context.clone(), - ..Default::default() - }; - if let Some(context) = params.runs_on_context { - params - .container_helper - .run_in_container_with_context(&clean_config, context) - .await - .ok(); - } else { - params - .container_helper - .run_in_container(clean_config) - .await - .ok(); - } - - // Wipe the package state for this sysroot so a failed - // re-install can't leave a stale package map pointing at a - // now-empty sysroot. - match params.sysroot_type { - SysrootType::Rootfs => params.lock_file.clear_rootfs(params.target), - SysrootType::Initramfs => params.lock_file.clear_initramfs(params.target), - _ => {} - } - // Remove and immediately re-pin the new kver. Remove first so - // the entry is correct even if the install below fails (empty - // sysroot + correct kver = retry without re-clean). Re-pin - // so the sdk/install.rs merge site can see the new kver after - // a successful install — without it the `if let Some(kver)` - // check in that merge finds nothing and the old kver from the - // initial clone bleeds through into the saved lockfile. - params - .lock_file - .remove_kernel_version(params.target, ¶ms.sysroot_type); - params - .lock_file - .set_kernel_version(params.target, ¶ms.sysroot_type, new_kver); - } - } - } - // Build package specs for all configured packages. When we have a // resolved kernel version, substitute any `{{ avocado.kernel.version }}` // templates in package keys so BSP yamls can produce fully-versioned @@ -593,6 +616,97 @@ pub async fn install_sysroot(params: &mut SysrootInstallParams<'_>) -> Result<() _ => None, }; + // The set of names a completed install of this sysroot is expected to + // have recorded in the lockfile: config-declared packages plus whatever + // was auto-appended above. Used as the reference set for removal + // detection and, after the install, as the lock query list. + let mut effective_names: Vec = if packages.is_empty() { + vec![default_pkg.to_string()] + } else { + packages.keys().cloned().collect() + }; + effective_names.extend(auto_module_pkg.iter().cloned()); + effective_names.extend(auto_kernel_image_pkg.iter().cloned()); + effective_names.sort(); + effective_names.dedup(); + let effective_name_set: HashSet = effective_names.iter().cloned().collect(); + + // Decide whether this install has to start from an empty sysroot. dnf + // install is additive-only, so anything that makes the *existing* + // sysroot contents wrong — rather than merely incomplete — needs a wipe + // first. Two things qualify, and both resolve to the same single clean. + let kernel_pin_change = resolved_kver.as_deref().and_then(|new_kver| { + prev_pinned_kver + .filter(|prev| prev != new_kver) + .map(|prev| (prev, new_kver)) + }); + let removed_packages = if kernel_pin_change.is_some() { + // Moot: the whole package map is about to be cleared below. + Vec::new() + } else { + detect_sysroot_package_removals( + &effective_name_set, + ¶ms.sysroot_type, + params.target, + params.lock_file, + ) + }; + + let needs_clean_reinstall = kernel_pin_change.is_some() || !removed_packages.is_empty(); + + if let Some((prev, new_kver)) = kernel_pin_change { + // A plain re-install would land the new kernel-image and module + // packagegroup *alongside* the prior pin's packages, leaving + // /lib/modules//, the old kernel-image and stale module + // packages behind. + print_info( + &format!( + "{label}: kernel pin changed ({prev} -> {new_kver}); cleaning sysroot for fresh install" + ), + OutputLevel::Normal, + ); + + // Wipe the package state for this sysroot so a failed re-install + // can't leave a stale package map pointing at a now-empty sysroot. + match params.sysroot_type { + SysrootType::Rootfs => params.lock_file.clear_rootfs(params.target), + SysrootType::Initramfs => params.lock_file.clear_initramfs(params.target), + _ => {} + } + // Remove and immediately re-pin the new kver. Remove first so the + // entry is correct even if the install below fails (empty sysroot + + // correct kver = retry without re-clean). Re-pin so the + // sdk/install.rs merge site can see the new kver after a successful + // install — without it the `if let Some(kver)` check in that merge + // finds nothing and the old kver from the initial clone bleeds + // through into the saved lockfile. + params + .lock_file + .remove_kernel_version(params.target, ¶ms.sysroot_type); + params + .lock_file + .set_kernel_version(params.target, ¶ms.sysroot_type, new_kver); + } else if !removed_packages.is_empty() { + print_info( + &format!( + "Packages removed from {label}: {}. Cleaning sysroot for fresh install.", + removed_packages.join(", ") + ), + OutputLevel::Normal, + ); + // Drop only the stale entries, preserving version pins for the + // packages that remain. + params.lock_file.remove_packages_from_sysroot( + params.target, + ¶ms.sysroot_type, + &removed_packages, + ); + } + + if needs_clean_reinstall { + clean_sysroot(params, sysroot_dir).await; + } + let mut pkg_specs: Vec = if packages.is_empty() { vec![build_package_spec_with_lock( params.lock_file, @@ -636,19 +750,6 @@ pub async fn install_sysroot(params: &mut SysrootInstallParams<'_>) -> Result<() } let pkg = pkg_specs.join(" "); - // Collect all package names for lock file queries - let mut all_package_names: Vec = if packages.is_empty() { - vec![default_pkg.to_string()] - } else { - packages.keys().cloned().collect() - }; - if let Some(ref name) = auto_module_pkg { - all_package_names.push(name.clone()); - } - if let Some(ref name) = auto_kernel_image_pkg { - all_package_names.push(name.clone()); - } - let yes = if params.force { "-y" } else { "" }; let dnf_args_str = if let Some(args) = ¶ms.dnf_args { format!(" {} ", args.join(" ")) @@ -764,7 +865,7 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ .container_helper .query_installed_packages( ¶ms.sysroot_type, - &all_package_names, + &effective_names, params.container_image, params.target, params.repo_url.map(|s| s.to_string()), @@ -788,7 +889,10 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ OutputLevel::Normal, ); } - params.lock_file.save(params.src_dir)?; + // Persisting is the caller's job. The rootfs and initramfs tasks + // run concurrently on separate lockfile clones, so saving a clone + // here is last-writer-wins against the sibling task; the caller + // merges both and saves once. } // Stage the kernel sysroot from the rootfs (Phase 2c). Only when: @@ -817,7 +921,6 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ kernel_image_pkg, &pkg_version, params.lock_file, - params.src_dir, params.repo_url, params.repo_release, params.merged_container_args.clone(), @@ -839,63 +942,56 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ } } - // Write install stamp (unless --no-stamps or no parsed config available) + // Write install stamp (unless --no-stamps or no parsed config available). + // + // Computed fresh rather than reusing the value the skip check above + // derived: the install just re-pinned this sysroot's packages, and the + // stamp has to record the lock state the *next* run will compare + // against. if !params.no_stamps { - if let Some(parsed) = params.parsed { - let stamp_result = match params.sysroot_type { + if let Some(inputs) = compute_install_stamp_inputs(params, &packages)? { + let stamp = match params.sysroot_type { SysrootType::Rootfs => { - let inputs = - compute_rootfs_input_hash(parsed, params.src_dir, params.target_board)?; - let outputs = StampOutputs::default(); - Ok(Stamp::rootfs_install(params.target, inputs, outputs)) + Stamp::rootfs_install(params.target, inputs, StampOutputs::default()) } SysrootType::Initramfs => { - let inputs = compute_initramfs_input_hash( - parsed, - params.src_dir, - params.target_board, - )?; - let outputs = StampOutputs::default(); - Ok(Stamp::initramfs_install(params.target, inputs, outputs)) + Stamp::initramfs_install(params.target, inputs, StampOutputs::default()) } - _ => Err(anyhow::anyhow!("Unsupported sysroot type for stamps")), + _ => unreachable!("sysroot type was validated at entry"), }; - if let Ok(stamp) = stamp_result { - let stamp_script = generate_write_stamp_script(&stamp)?; - let stamp_config = RunConfig { - container_image: params.container_image.to_string(), - target: params.target.to_string(), - command: stamp_script, - verbose: params.verbose, - source_environment: true, - interactive: false, - repo_url: params.repo_url.map(|s| s.to_string()), - repo_release: params.repo_release.map(|s| s.to_string()), - container_args: params.merged_container_args.clone(), - sdk_arch: params.sdk_arch.cloned(), - tui_context: params.tui_context.clone(), - ..Default::default() - }; - - if let Some(context) = params.runs_on_context { - params - .container_helper - .run_in_container_with_context(&stamp_config, context) - .await?; - } else { - params - .container_helper - .run_in_container(stamp_config) - .await?; - } + let stamp_config = RunConfig { + container_image: params.container_image.to_string(), + target: params.target.to_string(), + command: generate_write_stamp_script(&stamp)?, + verbose: params.verbose, + source_environment: true, + interactive: false, + repo_url: params.repo_url.map(|s| s.to_string()), + repo_release: params.repo_release.map(|s| s.to_string()), + container_args: params.merged_container_args.clone(), + sdk_arch: params.sdk_arch.cloned(), + tui_context: params.tui_context.clone(), + ..Default::default() + }; - if params.verbose { - print_info( - &format!("Wrote install stamp for {label}."), - OutputLevel::Normal, - ); - } + if let Some(context) = params.runs_on_context { + params + .container_helper + .run_in_container_with_context(&stamp_config, context) + .await?; + } else { + params + .container_helper + .run_in_container(stamp_config) + .await?; + } + + if params.verbose { + print_info( + &format!("Wrote install stamp for {label}."), + OutputLevel::Normal, + ); } } } @@ -1021,6 +1117,24 @@ impl RootfsInstallCommand { .unwrap_or(std::path::Path::new(".")); let mut lock_file = LockFile::load(src_dir)?; + let prefetched_stamp = read_sysroot_install_stamp( + &SysrootType::Rootfs, + self.no_stamps, + &container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.to_string(), + verbose: self.verbose, + repo_url: repo_url.clone(), + repo_release: repo_release.clone(), + container_args: merged_container_args.clone(), + sdk_arch: self.sdk_arch.clone(), + ..Default::default() + }, + runs_on_context.as_ref(), + ) + .await?; + let result = install_sysroot(&mut SysrootInstallParams { sysroot_type: SysrootType::Rootfs, config, @@ -1040,10 +1154,18 @@ impl RootfsInstallCommand { sdk_arch: self.sdk_arch.as_ref(), no_stamps: self.no_stamps, parsed: Some(&composed.merged_value), + prefetched_stamp, tui_context: None, }) .await; + // Persist the lockfile the install updated. `install_sysroot` no + // longer saves for itself — under `avocado sdk install` it runs on a + // clone that the caller merges and saves once. + if result.is_ok() { + lock_file.save(src_dir)?; + } + // Always teardown runs_on context if let Some(ref mut context) = runs_on_context { if let Err(e) = context.teardown().await { @@ -1060,7 +1182,139 @@ impl RootfsInstallCommand { #[cfg(test)] mod tests { - use super::build_overlay_script; + use super::{build_overlay_script, detect_sysroot_package_removals}; + use crate::utils::lockfile::{LockFile, SysrootType}; + use std::collections::{HashMap, HashSet}; + + const KVER: &str = "6.8.12-l4t-r39.2.0-1021.21"; + const TARGET: &str = "jetson-agx-thor"; + + fn lock_with(sysroot: &SysrootType, names: &[&str]) -> LockFile { + let mut lock = LockFile::new(); + let versions: HashMap = names + .iter() + .map(|n| (n.to_string(), "2026.9-r0.0".to_string())) + .collect(); + lock.update_sysroot_versions(TARGET, sysroot, versions); + lock + } + + fn name_set(names: &[&str]) -> HashSet { + names.iter().map(|n| n.to_string()).collect() + } + + #[test] + fn removal_detection_ignores_auto_appended_kernel_packages() { + // Regression test for the bug that made `avocado sdk install` wipe and + // reinstall both sysroots on every run. The lock shape below is + // verbatim from references/jetson-trt/avocado.lock: a default config + // (which declares only the meta-package) plus the two packages + // install_sysroot auto-appends for a pinned kernel. Comparing the lock + // against *config* names alone reads those two as removed forever. + let rootfs_lock = lock_with( + &SysrootType::Rootfs, + &[ + "avocado-pkg-rootfs", + &format!("kernel-image-{KVER}"), + &format!("packagegroup-avocado-rootfs-modules-{KVER}"), + ], + ); + let rootfs_effective = name_set(&[ + "avocado-pkg-rootfs", + &format!("kernel-image-{KVER}"), + &format!("packagegroup-avocado-rootfs-modules-{KVER}"), + ]); + assert!( + detect_sysroot_package_removals( + &rootfs_effective, + &SysrootType::Rootfs, + TARGET, + &rootfs_lock, + ) + .is_empty(), + "steady-state rootfs must not report removals" + ); + + // Initramfs gets the module packagegroup but no kernel-image. + let initramfs_lock = lock_with( + &SysrootType::Initramfs, + &[ + "avocado-pkg-initramfs", + &format!("packagegroup-avocado-initramfs-modules-{KVER}"), + ], + ); + let initramfs_effective = name_set(&[ + "avocado-pkg-initramfs", + &format!("packagegroup-avocado-initramfs-modules-{KVER}"), + ]); + assert!( + detect_sysroot_package_removals( + &initramfs_effective, + &SysrootType::Initramfs, + TARGET, + &initramfs_lock, + ) + .is_empty(), + "steady-state initramfs must not report removals" + ); + } + + #[test] + fn removal_detection_flags_genuinely_dropped_config_package() { + let lock = lock_with( + &SysrootType::Rootfs, + &["avocado-pkg-rootfs", "vim", &format!("kernel-image-{KVER}")], + ); + // `vim` was removed from rootfs.packages; the auto-appends still apply. + let effective = name_set(&["avocado-pkg-rootfs", &format!("kernel-image-{KVER}")]); + + assert_eq!( + detect_sysroot_package_removals(&effective, &SysrootType::Rootfs, TARGET, &lock), + vec!["vim".to_string()], + ); + } + + #[test] + fn removal_detection_flags_stale_kernel_auto_appends() { + // A kernel repin leaves the previous kver's auto-appended packages in + // the lock. Those genuinely are stale and must force a clean, so the + // new pin isn't installed alongside the old one. + let lock = lock_with( + &SysrootType::Rootfs, + &[ + "avocado-pkg-rootfs", + &format!("kernel-image-{KVER}"), + &format!("packagegroup-avocado-rootfs-modules-{KVER}"), + ], + ); + let new_kver = "6.8.12-l4t-r39.2.0-9999.99"; + let effective = name_set(&[ + "avocado-pkg-rootfs", + &format!("kernel-image-{new_kver}"), + &format!("packagegroup-avocado-rootfs-modules-{new_kver}"), + ]); + + assert_eq!( + detect_sysroot_package_removals(&effective, &SysrootType::Rootfs, TARGET, &lock), + vec![ + format!("kernel-image-{KVER}"), + format!("packagegroup-avocado-rootfs-modules-{KVER}"), + ], + ); + } + + #[test] + fn removal_detection_is_noop_on_first_install() { + // Nothing locked yet — every config package is about to be installed, + // not removed. + let lock = LockFile::new(); + let effective = name_set(&["avocado-pkg-rootfs"]); + + assert!( + detect_sysroot_package_removals(&effective, &SysrootType::Rootfs, TARGET, &lock) + .is_empty() + ); + } #[test] fn overlay_script_uses_cp_a_in_merge_mode() { diff --git a/src/commands/sdk/clean.rs b/src/commands/sdk/clean.rs index 903d146e..c0f0492e 100644 --- a/src/commands/sdk/clean.rs +++ b/src/commands/sdk/clean.rs @@ -7,7 +7,8 @@ use crate::utils::{ config::{ComposedConfig, Config}, container::{RunConfig, SdkContainer}, output::{print_error, print_info, print_success, OutputLevel}, - stamps::{generate_batch_read_stamps_script, validate_stamps_batch, StampRequirement}, + prerequisites::read_stamps_batch, + stamps::StampRequirement, target::resolve_target_required, }; @@ -111,28 +112,23 @@ impl SdkCleanCommand { if !self.sections.is_empty() { // Validate SDK is installed before running clean scripts let requirements = vec![StampRequirement::sdk_install()]; - let batch_script = generate_batch_read_stamps_script(&requirements); - let run_config = RunConfig { - container_image: container_image.to_string(), - target: target.clone(), - command: batch_script, - verbose: false, - source_environment: true, - interactive: false, - repo_url: repo_url.clone(), - repo_release: repo_release.clone(), - container_args: merged_container_args.clone(), - dnf_args: self.dnf_args.clone(), - sdk_arch: self.sdk_arch.clone(), - ..Default::default() - }; - - let output = container_helper - .run_in_container_with_output(run_config) - .await?; - - let validation = - validate_stamps_batch(&requirements, output.as_deref().unwrap_or(""), &[]); + let validation = read_stamps_batch( + &requirements, + &container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.clone(), + repo_url: repo_url.clone(), + repo_release: repo_release.clone(), + container_args: merged_container_args.clone(), + dnf_args: self.dnf_args.clone(), + sdk_arch: self.sdk_arch.clone(), + ..Default::default() + }, + None, + ) + .await? + .validate(&requirements, &[]); if !validation.is_satisfied() { validation diff --git a/src/commands/sdk/install.rs b/src/commands/sdk/install.rs index f90c9828..2c47a382 100644 --- a/src/commands/sdk/install.rs +++ b/src/commands/sdk/install.rs @@ -12,11 +12,12 @@ use crate::utils::{ kernel_resolver::{off_kernel_dnf_excludes, resolve_and_pin_kernel_version, ResolveParams}, lockfile::{build_package_spec_with_lock, LockFile, SysrootType}, output::{print_error, print_info, print_success, OutputLevel}, + prerequisites::read_stamps_batch, runs_on::RunsOnContext, stamps::{ compute_compile_deps_input_hash, compute_sdk_input_hash, generate_write_sdk_stamp_script_dynamic_arch, generate_write_stamp_script, get_local_arch, - Stamp, StampOutputs, + Stamp, StampOutputs, StampRequirement, }, target::validate_and_log_target, tui::{TaskId, TaskStatus, TuiGuard}, @@ -414,6 +415,40 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ None }; + // Read both sysroots' install stamps in a single container + // invocation, before the parallel phase starts. `install_sysroot` + // compares each against its freshly computed inputs and skips the + // whole install when they match — so an unchanged project does no + // container work for rootfs/initramfs at all. Batching matters + // because stamps live in the `/opt/_avocado` named volume, not a host + // bind mount, so each read otherwise costs its own container run. + let (rootfs_stamp, initramfs_stamp) = if self.no_stamps { + (None, None) + } else { + let rootfs_req = StampRequirement::rootfs_install(); + let initramfs_req = StampRequirement::initramfs_install(); + let batch = read_stamps_batch( + &[rootfs_req.clone(), initramfs_req.clone()], + container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.to_string(), + verbose: self.verbose, + repo_url: repo_url.map(|s| s.to_string()), + repo_release: repo_release.map(|s| s.to_string()), + container_args: merged_container_args.cloned(), + sdk_arch: self.sdk_arch.clone(), + ..Default::default() + }, + runs_on_context, + ) + .await?; + ( + batch.stamp_for(&rootfs_req), + batch.stamp_for(&initramfs_req), + ) + }; + // Register parallel tasks on TUI and signal status transitions. if let Some(r) = crate::utils::tui::get_active_renderer() { r.set_status(&TaskId::SdkInstall, TaskStatus::Success); @@ -481,6 +516,7 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ sdk_arch: self.sdk_arch.as_ref(), no_stamps: self.no_stamps, parsed: Some(&composed.merged_value), + prefetched_stamp: rootfs_stamp, tui_context: rootfs_tui, }; let mut initramfs_params = SysrootInstallParams { @@ -502,6 +538,7 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ sdk_arch: self.sdk_arch.as_ref(), no_stamps: self.no_stamps, parsed: Some(&composed.merged_value), + prefetched_stamp: initramfs_stamp, tui_context: initramfs_tui, }; diff --git a/src/commands/sdk/package.rs b/src/commands/sdk/package.rs index 9aea2aec..8bc921bc 100644 --- a/src/commands/sdk/package.rs +++ b/src/commands/sdk/package.rs @@ -12,7 +12,8 @@ use crate::utils::{ config::{Config, PackageConfig, SplitPackageConfig}, container::{RunConfig, SdkContainer}, output::{print_info, print_success, OutputLevel}, - stamps::{generate_batch_read_stamps_script, validate_stamps_batch, StampRequirement}, + prerequisites::read_stamps_batch, + stamps::StampRequirement, target::resolve_target_required, }; @@ -102,28 +103,23 @@ impl SdkPackageCommand { SdkContainer::from_config(&self.config_path, config)?.verbose(self.verbose); let requirements = vec![StampRequirement::sdk_install()]; - let batch_script = generate_batch_read_stamps_script(&requirements); - let run_config = RunConfig { - container_image: container_image.to_string(), - target: target.clone(), - command: batch_script, - verbose: false, - source_environment: true, - interactive: false, - repo_url: config.get_sdk_repo_url(), - repo_release: config.get_sdk_repo_release(), - container_args: config.merge_sdk_container_args(self.container_args.as_ref()), - dnf_args: self.dnf_args.clone(), - sdk_arch: self.sdk_arch.clone(), - ..Default::default() - }; - - let output = container_helper - .run_in_container_with_output(run_config) - .await?; - - let validation = - validate_stamps_batch(&requirements, output.as_deref().unwrap_or(""), &[]); + let validation = read_stamps_batch( + &requirements, + &container_helper, + RunConfig { + container_image: container_image.to_string(), + target: target.clone(), + repo_url: config.get_sdk_repo_url(), + repo_release: config.get_sdk_repo_release(), + container_args: config.merge_sdk_container_args(self.container_args.as_ref()), + dnf_args: self.dnf_args.clone(), + sdk_arch: self.sdk_arch.clone(), + ..Default::default() + }, + None, + ) + .await? + .validate(&requirements, &[]); if !validation.is_satisfied() { validation diff --git a/src/utils/prerequisites.rs b/src/utils/prerequisites.rs index 6b79c1e3..47725dd9 100644 --- a/src/utils/prerequisites.rs +++ b/src/utils/prerequisites.rs @@ -9,10 +9,85 @@ use anyhow::{Context, Result}; use crate::utils::container::{RunConfig, SdkContainer}; +use crate::utils::runs_on::RunsOnContext; use crate::utils::stamps::{ - generate_batch_read_stamps_script, validate_stamps_batch, StampRequirement, + generate_batch_read_stamps_script, parse_batch_stamps_output, validate_stamps_batch, + CurrentInput, Stamp, StampRequirement, StampValidationResult, }; +/// Stamps read from the SDK container in a single invocation, keyed by +/// relative stamp path. +pub struct StampBatch { + /// Raw batch output, kept so [`StampBatch::validate`] can defer to the + /// existing string-based validator. + raw: String, + stamps: std::collections::HashMap>, +} + +impl StampBatch { + /// The stamp recorded for `req`, or `None` when it is absent or its JSON + /// can't be parsed. An unparseable stamp is treated as missing on + /// purpose: it was written by a different (or corrupted) CLI and its + /// hashes can't be trusted for a skip decision. + pub fn stamp_for(&self, req: &StampRequirement) -> Option { + self.stamps + .get(&req.relative_path())? + .as_deref() + .and_then(|json| Stamp::from_json(json).ok()) + } + + /// Validate `requirements` against freshly computed inputs. See + /// [`validate_stamps_batch`] for how requirements are matched to inputs. + pub fn validate( + &self, + requirements: &[StampRequirement], + current_inputs: &[CurrentInput<'_>], + ) -> StampValidationResult { + validate_stamps_batch(requirements, &self.raw, current_inputs) + } +} + +/// Read every stamp in `requirements` in one container invocation. +/// +/// Stamps live in the `/opt/_avocado` named volume rather than a host bind +/// mount, so reading them always costs a container run — hence batching. +/// +/// `base_run_config` supplies the caller's container settings (image, +/// target, repo, container args, arch, …); its `command`, `source_environment` +/// and `interactive` fields are overwritten. Pass `runs_on_context` to read +/// from a remote host set up with `--runs-on`. +pub async fn read_stamps_batch( + requirements: &[StampRequirement], + container: &SdkContainer, + base_run_config: RunConfig, + runs_on_context: Option<&RunsOnContext>, +) -> Result { + let run_config = RunConfig { + command: generate_batch_read_stamps_script(requirements), + source_environment: true, + interactive: false, + ..base_run_config + }; + + let raw = if let Some(context) = runs_on_context { + container + .run_in_container_with_output_remote(&run_config, context) + .await + .context("Failed to read stamps in remote SDK container")? + } else { + container + .run_in_container_with_output(run_config) + .await + .context("Failed to read stamps in SDK container")? + } + .unwrap_or_default(); + + Ok(StampBatch { + stamps: parse_batch_stamps_output(&raw), + raw, + }) +} + /// A command that has prerequisite stamps that must be satisfied before it can run. pub trait TaskPrerequisites { /// Returns the list of stamps that must be present before this task runs. @@ -41,24 +116,19 @@ pub async fn check_prerequisites( return Ok(()); } - let batch_script = generate_batch_read_stamps_script(&requirements); - - let run_config = RunConfig { - container_image: container_image.to_string(), - target: target.to_string(), - command: batch_script, - source_environment: true, - interactive: false, - ..Default::default() - }; - - let stdout = container - .run_in_container_with_output(run_config) - .await - .context("Failed to run prerequisite stamp check")? - .unwrap_or_default(); + let batch = read_stamps_batch( + &requirements, + container, + RunConfig { + container_image: container_image.to_string(), + target: target.to_string(), + ..Default::default() + }, + None, + ) + .await?; - let validation = validate_stamps_batch(&requirements, &stdout, &[]); + let validation = batch.validate(&requirements, &[]); if !validation.is_satisfied() { validation diff --git a/src/utils/stamps.rs b/src/utils/stamps.rs index 764ca778..51f87056 100644 --- a/src/utils/stamps.rs +++ b/src/utils/stamps.rs @@ -106,8 +106,9 @@ impl StampInputs { } } - /// Create stamp inputs with both hashes (for future output-based staleness detection) - #[allow(unused)] + /// Create stamp inputs with both hashes. Used by the sysroot install + /// steps, which fold the lockfile pins in force at install time into + /// `package_list_hash` so a re-pin invalidates independently of config. pub fn with_package_list(config_hash: String, package_list_hash: String) -> Self { Self { config_hash, @@ -1230,35 +1231,97 @@ fn ext_build_hash_data( Ok(hash_data) } -/// Compute input hash for **rootfs install**. +/// The inputs to a sysroot install stamp that cannot be read out of the +/// merged YAML: the *effective* package set (config default already +/// applied), the SDK feed identity the install resolves against, and the +/// lockfile pins currently in force. /// -/// Includes `rootfs.packages`, `rootfs.overlay`, and the narrowed kernel -/// selection (`package`/`version`/`compile`/`install` only — adding an -/// unrelated `kernel.metadata` field does NOT invalidate). Also includes -/// the `post_install` hook path and its file contents so an in-place -/// script edit invalidates without `--no-stamps`. -pub fn compute_rootfs_input_hash( +/// These are what make the hash trustworthy enough to *skip* an install on. +/// Hashing the raw `rootfs.packages` node alone cannot tell an absent +/// section from one that spells out the default meta-package, and says +/// nothing about a snapshot bump or a hand-edited `avocado.lock`. +pub struct SysrootStampInputs<'a> { + /// Effective package map — `Config::get_{rootfs,initramfs}_packages`. + pub packages: &'a std::collections::HashMap, + /// `sdk.repo_url`: a feed switch must invalidate. + pub repo_url: Option<&'a str>, + /// `sdk.repo_release`: the resolved snapshot, which `avocado update` moves. + pub repo_release: Option<&'a str>, + /// `sdk.disable_weak_dependencies`: changes what dnf pulls in. + pub disable_weak_dependencies: bool, + /// Locked NVR pins for this sysroot, as recorded in `avocado.lock`. + pub locked_packages: Option<&'a std::collections::HashMap>, +} + +/// Digest of a sysroot's lockfile pins as `name=version` lines ordered by +/// name. +/// +/// Deliberately always returns a hash rather than `None` for an empty pin +/// set: [`Stamp::is_current`] only compares `package_list_hash` when *both* +/// sides carry one, so returning `None` after `avocado unlock` cleared the +/// section would let a stamp written against real pins compare equal by +/// omission. An empty set hashing to its own distinct value makes that read +/// as stale. +fn package_list_hash(locked: Option<&std::collections::HashMap>) -> String { + let mut lines: Vec = locked + .map(|pins| { + pins.iter() + .map(|(name, version)| format!("{name}={version}")) + .collect() + }) + .unwrap_or_default(); + lines.sort(); + compute_hash(&lines.join("\n")) +} + +/// Render the effective package map as a deterministically ordered YAML +/// mapping. `HashMap` iteration order varies per process and +/// [`compute_config_hash`] serializes in insertion order, so the keys have +/// to be sorted here or the hash is unstable between runs. +fn packages_for_hash( + packages: &std::collections::HashMap, +) -> serde_yaml::Value { + let mut names: Vec<&String> = packages.keys().collect(); + names.sort(); + let mut out = serde_yaml::Mapping::new(); + for name in names { + out.insert( + serde_yaml::Value::String(name.clone()), + packages[name].clone(), + ); + } + serde_yaml::Value::Mapping(out) +} + +/// Shared input-hash core for the rootfs and initramfs installs, which take +/// identical inputs under different config sections. `section` is the +/// top-level key (`"rootfs"` / `"initramfs"`). +fn compute_sysroot_install_input_hash( + section: &str, config: &serde_yaml::Value, project_root: &Path, cli_target_board: Option<&str>, + resolved: &SysrootStampInputs<'_>, ) -> Result { let mut hash_data = serde_yaml::Mapping::new(); - if let Some(rootfs) = config.get("rootfs") { - if let Some(packages) = rootfs.get("packages") { - hash_data.insert( - serde_yaml::Value::String("rootfs.packages".to_string()), - packages.clone(), - ); - } - if let Some(overlay) = rootfs.get("overlay") { + // The effective set, not the raw `
.packages` node — an absent + // section and one that names the default meta-package install the same + // thing and must hash the same. + hash_data.insert( + serde_yaml::Value::String(format!("{section}.packages")), + packages_for_hash(resolved.packages), + ); + + if let Some(sysroot) = config.get(section) { + if let Some(overlay) = sysroot.get("overlay") { hash_data.insert( - serde_yaml::Value::String("rootfs.overlay".to_string()), + serde_yaml::Value::String(format!("{section}.overlay")), overlay.clone(), ); fold_overlay_content_hash( &mut hash_data, - "rootfs.overlay_content", + &format!("{section}.overlay_content"), overlay, config, project_root, @@ -1267,9 +1330,9 @@ pub fn compute_rootfs_input_hash( cli_target_board, )?; } - if let Some(post_install) = rootfs.get("post_install").and_then(|v| v.as_str()) { + if let Some(post_install) = sysroot.get("post_install").and_then(|v| v.as_str()) { hash_data.insert( - serde_yaml::Value::String("rootfs.post_install".to_string()), + serde_yaml::Value::String(format!("{section}.post_install")), script_hash_value(project_root, post_install), ); } @@ -1282,61 +1345,66 @@ pub fn compute_rootfs_input_hash( ); } + // Feed identity and resolver flags. A snapshot bump, a feed switch, or a + // weak-deps flip all change what lands in the sysroot even when every + // config section above is byte-identical. + for (key, value) in [ + ("sdk.repo_url", resolved.repo_url), + ("sdk.repo_release", resolved.repo_release), + ] { + if let Some(v) = value { + hash_data.insert( + serde_yaml::Value::String(key.to_string()), + serde_yaml::Value::String(v.to_string()), + ); + } + } + hash_data.insert( + serde_yaml::Value::String("sdk.disable_weak_dependencies".to_string()), + serde_yaml::Value::Bool(resolved.disable_weak_dependencies), + ); + let config_hash = compute_config_hash(&serde_yaml::Value::Mapping(hash_data))?; - Ok(StampInputs::new(config_hash)) + Ok(StampInputs::with_package_list( + config_hash, + package_list_hash(resolved.locked_packages), + )) +} + +/// Compute input hash for **rootfs install**. +/// +/// Includes the effective `rootfs.packages` set, `rootfs.overlay`, and the +/// narrowed kernel selection (`package`/`version`/`compile`/`install` only — +/// adding an unrelated `kernel.metadata` field does NOT invalidate). Also +/// includes the `post_install` hook path and its file contents so an +/// in-place script edit invalidates without `--no-stamps`, the SDK feed +/// identity, and a digest of the sysroot's lockfile pins. +pub fn compute_rootfs_input_hash( + config: &serde_yaml::Value, + project_root: &Path, + cli_target_board: Option<&str>, + resolved: &SysrootStampInputs<'_>, +) -> Result { + compute_sysroot_install_input_hash("rootfs", config, project_root, cli_target_board, resolved) } /// Compute input hash for **initramfs install**. /// -/// Same shape as [`compute_rootfs_input_hash`] — narrowed kernel block, -/// `post_install` content hashed alongside its path. +/// Same inputs as [`compute_rootfs_input_hash`], read from the `initramfs` +/// config section. pub fn compute_initramfs_input_hash( config: &serde_yaml::Value, project_root: &Path, cli_target_board: Option<&str>, + resolved: &SysrootStampInputs<'_>, ) -> Result { - let mut hash_data = serde_yaml::Mapping::new(); - - if let Some(initramfs) = config.get("initramfs") { - if let Some(packages) = initramfs.get("packages") { - hash_data.insert( - serde_yaml::Value::String("initramfs.packages".to_string()), - packages.clone(), - ); - } - if let Some(overlay) = initramfs.get("overlay") { - hash_data.insert( - serde_yaml::Value::String("initramfs.overlay".to_string()), - overlay.clone(), - ); - fold_overlay_content_hash( - &mut hash_data, - "initramfs.overlay_content", - overlay, - config, - project_root, - None, - None, - cli_target_board, - )?; - } - if let Some(post_install) = initramfs.get("post_install").and_then(|v| v.as_str()) { - hash_data.insert( - serde_yaml::Value::String("initramfs.post_install".to_string()), - script_hash_value(project_root, post_install), - ); - } - } - - if let Some(kernel) = config.get("kernel") { - hash_data.insert( - serde_yaml::Value::String("kernel".to_string()), - narrow_kernel_for_hash(kernel), - ); - } - - let config_hash = compute_config_hash(&serde_yaml::Value::Mapping(hash_data))?; - Ok(StampInputs::new(config_hash)) + compute_sysroot_install_input_hash( + "initramfs", + config, + project_root, + cli_target_board, + resolved, + ) } /// Compute input hash for **runtime install**. @@ -1862,6 +1930,40 @@ pub fn check_stamp_requirement( mod tests { use super::*; + /// The effective rootfs package set for a default project — what + /// `Config::get_rootfs_packages` returns when `rootfs.packages` is absent. + fn default_rootfs_packages() -> std::collections::HashMap { + std::collections::HashMap::from([( + "avocado-pkg-rootfs".to_string(), + serde_yaml::Value::String("*".to_string()), + )]) + } + + /// Resolved inputs for hash tests: the default package set, no feed + /// identity, no lock pins. Tests exercising a specific resolved input + /// build their own [`SysrootStampInputs`]. + fn test_sysroot_inputs( + packages: &std::collections::HashMap, + ) -> SysrootStampInputs<'_> { + SysrootStampInputs { + packages, + repo_url: None, + repo_release: None, + disable_weak_dependencies: false, + locked_packages: None, + } + } + + /// `compute_rootfs_input_hash`'s config hash for a default package set — + /// the shape most of the hash tests below want, since they vary a config + /// section and assert on the resulting hash. + fn rootfs_config_hash(config: &serde_yaml::Value, project_root: &Path) -> String { + let packages = default_rootfs_packages(); + compute_rootfs_input_hash(config, project_root, None, &test_sysroot_inputs(&packages)) + .unwrap() + .config_hash + } + #[test] fn test_stamp_creation() { let inputs = StampInputs::new("sha256:abc123".to_string()); @@ -3600,12 +3702,8 @@ kernel: "#, ) .unwrap(); - let h_base = compute_rootfs_input_hash(&base, std::path::Path::new("."), None) - .unwrap() - .config_hash; - let h_extra = compute_rootfs_input_hash(&with_metadata, std::path::Path::new("."), None) - .unwrap() - .config_hash; + let h_base = rootfs_config_hash(&base, std::path::Path::new(".")); + let h_extra = rootfs_config_hash(&with_metadata, std::path::Path::new(".")); assert_eq!( h_base, h_extra, "adding unrelated keys under `kernel:` must not invalidate the rootfs install stamp" @@ -3634,12 +3732,8 @@ kernel: "#, ) .unwrap(); - let h_v1 = compute_rootfs_input_hash(&v1, std::path::Path::new("."), None) - .unwrap() - .config_hash; - let h_v2 = compute_rootfs_input_hash(&v2, std::path::Path::new("."), None) - .unwrap() - .config_hash; + let h_v1 = rootfs_config_hash(&v1, std::path::Path::new(".")); + let h_v2 = rootfs_config_hash(&v2, std::path::Path::new(".")); assert_ne!(h_v1, h_v2); } @@ -3658,16 +3752,210 @@ rootfs: "#, ) .unwrap(); - let h1 = compute_rootfs_input_hash(&config, tmp.path(), None) - .unwrap() - .config_hash; + let h1 = rootfs_config_hash(&config, tmp.path()); std::fs::write(&script, b"#!/bin/sh\necho v2\n").unwrap(); - let h2 = compute_rootfs_input_hash(&config, tmp.path(), None) + let h2 = rootfs_config_hash(&config, tmp.path()); + + assert_ne!(h1, h2); + } + + #[test] + fn rootfs_hash_stable_across_absent_and_explicit_default_packages() { + // The install-skip decision rests on this: a project with no `rootfs:` + // section and one that spells out the default meta-package install + // exactly the same thing, so they must hash the same. Hashing the raw + // config node instead of the effective set makes these differ and + // forces a reinstall on any project that writes the default out. + let absent: serde_yaml::Value = serde_yaml::from_str("sdk:\n image: foo\n").unwrap(); + let explicit: serde_yaml::Value = serde_yaml::from_str( + r#" +sdk: + image: foo +rootfs: + packages: + avocado-pkg-rootfs: "*" +"#, + ) + .unwrap(); + + assert_eq!( + rootfs_config_hash(&absent, std::path::Path::new(".")), + rootfs_config_hash(&explicit, std::path::Path::new(".")), + ); + } + + #[test] + fn rootfs_hash_changes_on_added_package() { + let config: serde_yaml::Value = serde_yaml::from_str("sdk:\n image: foo\n").unwrap(); + let root = std::path::Path::new("."); + + let base = default_rootfs_packages(); + let mut with_vim = base.clone(); + with_vim.insert( + "vim".to_string(), + serde_yaml::Value::String("*".to_string()), + ); + + let h_base = compute_rootfs_input_hash(&config, root, None, &test_sysroot_inputs(&base)) + .unwrap() + .config_hash; + let h_vim = compute_rootfs_input_hash(&config, root, None, &test_sysroot_inputs(&with_vim)) .unwrap() .config_hash; - assert_ne!(h1, h2); + assert_ne!(h_base, h_vim); + } + + #[test] + fn rootfs_hash_changes_on_feed_identity_and_weak_deps() { + let config: serde_yaml::Value = serde_yaml::from_str("sdk:\n image: foo\n").unwrap(); + let root = std::path::Path::new("."); + let packages = default_rootfs_packages(); + + let hash_of = |resolved: &SysrootStampInputs<'_>| { + compute_rootfs_input_hash(&config, root, None, resolved) + .unwrap() + .config_hash + }; + + let base = test_sysroot_inputs(&packages); + let h_base = hash_of(&base); + + // A snapshot bump moves repo_release — this is the hook that makes + // `avocado update` land instead of being skipped as up to date. + let h_release = hash_of(&SysrootStampInputs { + repo_release: Some("2026.9.20260727"), + ..test_sysroot_inputs(&packages) + }); + assert_ne!(h_base, h_release, "repo_release must invalidate"); + + let h_url = hash_of(&SysrootStampInputs { + repo_url: Some("https://repo.avocadolinux.org/2026/next"), + ..test_sysroot_inputs(&packages) + }); + assert_ne!(h_base, h_url, "repo_url must invalidate"); + + let h_weak = hash_of(&SysrootStampInputs { + disable_weak_dependencies: true, + ..test_sysroot_inputs(&packages) + }); + assert_ne!( + h_base, h_weak, + "disable_weak_dependencies must invalidate — it changes what dnf pulls" + ); + } + + #[test] + fn rootfs_package_list_hash_tracks_lock_pins() { + let config: serde_yaml::Value = serde_yaml::from_str("sdk:\n image: foo\n").unwrap(); + let root = std::path::Path::new("."); + let packages = default_rootfs_packages(); + + let pinned: std::collections::HashMap = + std::collections::HashMap::from([( + "avocado-pkg-rootfs".to_string(), + "2026.9-r0.0".to_string(), + )]); + let repinned: std::collections::HashMap = std::collections::HashMap::from( + [("avocado-pkg-rootfs".to_string(), "2026.10-r0.0".to_string())], + ); + + let inputs_for = |locked: Option<&std::collections::HashMap>| { + compute_rootfs_input_hash( + &config, + root, + None, + &SysrootStampInputs { + locked_packages: locked, + ..test_sysroot_inputs(&packages) + }, + ) + .unwrap() + }; + + let a = inputs_for(Some(&pinned)); + let b = inputs_for(Some(&repinned)); + let cleared = inputs_for(None); + + // The config side is untouched by a re-pin; only the package list moves. + assert_eq!(a.config_hash, b.config_hash); + assert_ne!(a.package_list_hash, b.package_list_hash); + + // `avocado unlock` clears the section. That has to read as stale, which + // is why an empty pin set hashes to a value rather than to None — + // `is_current` only compares two Some sides. + assert!(cleared.package_list_hash.is_some()); + assert_ne!(a.package_list_hash, cleared.package_list_hash); + + let stamp = Stamp::rootfs_install("qemux86-64", a.clone(), StampOutputs::default()); + assert!(stamp.is_current(&a)); + assert!(!stamp.is_current(&b), "a re-pin must invalidate the stamp"); + assert!( + !stamp.is_current(&cleared), + "avocado unlock must invalidate the stamp" + ); + } + + #[test] + fn rootfs_package_list_hash_is_order_independent() { + // Lock pins come out of a HashMap, so iteration order varies between + // runs. The digest must not. + let a = std::collections::HashMap::from([ + ("alpha".to_string(), "1".to_string()), + ("beta".to_string(), "2".to_string()), + ("gamma".to_string(), "3".to_string()), + ]); + let b = std::collections::HashMap::from([ + ("gamma".to_string(), "3".to_string()), + ("alpha".to_string(), "1".to_string()), + ("beta".to_string(), "2".to_string()), + ]); + + assert_eq!(package_list_hash(Some(&a)), package_list_hash(Some(&b))); + } + + #[test] + fn initramfs_hash_is_independent_of_rootfs_section() { + // The two sysroots install independently; a rootfs-only edit must not + // invalidate the initramfs stamp (and so reinstall it for nothing). + let base: serde_yaml::Value = serde_yaml::from_str( + r#" +initramfs: + packages: + avocado-pkg-initramfs: "*" +"#, + ) + .unwrap(); + let with_rootfs: serde_yaml::Value = serde_yaml::from_str( + r#" +initramfs: + packages: + avocado-pkg-initramfs: "*" +rootfs: + packages: + avocado-pkg-rootfs: "*" + vim: "*" +"#, + ) + .unwrap(); + + let packages = std::collections::HashMap::from([( + "avocado-pkg-initramfs".to_string(), + serde_yaml::Value::String("*".to_string()), + )]); + let root = std::path::Path::new("."); + + let h_base = + compute_initramfs_input_hash(&base, root, None, &test_sysroot_inputs(&packages)) + .unwrap() + .config_hash; + let h_with = + compute_initramfs_input_hash(&with_rootfs, root, None, &test_sysroot_inputs(&packages)) + .unwrap() + .config_hash; + + assert_eq!(h_base, h_with); } #[test] @@ -3694,13 +3982,9 @@ rootfs: .unwrap(); std::env::set_var("STAMP_OVL_TOKEN", "aaa"); - let h1 = compute_rootfs_input_hash(&config, tmp.path(), None) - .unwrap() - .config_hash; + let h1 = rootfs_config_hash(&config, tmp.path()); std::env::set_var("STAMP_OVL_TOKEN", "bbb"); - let h2 = compute_rootfs_input_hash(&config, tmp.path(), None) - .unwrap() - .config_hash; + let h2 = rootfs_config_hash(&config, tmp.path()); // Changing a value referenced by a preprocessed overlay file must // invalidate the rootfs install hash so the image rebuilds. @@ -3826,13 +4110,9 @@ rootfs: ) .unwrap(); - let h1 = compute_rootfs_input_hash(&config, tmp.path(), None) - .unwrap() - .config_hash; + let h1 = rootfs_config_hash(&config, tmp.path()); std::fs::write(tmp.path().join("overlay/etc/f.txt"), "v2-different").unwrap(); - let h2 = compute_rootfs_input_hash(&config, tmp.path(), None) - .unwrap() - .config_hash; + let h2 = rootfs_config_hash(&config, tmp.path()); assert_eq!(h1, h2); }