From f6ca9f671c82aae8e799c73619fcec4247427483 Mon Sep 17 00:00:00 2001 From: Vasilito Date: Sat, 25 Apr 2026 17:38:18 +0100 Subject: [PATCH] Fix build system: async TUI log writer, error messages, wget retries --- mk/disk.mk | 16 ++++---- mk/prefix.mk | 21 ++++++---- src/bin/repo.rs | 102 +++++++++++++++++++++++++++++++++--------------- 3 files changed, 92 insertions(+), 47 deletions(-) diff --git a/mk/disk.mk b/mk/disk.mk index 321ded02..95da27cc 100644 --- a/mk/disk.mk +++ b/mk/disk.mk @@ -5,8 +5,8 @@ ifeq ($(FSTOOLS_IN_PODMAN),1) $(PODMAN_RUN) make $@ else mkdir -p $(BUILD) - -$(FUMOUNT) $(MOUNT_DIR) || true - -$(FUMOUNT) /tmp/redox_installer || true + $(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || echo "Warning: failed to unmount $(MOUNT_DIR) (may not have been mounted)" + $(FUMOUNT) /tmp/redox_installer 2>/dev/null || echo "Warning: failed to unmount /tmp/redox_installer (may not have been mounted)" rm -rf $@ $@.partial $(MOUNT_DIR) FILESYSTEM_SIZE=$(FILESYSTEM_SIZE) && \ if [ -z "$$FILESYSTEM_SIZE" ] ; then \ @@ -23,7 +23,7 @@ ifeq ($(FSTOOLS_IN_PODMAN),1) else mkdir -p $(LIVE_BUILD) rm -rf $@ $@.partial - -$(FUMOUNT) /tmp/redox_installer || true + $(FUMOUNT) /tmp/redox_installer 2>/dev/null || echo "Warning: failed to unmount /tmp/redox_installer (may not have been mounted)" FILESYSTEM_SIZE=$(FILESYSTEM_SIZE) && \ if [ -z "$$FILESYSTEM_SIZE" ] ; then \ FILESYSTEM_SIZE=$(shell $(INSTALLER) --filesystem-size -c $(FILESYSTEM_CONFIG)); \ @@ -49,9 +49,9 @@ ifeq ($(FSTOOLS_IN_PODMAN),1) $(PODMAN_RUN) make $@ else mkdir -p $(BUILD) - -$(FUMOUNT) $(MOUNT_DIR) || true + $(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || echo "Warning: failed to unmount $(MOUNT_DIR) (may not have been mounted)" rm -rf $@ $@.partial $(MOUNT_DIR) - -$(FUMOUNT) /tmp/redox_installer || true + $(FUMOUNT) /tmp/redox_installer 2>/dev/null || echo "Warning: failed to unmount /tmp/redox_installer (may not have been mounted)" FILESYSTEM_SIZE=$(FILESYSTEM_SIZE) && \ if [ -z "$$FILESYSTEM_SIZE" ] ; then \ FILESYSTEM_SIZE=$(shell $(INSTALLER) --filesystem-size -c $(FILESYSTEM_CONFIG)); \ @@ -64,7 +64,7 @@ else pgrep redoxfs umask 002 && $(INSTALLER) $(INSTALLER_OPTS) -c $(FILESYSTEM_CONFIG) $(MOUNT_DIR) sync - -$(FUMOUNT) $(MOUNT_DIR) || true + $(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || echo "Warning: failed to unmount $(MOUNT_DIR) after install" rm -rf $(MOUNT_DIR) mv $@.partial $@ endif @@ -104,8 +104,8 @@ ifeq ($(FSTOOLS_IN_PODMAN),1) $(PODMAN_RUN) make $@ else @sync - -$(FUMOUNT) $(MOUNT_DIR) + $(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || echo "Warning: failed to unmount $(MOUNT_DIR)" @rm -rf $(MOUNT_DIR) - @-$(FUMOUNT) /tmp/redox_installer 2>/dev/null || true + @$(FUMOUNT) /tmp/redox_installer 2>/dev/null || echo "Warning: failed to unmount /tmp/redox_installer" @echo "\033[1;36;49mFilesystem unmounted\033[0m" endif diff --git a/mk/prefix.mk b/mk/prefix.mk index 380bf07c..313a3727 100644 --- a/mk/prefix.mk +++ b/mk/prefix.mk @@ -114,7 +114,8 @@ ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else mkdir -p "$(@D)" - wget -O $@.partial "https://static.redox-os.org/toolchain/$(HOST_TARGET)/$(TARGET)/$(@F)" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.redox-os.org/toolchain/$(HOST_TARGET)/$(TARGET)/$(@F)" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ endif @@ -137,7 +138,8 @@ ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else mkdir -p "$(@D)" - wget -O $@.partial "https://static.redox-os.org/pkg/id_ed25519.pub.toml" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.redox-os.org/pkg/id_ed25519.pub.toml" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ endif @@ -146,7 +148,8 @@ ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else mkdir -p "$(@D)" - wget -O $@.partial "https://static.redox-os.org/pkg/$(TARGET)/$(@F)" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.redox-os.org/pkg/$(TARGET)/$(@F)" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ endif @@ -337,7 +340,8 @@ ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else mkdir -p "$(@D)" - wget -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/$*-nightly-$(HOST_TARGET).tar.xz" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/$*-nightly-$(HOST_TARGET).tar.xz" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ endif @@ -346,7 +350,8 @@ ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else mkdir -p "$(@D)" - wget -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/rust-std-nightly-$(HOST_TARGET).tar.xz" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/rust-std-nightly-$(HOST_TARGET).tar.xz" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ endif @@ -356,7 +361,8 @@ ifeq ($(PODMAN_BUILD),1) else mkdir -p "$(@D)" ifeq ($(TARGET),x86_64-unknown-redox) - wget -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/rust-std-nightly-$(TARGET).tar.xz" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/rust-std-nightly-$(TARGET).tar.xz" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ else touch $@ @@ -368,7 +374,8 @@ ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else mkdir -p "$(@D)" - wget -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/rust-src-nightly.tar.xz" + wget --tries=3 --timeout=30 --waitretry=5 -O $@.partial "https://static.rust-lang.org/dist/$(UPSTREAM_RUSTC_VERSION)/rust-src-nightly.tar.xz" + @test -s $@.partial || { echo "Error: download failed or produced empty file: $@"; rm -f $@.partial; exit 1; } mv $@.partial $@ endif diff --git a/src/bin/repo.rs b/src/bin/repo.rs index 0ab3c214..406930e3 100644 --- a/src/bin/repo.rs +++ b/src/bin/repo.rs @@ -223,16 +223,14 @@ fn main_inner() -> anyhow::Result<()> { } if command == CliCommand::Cook && config.cook.tui { match run_tui_cook(config.clone(), recipes.clone()) { - Ok(TuiApp { - dump_logs_on_exit: Some((name, err)), - .. - }) => { - let _ = stderr().write(err.as_bytes()); - let _ = stderr().write(b"\n\n"); - print_failed(&command, &name); - return Err(anyhow!("Execution has failed")); - } - Ok(app) => { + Ok(mut app) => { + app.shutdown_log_writer(); + if let Some((name, err)) = app.dump_logs_on_exit.take() { + let _ = stderr().write(err.as_bytes()); + let _ = stderr().write(b"\n\n"); + print_failed(&command, &name); + return Err(anyhow!("Execution has failed")); + } for (recipe, status) in app.recipes { match status { RecipeStatus::Cached => print_cached(&command, &recipe.name), @@ -368,7 +366,10 @@ fn repo_inner( let th = thread::spawn(move || { while let Ok(update) = status_rx.recv() { match &update { - StatusUpdate::CookThreadFinished => break, + StatusUpdate::CookThreadFinished => { + app.shutdown_log_writer(); + break; + } StatusUpdate::FailCook(r, _) => { let (logs, line) = app.get_recipe_log(&r.name); if let Some(logs) = logs { @@ -812,7 +813,7 @@ fn handle_push(recipes: &Vec, config: &CliConfig) -> anyhow::Result< let mut total_count: u64 = 0; let mut visited: HashSet = HashSet::new(); let num_recipes = recipes.len(); - PUSH_SYSROOT_DIR.set(config.sysroot_dir.clone()).unwrap(); + PUSH_SYSROOT_DIR.get_or_init(|| config.sysroot_dir.clone()); let handle_push_inner = move |package_name: &PackageName, _prefix: &str, _is_last: bool, @@ -972,6 +973,19 @@ enum StatusUpdate { CookThreadFinished, } +/// Messages sent to the background log-writer thread so that file I/O +/// never blocks the TUI event loop. +enum LogWriterMessage { + /// Write `content` to `path` (log file for `name`). + Write { + _name: PackageName, + path: PathBuf, + content: String, + }, + /// Shut down the writer thread. + Shutdown, +} + #[derive(PartialEq)] enum JobType { Fetch, @@ -1014,10 +1028,30 @@ struct TuiApp { prompt: Option, dump_logs_anyway: bool, dump_logs_on_exit: Option<(PackageName, String)>, + log_writer_tx: mpsc::Sender, } impl TuiApp { fn new(recipes: Vec) -> Self { + let (log_writer_tx, log_writer_rx) = mpsc::channel::(); + thread::spawn(move || { + while let Ok(msg) = log_writer_rx.recv() { + match msg { + LogWriterMessage::Write { _name, path, content } => { + if content.trim_end().is_empty() { + continue; + } + if let Some(parent) = path.parent() { + let _ = fs::create_dir_all(parent); + } + if let Err(e) = fs::write(&path, &content) { + eprintln!("log writer: failed to write {}: {e}", path.display()); + } + } + LogWriterMessage::Shutdown => break, + } + } + }); Self { recipes: recipes .iter() @@ -1046,6 +1080,7 @@ impl TuiApp { prompt: None, dump_logs_anyway: false, dump_logs_on_exit: None, + log_writer_tx, } } @@ -1087,17 +1122,6 @@ impl TuiApp { (log_text, log_line) } - pub fn write_log(&self, recipe_name: &PackageName, log_path: &PathBuf) -> anyhow::Result<()> { - let (Some(logs), line) = self.get_recipe_log(recipe_name) else { - return Ok(()); - }; - let str = strip_ansi_escapes::strip_str(join_logs(logs, line)); - if !str.trim_end().is_empty() { - fs::write(log_path, str)?; - } - return Ok(()); - } - // Update the state based on a message from a worker thread fn update_status(&mut self, update: StatusUpdate) { let (name, new_status) = match update { @@ -1127,20 +1151,30 @@ impl TuiApp { let _ = std::io::stdout().write_all(&chunk); } let log_list = self.logs.entry(name.clone()).or_default(); - // TODO: multibyte-aware line split? - while let Some(newline_pos) = buffer.iter().position(|&b| b == b'\n') { - let line_bytes = buffer.drain(..=newline_pos); - let line_str = String::from_utf8_lossy(&line_bytes.as_slice()); - let line_str_pos = line_str.trim_end(); - let line_str = line_str_pos.rsplit('\r').next().unwrap_or(&line_str_pos); + let text = String::from_utf8_lossy(&buffer); + let mut last_end = 0; + while let Some(pos) = text[last_end..].find('\n') { + let line_end = last_end + pos; + let line_str = text[last_end..line_end].trim_end(); + let line_str = line_str.rsplit('\r').next().unwrap_or(line_str); log_list.push(line_str.to_owned()); + last_end = line_end + 1; } + let consumed = text[..last_end].len(); + buffer.drain(..consumed); return; } StatusUpdate::FlushLog(name, path) => { - // TODO: This blocks the TUI, maybe open separate thread? - // FIXME: handle error here? - let _ = self.write_log(&name, &path); + let (logs, line) = self.get_recipe_log(&name); + let content = strip_ansi_escapes::strip_str(join_logs( + logs.unwrap_or(&Vec::new()), + line, + )); + let _ = self.log_writer_tx.send(LogWriterMessage::Write { + _name: name, + path, + content, + }); return; } StatusUpdate::Cooked(recipe, cached) => { @@ -1196,6 +1230,10 @@ impl TuiApp { .map(|(r, _)| r.name.clone()) .collect(); } + + fn shutdown_log_writer(&self) { + let _ = self.log_writer_tx.send(LogWriterMessage::Shutdown); + } } fn run_tui_cook(config: CliConfig, recipes: Vec) -> Result {