diff --git a/src/build/mod.rs b/src/build/mod.rs index 730985e..b91920c 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -169,6 +169,13 @@ fn retry_after_revendor( opts: &SourceBuildOptions, original: Box, ) -> Result> { + // The offer is only made when someone can answer it: headless prompters + // answer with the default (false) without the error being logged here — + // the caller logs the returned error itself, exactly once. + if !prompter.interactive() { + view.finish_failure(); + return Err(original); + } log::error!("{original}"); if !prompter .confirm( @@ -301,7 +308,14 @@ pub fn run_source_build( package: &entry.source, version: &entry.version.full(), target: &entry.distribution, + display: format!( + "Building source package {} ({}) for {}", + entry.source, + entry.version.full(), + entry.distribution + ), source_only: true, + tee_log: true, }); let source_display = entry.source.clone(); @@ -389,9 +403,9 @@ pub fn run_source_build( }; let report = crate::debian::deps::check_build_depends(&ctrl, &check_opts)?; if !report.is_ok() { - // The diagnostics travel inside the error (its Display carries - // report.message()); main renders it and maps the type to exit - // status 3, like dpkg-buildpackage. + // The typed error carries the diagnostics (UnmetReport is + // public); the caller renders them and maps the type to exit + // status 3, like dpkg-buildpackage does. return Err(Box::new(crate::debian::deps::UnmetBuildDependencies( report, ))); diff --git a/src/changelog.rs b/src/changelog.rs index ee17b58..207eea2 100644 --- a/src/changelog.rs +++ b/src/changelog.rs @@ -5,13 +5,30 @@ use std::fs::File; use std::io::{Read, Write}; use std::path::Path; +/// Outcome of a successful [`generate_entry`] call: everything the CLI +/// renders for the user, and everything a library consumer needs to chain +/// further steps. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GeneratedEntry { + /// Source package name from the previous changelog entry. + pub package: String, + /// Version the changelog carried before the new entry was prepended. + pub previous_version: String, + /// Version of the freshly added entry. + pub new_version: String, + /// Distribution series the new entry targets. + pub series: String, + /// The changelog file that was updated. + pub path: std::path::PathBuf, +} + /// Automatically generate a changelog entry from a commit history and previous changelog pub fn generate_entry( changelog_file: &str, cwd: Option<&Path>, user_version: Option<&str>, target_series: Option<&str>, -) -> Result<(), Box> { +) -> Result> { let changelog_path = if let Some(path) = cwd { path.join(changelog_file) } else { @@ -19,8 +36,7 @@ pub fn generate_entry( }; // Parse existing changelog to get current (old) version - let (package, old_version, series) = parse_changelog_header(&changelog_path)?; - log::info!("Found package: {}, version: {}", package, old_version); + let (package, old_version, current_series) = parse_changelog_header(&changelog_path)?; // Open git repo, and find commits since last version tag let repo_path = if let Some(path) = cwd { @@ -44,7 +60,7 @@ pub fn generate_entry( }; let (maintainer_name, maintainer_email) = get_maintainer_info()?; - let series = target_series.unwrap_or(&series).to_string(); + let series = target_series.unwrap_or(¤t_series).to_string(); let new_entry = format_entry( &package, &new_version, @@ -56,9 +72,13 @@ pub fn generate_entry( prepend_to_file(&changelog_path, &new_entry)?; - log::info!("Added new changelog entry to {}", changelog_path.display()); - - Ok(()) + Ok(GeneratedEntry { + package, + previous_version: old_version, + new_version, + series, + path: changelog_path, + }) } /// Compute the next (most probable) version number of a package, from old version and @@ -195,17 +215,16 @@ pub async fn series_candidates(changelog_path: &Path) -> Option { - match crate::distro_info::get_ordered_series_name(&dist).await { - Ok(options) if !options.is_empty() => Some(SeriesCandidates::Choose { - options, - default: current.clone(), - fallback: current, - }), - // An empty list offers nothing to choose from - _ => Some(SeriesCandidates::Keep(current)), - } - } + Ok(dist) => match crate::distro_info::get_ordered_series_name(&dist).await { + // Even an empty list goes through the selector: its + // fallback prints and takes the default, like it always has + Ok(options) => Some(SeriesCandidates::Choose { + options, + default: current.clone(), + fallback: current, + }), + Err(_) => Some(SeriesCandidates::Keep(current)), + }, Err(_) => Some(SeriesCandidates::Keep(current)), } } diff --git a/src/deb/mod.rs b/src/deb/mod.rs index 2870807..35b842c 100644 --- a/src/deb/mod.rs +++ b/src/deb/mod.rs @@ -225,7 +225,9 @@ async fn build_binary_package_impl( package: &package, version: &version, target: &format!("{series}/{arch}"), + display: format!("Building {package} ({version}) for {series}/{arch}"), source_only: false, + tee_log: true, }); // Create an ephemeral unshare context for all Local builds. It is kept in diff --git a/src/debian/deps.rs b/src/debian/deps.rs index 097141f..71602a9 100644 --- a/src/debian/deps.rs +++ b/src/debian/deps.rs @@ -890,11 +890,7 @@ pub struct UnmetBuildDependencies(pub UnmetReport); impl std::fmt::Display for UnmetBuildDependencies { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "build dependencies/conflicts unsatisfied; aborting\n{}", - self.0.message() - ) + write!(f, "build dependencies/conflicts unsatisfied; aborting") } } diff --git a/src/main.rs b/src/main.rs index 594625f..0f94e24 100644 --- a/src/main.rs +++ b/src/main.rs @@ -418,15 +418,23 @@ fn main() { } }; - if let Err(e) = generate_entry( + let entry = match generate_entry( "debian/changelog", Some(&cwd), version, target_series.as_deref(), ) { - error!("{}", e); - std::process::exit(1); - } + Ok(entry) => entry, + Err(e) => { + error!("{}", e); + std::process::exit(1); + } + }; + println!( + "Found package: {}, version: {}", + entry.package, entry.previous_version + ); + println!("Added new changelog entry to {}", entry.path.display()); let editor = match std::env::var("EDITOR") { Ok(e) => e, @@ -461,8 +469,15 @@ fn main() { // Live build view, unless --verbose (DebUi additionally disables // itself when stdout is not a terminal) let quiet = pkh::report::Quiet; - let live = pkh::ui::deb::DebUi::new(&multi); - let view: &dyn pkh::report::BuildView = if verbose { &quiet } else { &live }; + let live = if verbose { + None + } else { + Some(pkh::ui::deb::DebUi::new(&multi)) + }; + let view: &dyn pkh::report::BuildView = live + .as_ref() + .map(|v| v as &dyn pkh::report::BuildView) + .unwrap_or(&quiet); let prompter = pkh::ui::prompt::TerminalPrompter; let orig_source = match sub_matches.get_one::("orig").map(String::as_str) { @@ -490,12 +505,19 @@ fn main() { } } if output.signed { - info!("Package built and signed successfully!"); + println!("Package built and signed successfully!"); } else { - info!("Package built successfully (unsigned)."); + println!("Package built successfully (unsigned)."); } } Err(e) => { + // The unmet-dependency diagnostics first, then the + // summary: the exact rendering the flow used to do. + if let Some(unmet) = + e.downcast_ref::() + { + eprintln!("{}", unmet.0.message()); + } error!("{}", e); // Unmet build dependencies/conflicts exit with status 3, // like dpkg-buildpackage does. @@ -581,8 +603,15 @@ fn main() { // Live build view, unless --verbose (DebUi additionally disables // itself when stdout is not a terminal) let quiet = pkh::report::Quiet; - let live = pkh::ui::deb::DebUi::new(&multi); - let view: &dyn pkh::report::BuildView = if verbose { &quiet } else { &live }; + let live = if verbose { + None + } else { + Some(pkh::ui::deb::DebUi::new(&multi)) + }; + let view: &dyn pkh::report::BuildView = live + .as_ref() + .map(|v| v as &dyn pkh::report::BuildView) + .unwrap_or(&quiet); let result = rt.block_on(async { pkh::deb::build_binary_package(pkh::deb::DebBuildOptions { diff --git a/src/new/questions.rs b/src/new/questions.rs index 00a91f9..6dc14fa 100644 --- a/src/new/questions.rs +++ b/src/new/questions.rs @@ -541,7 +541,7 @@ async fn run_wizard( // Summary screen + final confirmation: Ctrl+C or 'n' abort with // nothing written (generation is all-or-nothing later anyway). - log::info!("{}", summary_text(&opts, toolchain_pin.as_deref())); + prompter.present(&summary_text(&opts, toolchain_pin.as_deref())); if !prompter.confirm("Generate?", true)? { return Err("Aborted: nothing was written to disk.".into()); } @@ -575,6 +575,9 @@ pub async fn offer_verification( }; if outcome.vendoring_failed { + // Set apart from the surrounding success output by blank lines: a + // single warning between two success lines is easy to miss. + prompter.present(""); log::warn!( "The Cargo dependencies could NOT be vendored: this package will \ not build until the vendoring is completed by hand:\n\ @@ -582,6 +585,7 @@ pub async fn offer_verification( \x20 2. add the printed source replacement to .cargo/config.toml, \ plus `[net] offline = true`" ); + prompter.present(""); } if no_verify || !prompter.interactive() { diff --git a/src/package_info.rs b/src/package_info.rs index acf0e7d..cb9b07f 100644 --- a/src/package_info.rs +++ b/src/package_info.rs @@ -976,8 +976,6 @@ mod tests { } } - use super::*; - /// Serve canned byte responses on a local port, one per connection (the /// last response repeats), and return the base URL /// diff --git a/src/put/mod.rs b/src/put/mod.rs index 40b033d..2676887 100644 --- a/src/put/mod.rs +++ b/src/put/mod.rs @@ -26,7 +26,7 @@ use crate::debian::checksums::FileChecksums; use crate::debian::control::ControlInfo; use crate::debian::version::DebianVersion; use crate::launchpad; -use crate::report::{BuildView, Prompter}; +use crate::report::{BuildTarget, BuildView, Prompter}; /// Everything `put` needs to run. pub struct PutOptions<'a> { @@ -89,10 +89,20 @@ async fn put_impl(opts: &PutOptions<'_>) -> Result<(), Box Result<(), String>; /// Identity of the build whose events follow, as announced through /// [`BuildView::target`]. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct BuildTarget<'a> { /// Source package name (e.g. `hello`). pub package: &'a str, /// Full version being built (e.g. `2.10-3`). pub version: &'a str, /// What the build targets: a distribution series, optionally with an - /// architecture (`noble`, `sid`, `noble/arm64`). + /// architecture (`noble`, `sid`, `noble/arm64`), or the upload target + /// label. pub target: &'a str, - /// Whether this is a source-only build (producing a `.dsc`) as opposed - /// to a binary build (producing `.deb` files). + /// Ready-to-render status line for display adapters, composed by the + /// flow (e.g. "Building source package hello (2.10-3) for unstable", + /// "Uploading hello (2.10-3) to ppa:user/ppa"): the wording is the + /// flow's, adapters render it verbatim. + pub display: String, + /// Whether this is a source-only build (producing a `.dsc`); names the + /// terminal adapter's tee log (`build-*.log` vs `deb-*.log`). pub source_only: bool, + /// Whether the view should tee raw subprocess output to its log file. + /// Flows without subprocess output (uploads) pass `false` and create + /// no log file. + pub tee_log: bool, } /// Observer of a running build: target identification, phases, status @@ -156,6 +166,15 @@ pub trait Prompter: Send + Sync { fn accept_host_key(&self, _host: &str, _key_type: &str, _fingerprint: &str) -> bool { false } + + /// Present information to the user outside of any question: the + /// scaffold wizard's summary screen, prominent notices around a + /// warning. Terminal implementations print the text as-is (unstyled, + /// on stdout); server implementations forward it as a display event. + /// Implementations that cannot show anything drop it — flows only + /// present context that is also available structurally (returned data, + /// log records). + fn present(&self, _text: &str) {} } /// Inert view and prompter: drops every event and answers every question diff --git a/src/ui/deb.rs b/src/ui/deb.rs index ee8501b..f01d998 100644 --- a/src/ui/deb.rs +++ b/src/ui/deb.rs @@ -83,8 +83,7 @@ impl DebUi { let pb = multi.add(ProgressBar::new(0)); pb.enable_steady_tick(Duration::from_millis(80)); pb.set_style(spinner_style()); - // Neutral identity until a `target` event names the build - pb.set_prefix("pkh"); + pb.set_prefix("Building package"); pb.set_message("(starting…)"); pb } else { @@ -270,24 +269,18 @@ impl DebUi { /// widget. impl crate::report::BuildView for DebUi { fn target(&self, target: BuildTarget<'_>) { - let kind = if target.source_only { - "source package " - } else { - "" - }; if self.shared.enabled { - self.shared.top.set_prefix(format!( - "Building {kind}{} ({}) for {}", - target.package, target.version, target.target - )); + self.shared.top.set_prefix(target.display.clone()); + } + if target.tee_log { + let kind = if target.source_only { "build" } else { "deb" }; + self.open_log( + kind, + target.package, + target.version, + &format!("for {}", target.target), + ); } - let log_kind = if target.source_only { "build" } else { "deb" }; - self.open_log( - log_kind, - target.package, - target.version, - &format!("for {}", target.target), - ); } fn phase(&self, name: &str, classifier: Box) { diff --git a/src/ui/prompt.rs b/src/ui/prompt.rs index 0cf9c94..9d15e5e 100644 --- a/src/ui/prompt.rs +++ b/src/ui/prompt.rs @@ -167,6 +167,10 @@ impl crate::report::Prompter for TerminalPrompter { println!("{key_type} key fingerprint is {fingerprint}."); confirm("Accept and store this host key?", false).unwrap_or(false) } + + fn present(&self, text: &str) { + println!("{text}"); + } } /// Run `prompt` with the terminal in raw mode, always restoring it