From 1d16d9d59e5418214fb5f1999ac1d0fe71629fce Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 27 Aug 2026 13:57:15 -0400 Subject: [PATCH 1/2] fix(install): fail a sysroot install that could not finish, instead of reporting success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `avocado install` reports `rootfs install` as succeeded, then `avocado build` refuses to build: ``` ✓ rootfs install (43s) ... [SUCCESS] All components installed successfully! [ERROR] Cannot build runtime 'dev' - dependencies not satisfied [INFO] Missing steps: [INFO] - rootfs install (rootfs/install.stamp) [INFO] To fix: [INFO] avocado rootfs install ``` The suggested fix is the command that just passed, so running it changes nothing and the user is left assuming they misconfigured something. ## Root cause `install_sysroot` withholds the install stamp when the sysroot it produced is not the one the config asked for — the sysroot could not be cleaned first, the installed versions could not be read, or (rootfs, pinned kernel) the kernel sysroot could not be staged. That part is deliberate and correct: a stamp written over a broken sysroot latches it, and `runtime build` is right to refuse a sysroot with no stamp. What was wrong is that it then returned `Ok(())`. The reason went out through `print_warning`, which early-returns whenever `tui_is_active()` — the default for `avocado install` — so the only account of the failure was discarded, the task rendered ✓, and the exit code was 0. The two commands then disagreed with no way to tell which was right. Found on a board whose `kernel-image` package ships no files, so the rootfs sysroot's `/boot` came up empty and kernel staging had nothing to stage. That is a BSP bug, but nothing in the CLI's output pointed at it. ## Change `install_sysroot` returns `Err` for all three cases, carrying what happened, what still landed, and why `build` will keep reporting the step as missing. Failed tasks already render their error, so the reason now reaches the user where the warning did not. `--no-stamps` does not suppress it: these are install failures, and the stamp was only how they surfaced. The pins have to survive that error. They record what is genuinely on disk, and the callers only persisted the lock on `Ok` — dropping them would leave the next run re-resolving the kernel against feed head with no `prev_pinned_kver` to compare against, installing additively on top. So `SysrootInstallParams::pins_recorded` is set once the pins are in the lock, and all three callers persist (or merge) on that rather than on the `Result`: `rootfs install`, `initramfs install`, and the batched `sdk install`. No new failures are introduced. Every case this now reports already blocked `runtime build`; it blocks at the command that can explain itself. ## Verification `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings` clean; `cargo test` 1704 passed, 0 failed. The decision and its wording are covered by five tests on `incomplete_install_reason` / `incomplete_install_error`: a complete install reports nothing, each of the three faults reports, an unclean sysroot outranks the later two signals (it can be the cause of them), the staging error's own text is carried through, and the message names both what landed and the build-time symptom it explains. Not covered by a test: that `install_sysroot` calls them, since reaching that line needs a live SDK container. Reproducing end to end takes a target whose `kernel-image` package installs no `/boot/Image-` — e.g. any machine built with `INITRAMFS_IMAGE_BUNDLE = "1"`, which makes oe-core's `kernel_do_install` skip the image. --- src/commands/initramfs/install.rs | 36 +++-- src/commands/rootfs/install.rs | 223 ++++++++++++++++++++++++------ src/commands/sdk/install.rs | 12 +- 3 files changed, 219 insertions(+), 52 deletions(-) diff --git a/src/commands/initramfs/install.rs b/src/commands/initramfs/install.rs index 1aee3533..81f965ae 100644 --- a/src/commands/initramfs/install.rs +++ b/src/commands/initramfs/install.rs @@ -149,10 +149,10 @@ impl InitramfsInstallCommand { // `runs_on` NFS server and remote mount, and an early return here would // skip it — the next `--runs-on` run picks a fresh nfs_port and stacks // another one on top. Carried into `result` and returned past teardown. - let result = match prefetched_stamp { - Err(e) => Err(e), + let (result, pins_recorded) = match prefetched_stamp { + Err(e) => (Err(e), false), Ok(prefetched_stamp) => { - install_sysroot(&mut SysrootInstallParams { + let mut params = SysrootInstallParams { sysroot_type: SysrootType::Initramfs, config, lock_file: &mut lock_file, @@ -173,17 +173,37 @@ impl InitramfsInstallCommand { parsed: Some(&composed.merged_value), prefetched_stamp, tui_context: None, - }) - .await + pins_recorded: false, + }; + let outcome = install_sysroot(&mut params).await; + // Copied out before `params` drops: it holds the `&mut` on + // `lock_file`, which the save below needs back. + let pins_recorded = params.pins_recorded; + (outcome, pins_recorded) } }; // Persist the lockfile the install updated — `install_sysroot` leaves // saving to its caller. Folded into `result` rather than `?` so a save // failure still reaches the teardown below. - let result = match result { - Ok(()) => lock_file.save(src_dir), - Err(e) => Err(e), + // + // Also saved when the install failed *after* recording pins: they + // describe the packages that actually landed, and dropping them makes + // the next run re-resolve the kernel against feed head with no + // `prev_pinned_kver` to compare against. The install error is the one + // returned — it is what the user has to act on. + let result = match (result, pins_recorded) { + (Ok(()), _) => lock_file.save(src_dir), + (Err(e), true) => { + if let Err(save_err) = lock_file.save(src_dir) { + print_error( + &format!("Failed to save lock file: {save_err}"), + OutputLevel::Normal, + ); + } + Err(e) + } + (Err(e), false) => Err(e), }; if let Some(ref mut context) = runs_on_context { diff --git a/src/commands/rootfs/install.rs b/src/commands/rootfs/install.rs index 52defa9a..0c5cf9af 100644 --- a/src/commands/rootfs/install.rs +++ b/src/commands/rootfs/install.rs @@ -96,6 +96,14 @@ pub struct SysrootInstallParams<'a> { pub prefetched_stamp: Option, /// TUI context for output capture (if TUI is active). pub tui_context: Option, + /// Set by [`install_sysroot`] once the packages have landed and it has + /// recorded this sysroot's pins into `lock_file`. It stays set when the + /// install then fails, because the pins describe what is genuinely on + /// disk: a caller that drops them leaves the next run to re-resolve the + /// kernel against feed head with no `prev_pinned_kver` to compare against + /// and install additively on top. Callers must persist (or merge) the lock + /// whenever this is true, whatever the returned `Result` says. + pub pins_recorded: bool, } impl SysrootInstallParams<'_> { @@ -268,6 +276,53 @@ fi Ok(()) } +/// Why an install that placed its packages still must not report success. +/// +/// `None` means every claim the install stamp makes holds: the packages are on +/// disk, the lock records their versions, and (for rootfs with a pinned +/// kernel) the kernel sysroot is staged. `Some(reason)` means one of them does +/// not, so no stamp is written and `runtime build` will refuse to build. +/// +/// Split out of [`install_sysroot`] to keep the wording under test: it is the +/// only place a user learns why an install that looked fine left the build +/// unable to proceed. +fn incomplete_install_reason( + install_is_clean: bool, + versions_recorded: bool, + kernel_staging_error: Option<&str>, +) -> Option { + if !install_is_clean { + // Checked first: a sysroot that could not be cleaned may be carrying + // packages from a previous config, which makes the other two signals + // describe the wrong tree. + Some("the sysroot could not be cleaned first, so stale contents may remain".to_string()) + } else if !versions_recorded { + Some("the installed package versions could not be read".to_string()) + } else { + kernel_staging_error.map(|detail| { + format!( + "the kernel sysroot could not be staged ({detail}). The rootfs sysroot's /boot \ + has no kernel image for this kernel version, so the packages that were \ + installed do not include a bootable kernel" + ) + }) + } +} + +/// The error an install that placed its packages but could not finish reports. +/// +/// Kept next to [`incomplete_install_reason`] and under test because this text +/// is the whole remedy path: without it the user sees a passing `install` and a +/// `build` that names ` install` as missing, whose printed fix is the +/// command that just passed. +fn incomplete_install_error(label: &str, reason: &str) -> anyhow::Error { + anyhow::anyhow!( + "Installed the {label} sysroot's packages, but the install did not complete: {reason}. \ + No install stamp was recorded, so `avocado build` will keep reporting `{label} install` \ + as missing until this succeeds." + ) +} + /// 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 @@ -1050,7 +1105,7 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ // stamp anyway latches the broken state: every later run reports "up to // date" and the user is wedged until they guess --no-stamps. let versions_recorded = !installed_versions.is_empty(); - let mut kernel_staging_ok = true; + let mut kernel_staging_error: Option = None; let install_is_clean = clean_ok; if versions_recorded { @@ -1059,6 +1114,10 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ ¶ms.sysroot_type, installed_versions, ); + // From here the lock describes what is on disk. Everything below + // can still fail the install, so tell the caller to keep the pins + // regardless of what this function returns. + params.pins_recorded = true; if params.verbose { print_info( &format!("Updated lock file with {label} package versions."), @@ -1107,14 +1166,7 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ ) .await { - print_error( - &format!( - "Kernel sysroot staging failed: {e}. \ - provision may fall back to reading the Image from the rootfs sysroot." - ), - OutputLevel::Normal, - ); - kernel_staging_ok = false; + kernel_staging_error = Some(e.to_string()); } } } @@ -1125,32 +1177,38 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ // 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. - let stamp_is_trustworthy = versions_recorded && kernel_staging_ok && install_is_clean; - - if !params.no_stamps && !stamp_is_trustworthy { - print_warning( - &format!( - "Not recording an install stamp for {label}: {}. \ - The next run will reinstall rather than report it up to date.", - if !install_is_clean { - "the sysroot could not be cleaned first, so stale contents may remain" - } else if !versions_recorded { - "the installed package versions could not be read" - } else { - "kernel sysroot staging failed" - } - ), - OutputLevel::Normal, - ); + // Fail rather than return a success the stamp contradicts. + // + // Each of these means the sysroot on disk is not the one the config + // asked for, so no stamp is written -- and `runtime build` refuses to + // build without one. Reporting the step as succeeded left the two + // commands disagreeing: a green `install` followed by `build` naming + // `rootfs install` as missing, with `avocado rootfs install` as the + // suggested remedy -- the command that had just "succeeded". Nothing in + // that loop names the actual fault, so it reads as user error, and the + // reason (a `print_warning`) is invisible whenever the TUI is active, + // which is the default for `avocado install`. + // + // Failing here surfaces the reason through the failed-task rendering + // and stops the loop at the command that can still explain itself. + // `--no-stamps` does not suppress it: these are install failures, and + // the stamp is only how they became visible. + if let Some(reason) = incomplete_install_reason( + install_is_clean, + versions_recorded, + kernel_staging_error.as_deref(), + ) { + return Err(incomplete_install_error(label, &reason)); } - // Deliberately not `?` anywhere below. The pins recorded above are only - // persisted by the caller when this function returns Ok, so propagating a - // stamp-write failure would discard the pins for an install that already - // landed — the next run would re-resolve the kernel against feed head with - // no prev_pinned_kver to compare against and install additively on top. - // A missing stamp is the benign outcome: the next run reinstalls. - if !params.no_stamps && stamp_is_trustworthy { + // Deliberately not `?`. Unlike the checks above -- which report a + // sysroot that is not what the config asked for -- a stamp-write + // failure leaves a correct sysroot that simply is not recorded, and the + // next run reinstalls. Failing here would add nothing and cost the + // teardown the callers run on the way out. + // + // Reaching this line means `incomplete_install_reason` returned None. + if !params.no_stamps { if let Err(e) = write_install_stamp(params, &packages, label).await { print_warning( &format!( @@ -1302,10 +1360,10 @@ impl RootfsInstallCommand { // `runs_on` NFS server and remote mount, and an early return here would // skip it — the next `--runs-on` run picks a fresh nfs_port and stacks // another one on top. Carried into `result` and returned past teardown. - let result = match prefetched_stamp { - Err(e) => Err(e), + let (result, pins_recorded) = match prefetched_stamp { + Err(e) => (Err(e), false), Ok(prefetched_stamp) => { - install_sysroot(&mut SysrootInstallParams { + let mut params = SysrootInstallParams { sysroot_type: SysrootType::Rootfs, config, lock_file: &mut lock_file, @@ -1326,8 +1384,13 @@ impl RootfsInstallCommand { parsed: Some(&composed.merged_value), prefetched_stamp, tui_context: None, - }) - .await + pins_recorded: false, + }; + let outcome = install_sysroot(&mut params).await; + // Copied out before `params` drops: it holds the `&mut` on + // `lock_file`, which the save below needs back. + let pins_recorded = params.pins_recorded; + (outcome, pins_recorded) } }; @@ -1335,9 +1398,24 @@ impl RootfsInstallCommand { // longer saves for itself — under `avocado sdk install` it runs on a // clone that the caller merges and saves once. Folded into `result` // rather than `?` so a save failure still reaches the teardown below. - let result = match result { - Ok(()) => lock_file.save(src_dir), - Err(e) => Err(e), + // + // Also saved when the install failed *after* recording pins: they + // describe the packages that actually landed, and dropping them makes + // the next run re-resolve the kernel against feed head with no + // `prev_pinned_kver` to compare against. The install error is the one + // returned — it is what the user has to act on. + let result = match (result, pins_recorded) { + (Ok(()), _) => lock_file.save(src_dir), + (Err(e), true) => { + if let Err(save_err) = lock_file.save(src_dir) { + print_error( + &format!("Failed to save lock file: {save_err}"), + OutputLevel::Normal, + ); + } + Err(e) + } + (Err(e), false) => Err(e), }; // Always teardown runs_on context @@ -1356,10 +1434,71 @@ impl RootfsInstallCommand { #[cfg(test)] mod tests { - use super::{build_overlay_script, detect_sysroot_package_removals, parse_probe_output}; + use super::{ + build_overlay_script, detect_sysroot_package_removals, incomplete_install_error, + incomplete_install_reason, parse_probe_output, + }; use crate::utils::lockfile::{LockFile, SysrootType}; use std::collections::{HashMap, HashSet}; + // A sysroot install that places its packages but cannot finish the job must + // not report success: no stamp is written for it, and `runtime build` + // refuses to build without one. Returning Ok here is what produced a green + // `avocado install` followed by `avocado build` reporting `rootfs install` + // as missing and suggesting the very command that had just passed. + #[test] + fn a_complete_install_has_no_reason_to_fail() { + assert_eq!(incomplete_install_reason(true, true, None), None); + } + + #[test] + fn kernel_staging_failure_is_reported_with_its_cause() { + let reason = incomplete_install_reason( + true, + true, + Some("Failed to stage kernel sysroot for kernel-version '6.18.37'"), + ) + .expect("staging failure must not be silent"); + // The underlying error is carried, not swallowed — it is the only part + // that names which kernel version had no image. + assert!(reason.contains("6.18.37"), "{reason}"); + assert!(reason.contains("/boot"), "{reason}"); + } + + #[test] + fn an_unreadable_package_list_and_an_unclean_sysroot_each_fail() { + assert!(incomplete_install_reason(true, false, None) + .expect("unread versions must not be silent") + .contains("package versions")); + assert!(incomplete_install_reason(false, true, None) + .expect("an unclean sysroot must not be silent") + .contains("cleaned")); + } + + #[test] + fn an_unclean_sysroot_outranks_the_later_signals() { + // Order matters: a sysroot that could not be cleaned may hold packages + // from a previous config, which makes the other two signals describe + // the wrong tree. Report the cause, not a symptom of it. + let reason = incomplete_install_reason(false, false, Some("staging blew up")) + .expect("must not be silent"); + assert!(reason.contains("cleaned"), "{reason}"); + assert!(!reason.contains("staging blew up"), "{reason}"); + } + + #[test] + fn the_failure_names_the_sysroot_and_the_remedy_loop_it_breaks() { + let msg = incomplete_install_error("rootfs", "the kernel sysroot could not be staged") + .to_string(); + assert!(msg.contains("rootfs"), "{msg}"); + // Says the packages did land — otherwise it reads as "nothing worked". + assert!(msg.contains("Installed the rootfs sysroot"), "{msg}"); + // And ties the install-time fault to the build-time symptom, so the + // user does not go looking for a mistake of their own. + assert!(msg.contains("No install stamp"), "{msg}"); + assert!(msg.contains("avocado build"), "{msg}"); + } + const KVER: &str = "6.8.12-l4t-r39.2.0-1021.21"; const TARGET: &str = "jetson-agx-thor"; diff --git a/src/commands/sdk/install.rs b/src/commands/sdk/install.rs index d9993590..f6d2464a 100644 --- a/src/commands/sdk/install.rs +++ b/src/commands/sdk/install.rs @@ -550,6 +550,7 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ parsed: Some(&composed.merged_value), prefetched_stamp: rootfs_stamp, tui_context: rootfs_tui, + pins_recorded: false, }; let mut initramfs_params = SysrootInstallParams { sysroot_type: SysrootType::Initramfs, @@ -572,6 +573,7 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ parsed: Some(&composed.merged_value), prefetched_stamp: initramfs_stamp, tui_context: initramfs_tui, + pins_recorded: false, }; // Build the target-dev future (or a no-op if not needed) @@ -666,6 +668,12 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ // Merge lock file changes back into a single lock file for saving let mut final_lock = lock_file.clone(); + // A sysroot install that failed *after* recording its pins still has + // pins worth merging — they describe the packages that landed. See + // `SysrootInstallParams::pins_recorded`. + let rootfs_landed = rootfs_result.is_ok() || rootfs_params.pins_recorded; + let initramfs_landed = initramfs_result.is_ok() || initramfs_params.pins_recorded; + // Merge SDK packages lock if sdk_pkg_result.is_ok() { if let Some(target_locks) = sdk_pkg_lock.targets.get(target) { @@ -678,7 +686,7 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ // `kernels` entries staged from the rootfs install (Phase 2c) — they // live on the clone too and would otherwise be dropped on the merge // floor. - if rootfs_result.is_ok() { + if rootfs_landed { if let Some(target_locks) = rootfs_lock.targets.get(target) { let entry = final_lock.targets.entry(target.to_string()).or_default(); entry.rootfs = target_locks.rootfs.clone(); @@ -706,7 +714,7 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ } } // Merge initramfs lock. - if initramfs_result.is_ok() { + if initramfs_landed { if let Some(target_locks) = initramfs_lock.targets.get(target) { let entry = final_lock.targets.entry(target.to_string()).or_default(); entry.initramfs = target_locks.initramfs.clone(); From 5857367cea1e882b68281c9f9f2a64e72585d375 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Fri, 28 Aug 2026 07:31:44 -0400 Subject: [PATCH 2/2] fix(install): keep the lock whenever packages landed, surface the reason under --json - pins_recorded is set the moment the dnf run succeeds, not after the version query: from then on the lock's kernel pin and cleared sections describe the disk, and dropping them when the version query fails left the lock saying kernel A while the sysroot held B, so the next run installed A additively on top. Doc comment rewritten around that event. - with_tui_status! records the error on the task before marking it Failed, the same shape as the scheduler executor, so --json emits step_error with the reason instead of a bare failed step; the lock-save failure notices go to stderr through print_warning_stderr, which survives --json and TUI. - The kernel-staging reason says what to verify rather than diagnosing a cause the staging error may not have. - tui_is_active's doc no longer claims errors always print. CHANGELOG entry under Fixed for the three cases that now exit non-zero. --- CHANGELOG.md | 9 +++++++ src/commands/initramfs/install.rs | 9 ++++--- src/commands/rootfs/install.rs | 39 +++++++++++++++++-------------- src/commands/sdk/install.rs | 15 ++++++++---- src/utils/output.rs | 4 +++- 5 files changed, 48 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a200c2bb..39ac9e22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -197,6 +197,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 that collapse is what let an unparseable version fall through as a pass. ### Fixed +- **`avocado install` / `rootfs install` / `initramfs install` exit non-zero + when a sysroot install could not finish.** Three cases previously reported + success and wrote a current install stamp: the sysroot could not be cleaned + before a kernel change, the installed package versions could not be read + back, or the kernel sysroot could not be staged. Each now fails with the + reason (also emitted under `--json`) and leaves no stamp, so the next run + repairs instead of reporting "up to date". The lock is still saved whenever + packages landed, so a failed run cannot leave the kernel pin out of step + with the disk. - **`source_date_epoch` is honored outside extension images.** The key was read only by `ext image`, which exports it inside its own script. The rootfs build script has always passed `-T "${SOURCE_DATE_EPOCH:-0}"` to diff --git a/src/commands/initramfs/install.rs b/src/commands/initramfs/install.rs index 81f965ae..1f7c6b5c 100644 --- a/src/commands/initramfs/install.rs +++ b/src/commands/initramfs/install.rs @@ -7,7 +7,7 @@ use crate::utils::{ config::{ComposedConfig, Config}, container::{RunConfig, SdkContainer}, lockfile::{LockFile, SysrootType}, - output::{print_error, OutputLevel}, + output::{print_error, print_warning_stderr, OutputLevel}, runs_on::RunsOnContext, target::validate_and_log_target, }; @@ -196,10 +196,9 @@ impl InitramfsInstallCommand { (Ok(()), _) => lock_file.save(src_dir), (Err(e), true) => { if let Err(save_err) = lock_file.save(src_dir) { - print_error( - &format!("Failed to save lock file: {save_err}"), - OutputLevel::Normal, - ); + // stderr, not print_error: under --json print_error is + // suppressed and this notice must survive. + print_warning_stderr(&format!("Failed to save lock file: {save_err}")); } Err(e) } diff --git a/src/commands/rootfs/install.rs b/src/commands/rootfs/install.rs index 0c5cf9af..8bfc9060 100644 --- a/src/commands/rootfs/install.rs +++ b/src/commands/rootfs/install.rs @@ -53,7 +53,9 @@ use crate::utils::{ kernel_resolver::{off_kernel_dnf_excludes, resolve_and_pin_kernel_version, ResolveParams}, kernel_version::substitute_kernel_version, lockfile::{build_package_spec_with_lock, LockFile, SysrootType}, - output::{print_error, print_info, print_success, print_warning, OutputLevel}, + output::{ + print_error, print_info, print_success, print_warning, print_warning_stderr, OutputLevel, + }, prerequisites::read_stamps_batch, runs_on::RunsOnContext, stamps::{ @@ -96,12 +98,14 @@ pub struct SysrootInstallParams<'a> { pub prefetched_stamp: Option, /// TUI context for output capture (if TUI is active). pub tui_context: Option, - /// Set by [`install_sysroot`] once the packages have landed and it has - /// recorded this sysroot's pins into `lock_file`. It stays set when the - /// install then fails, because the pins describe what is genuinely on - /// disk: a caller that drops them leaves the next run to re-resolve the - /// kernel against feed head with no `prev_pinned_kver` to compare against - /// and install additively on top. Callers must persist (or merge) the lock + /// Set by [`install_sysroot`] the moment the package install has landed on + /// disk. From then on `lock_file` describes that disk - the kernel pin + /// chosen for it, the sections cleared for a clean reinstall, and (when + /// they could be read) the installed package versions - so it must be + /// persisted whatever happens next. It stays set when the install then + /// fails: a caller that drops the lock leaves disk holding kernel B while + /// the lock still says A, and the next run honors A's pin and installs + /// additively on top of B. Callers must persist (or merge) the lock /// whenever this is true, whatever the returned `Result` says. pub pins_recorded: bool, } @@ -301,9 +305,9 @@ fn incomplete_install_reason( } else { kernel_staging_error.map(|detail| { format!( - "the kernel sysroot could not be staged ({detail}). The rootfs sysroot's /boot \ - has no kernel image for this kernel version, so the packages that were \ - installed do not include a bootable kernel" + "the kernel sysroot could not be staged ({detail}). Verify that the rootfs \ + sysroot's /boot contains a kernel image for this kernel version; without one \ + the packages that were installed do not include a bootable kernel" ) }) } @@ -1080,6 +1084,10 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ if success { print_success(&format!("Installed {label} sysroot."), OutputLevel::Normal); + // Packages are on disk: the lock's kernel pin and cleared sections now + // describe it, whether or not the version query below succeeds. Tell + // the caller before anything else can fail. + params.pins_recorded = true; // Query installed versions for ALL config packages and update lock file let installed_versions = params @@ -1114,10 +1122,6 @@ $DNF_SDK_HOST $DNF_SDK_TARGET_REPO_CONF \ ¶ms.sysroot_type, installed_versions, ); - // From here the lock describes what is on disk. Everything below - // can still fail the install, so tell the caller to keep the pins - // regardless of what this function returns. - params.pins_recorded = true; if params.verbose { print_info( &format!("Updated lock file with {label} package versions."), @@ -1408,10 +1412,9 @@ impl RootfsInstallCommand { (Ok(()), _) => lock_file.save(src_dir), (Err(e), true) => { if let Err(save_err) = lock_file.save(src_dir) { - print_error( - &format!("Failed to save lock file: {save_err}"), - OutputLevel::Normal, - ); + // stderr, not print_error: under --json print_error is + // suppressed and this notice must survive. + print_warning_stderr(&format!("Failed to save lock file: {save_err}")); } Err(e) } diff --git a/src/commands/sdk/install.rs b/src/commands/sdk/install.rs index f6d2464a..3f2a7983 100644 --- a/src/commands/sdk/install.rs +++ b/src/commands/sdk/install.rs @@ -628,10 +628,17 @@ $DNF_SDK_HOST $DNF_NO_SCRIPTS $DNF_SDK_TARGET_REPO_CONF \ async { let result = $fut.await; if let Some(r) = crate::utils::tui::get_active_renderer() { - if result.is_ok() { - r.set_status(&$task_id, TaskStatus::Success); - } else { - r.set_status(&$task_id, TaskStatus::Failed); + match &result { + Ok(_) => r.set_status(&$task_id, TaskStatus::Success), + Err(e) => { + // Same shape as the scheduler executor: the + // message must be set for --json to emit a + // step_error event; a bare Failed status + // carries no reason and print_error is + // suppressed in that mode. + r.set_error(&$task_id, format!("{e:#}")); + r.set_status(&$task_id, TaskStatus::Failed); + } } } result diff --git a/src/utils/output.rs b/src/utils/output.rs index 2c69c58e..4cc62553 100644 --- a/src/utils/output.rs +++ b/src/utils/output.rs @@ -17,7 +17,9 @@ pub enum OutputLevel { /// When a TUI renderer is active, info/success/warning/plain messages are /// suppressed — the TUI task status lines are the progress indicator. -/// Errors always print (via `print_above`) so they're visible immediately. +/// `print_error` is suppressed too (the error is reported through task state +/// at shutdown); a notice that must survive both modes goes through +/// `print_warning_stderr` / `print_info_stderr`. /// /// Also returns true when JSON output mode is active: prose would interleave /// with the NDJSON event stream and confuse consumers, so we treat JSON