From 6dbde86575cb8250a38f854430ad908c1fa1cdeb Mon Sep 17 00:00:00 2001 From: archivr-qa Date: Mon, 24 Aug 2026 15:29:25 +0200 Subject: [PATCH] fix: show forced yt-dlp candidate in status --- Cargo.lock | 1 + crates/archivr-cli/Cargo.toml | 3 + crates/archivr-cli/src/main.rs | 65 +++++++++++++++++++-- crates/archivr-core/src/downloader/ytdlp.rs | 13 +++-- 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 731f292..c6f2510 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -100,6 +100,7 @@ dependencies = [ "reqwest", "rusqlite", "serde_json", + "tempfile", ] [[package]] diff --git a/crates/archivr-cli/Cargo.toml b/crates/archivr-cli/Cargo.toml index 6f0d0bf..a198852 100644 --- a/crates/archivr-cli/Cargo.toml +++ b/crates/archivr-cli/Cargo.toml @@ -16,3 +16,6 @@ regex.workspace = true rusqlite.workspace = true serde_json.workspace = true reqwest.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/archivr-cli/src/main.rs b/crates/archivr-cli/src/main.rs index 83dd947..6c1db54 100644 --- a/crates/archivr-cli/src/main.rs +++ b/crates/archivr-cli/src/main.rs @@ -3,7 +3,7 @@ use archivr_core::{ archive, capture::CaptureConfig, downloader::ytdlp::{ - pinned_yt_dlp, probe_version, resolve_yt_dlp, state_dir, state_dir_yt_dlp, + forced_yt_dlp, pinned_yt_dlp, probe_version, resolve_yt_dlp, state_dir, state_dir_yt_dlp, }, }; use clap::{Parser, Subcommand}; @@ -147,22 +147,32 @@ fn yt_dlp_state_dir() -> Result { .context("could not determine a state directory (is $HOME set?)") } -/// Renders one `status` row. Missing candidates show an em dash. -fn status_row(role: &str, path: Option<&Path>, chosen: &Path) { +/// Formats one `status` row. Missing candidates show an em dash. +fn format_status_row(role: &str, path: Option<&Path>, chosen: &Path) -> String { match path { Some(p) => { let version = probe_version(p).unwrap_or_else(|| "—".to_string()); let star = if p == chosen { "*" } else { "" }; - println!("{role}\t{}\t{version}\t{star}", p.display()); + format!("{role}\t{}\t{version}\t{star}", p.display()) } - None => println!("{role}\t—\t—\t"), + None => format!("{role}\t—\t—\t"), } } +/// Prints one `status` row. Missing candidates show an em dash. +fn status_row(role: &str, path: Option<&Path>, chosen: &Path) { + println!("{}", format_status_row(role, path, chosen)); +} + fn yt_dlp_status() -> Result<()> { let chosen = resolve_yt_dlp(); println!("role\tpath\tversion\tchosen"); + status_row( + "force (ARCHIVR_YT_DLP_FORCE)", + forced_yt_dlp().as_deref(), + &chosen, + ); status_row("env (ARCHIVR_YT_DLP)", pinned_yt_dlp().as_deref(), &chosen); // Show the state-dir slot even when empty, so users can see where an @@ -289,3 +299,48 @@ fn yt_dlp_update(requested_version: Option<&str>) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::format_status_row; + use archivr_core::downloader::ytdlp::{ + forced_yt_dlp, resolve_yt_dlp_uncached, YT_DLP_FORCE_ENV, + }; + use std::path::Path; + + fn fake_yt_dlp(path: &Path, version: &str) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, format!("#!/bin/sh\necho {version}\n")).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + } + + #[test] + fn forced_candidate_is_rendered_and_selected() { + let tmp = tempfile::tempdir().unwrap(); + let forced = tmp.path().join("forced/yt-dlp"); + fake_yt_dlp(&forced, "2020.01.01"); + unsafe { std::env::set_var(YT_DLP_FORCE_ENV, &forced) }; + + let candidate = forced_yt_dlp(); + assert_eq!(candidate.as_deref(), Some(forced.as_path())); + let chosen = resolve_yt_dlp_uncached(); + assert_eq!(chosen, forced); + assert_eq!( + format_status_row( + "force (ARCHIVR_YT_DLP_FORCE)", + candidate.as_deref(), + &chosen, + ), + format!( + "force (ARCHIVR_YT_DLP_FORCE)\t{}\t2020.01.01\t*", + forced.display() + ) + ); + + unsafe { std::env::remove_var(YT_DLP_FORCE_ENV) }; + } +} diff --git a/crates/archivr-core/src/downloader/ytdlp.rs b/crates/archivr-core/src/downloader/ytdlp.rs index e55943d..12de8a4 100644 --- a/crates/archivr-core/src/downloader/ytdlp.rs +++ b/crates/archivr-core/src/downloader/ytdlp.rs @@ -51,6 +51,12 @@ pub fn state_dir_yt_dlp() -> Option { state_dir().map(|d| d.join("yt-dlp").join("yt-dlp")) } +/// The explicit yt-dlp override, if it points to a file on disk. +pub fn forced_yt_dlp() -> Option { + let p = PathBuf::from(env::var_os(YT_DLP_FORCE_ENV).filter(|v| !v.is_empty())?); + p.is_file().then_some(p) +} + /// The nix-pinned yt-dlp advertised via `ARCHIVR_YT_DLP`, if it exists on disk. pub fn pinned_yt_dlp() -> Option { let p = PathBuf::from(env::var_os(YT_DLP_ENV).filter(|v| !v.is_empty())?); @@ -90,11 +96,8 @@ pub fn yt_dlp_candidates() -> Vec<(&'static str, PathBuf)> { /// Priority: `ARCHIVR_YT_DLP_FORCE` > newest of (pinned, state-dir) by version /// string > bare `yt-dlp` (PATH lookup, the historical behaviour). pub fn resolve_yt_dlp_uncached() -> PathBuf { - if let Some(forced) = env::var_os(YT_DLP_FORCE_ENV).filter(|v| !v.is_empty()) { - let forced = PathBuf::from(forced); - if forced.is_file() { - return forced; - } + if let Some(forced) = forced_yt_dlp() { + return forced; } yt_dlp_candidates()