Skip to content

Commit 42e6b3c

Browse files
authored
Fix fresh Flatpak tool installation checks (#497)
* Detect missing tools on fresh Flatpak profiles * Handle missing tools independently during update checks * Default fresh profiles to Install before probing tools * Format fresh-install tool check * Remove obsolete tool version display helper * Disable tool action until initial presence check completes
1 parent 00ec8f0 commit 42e6b3c

1 file changed

Lines changed: 97 additions & 125 deletions

File tree

‎src/preferences.rs‎

Lines changed: 97 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -667,39 +667,11 @@ pub fn show(
667667
crate::video_tools::TOOL_PROBE_TIMEOUT,
668668
)
669669
}
670-
fn tool_version(binary: &std::path::Path, version_arg: &str) -> Option<String> {
671-
let first = tool_first_line(binary, version_arg)?;
672-
// ffmpeg prints a whole sentence ("ffmpeg version n9.0.1 ..."):
673-
// keep the version token so the row stays readable.
674-
if binary
675-
.file_name()
676-
.is_some_and(|n| n.to_string_lossy() == "ffmpeg")
677-
&& let Some(token) = first.split_whitespace().nth(2)
678-
{
679-
return Some(format!("ffmpeg {token}"));
680-
}
681-
Some(first)
682-
}
683-
/// Probe installed tool versions: `(yt-dlp, ffmpeg, quickjs?)` plus
684-
/// gallery-dl; `None` when the core tools are missing, falling back to
685-
/// the binary path if `--version` fails. quickjs is `None` when no
686-
/// `qjs` is on hand — it is only provisioned for YouTube. Only tool
687-
/// presence matters for the row ("Ready" vs "Not installed"); the
688-
/// versions themselves are not shown.
689-
fn installed_tool_versions() -> Option<(String, String, Option<String>, Option<String>)> {
690-
crate::video::resolve_libraries().ok().map(|libs| {
691-
let yt = tool_version(&libs.youtube, "--version")
692-
.map(|v| format!("yt-dlp {v}"))
693-
.unwrap_or_else(|| libs.youtube.display().to_string());
694-
let ff = tool_version(&libs.ffmpeg, "-version")
695-
.unwrap_or_else(|| libs.ffmpeg.display().to_string());
696-
let qjs = crate::video_tools::find_quickjs()
697-
.and_then(|p| tool_version(&p, "--version").map(|v| format!("quickjs {v}")));
698-
let gdl = crate::gallery_tools::resolve_gallerydl()
699-
.ok()
700-
.and_then(|p| tool_version(&p, "--version").map(|v| format!("gallery-dl {v}")));
701-
(yt, ff, qjs, gdl)
702-
})
670+
/// The row only needs to know whether both required video tools are present.
671+
/// Check filesystem paths only: do not launch subprocesses or contact upstream
672+
/// services just to choose between Install and Check for Updates.
673+
fn core_tools_installed() -> bool {
674+
crate::video::resolve_libraries().is_ok()
703675
}
704676
/// What the tools-row button does: Install when tools are missing, Check to
705677
/// probe each tool's upstream for a newer release, Update once one is
@@ -715,21 +687,18 @@ pub fn show(
715687
row: &adw::ActionRow,
716688
btn: &gtk4::Button,
717689
action: &std::rc::Rc<std::cell::Cell<ToolAction>>,
718-
probed: Option<(String, String, Option<String>, Option<String>)>,
690+
installed: bool,
719691
) {
720-
match probed {
721-
// Tool names live in the group description; the row carries
722-
// state only: no versions, no ffmpeg/quickjs noise.
723-
Some(_) => {
724-
row.set_subtitle(&gettext("Ready"));
725-
btn.set_label(&gettext("Check for Updates"));
726-
action.set(ToolAction::Check);
727-
}
728-
None => {
729-
row.set_subtitle(&gettext("Not installed"));
730-
btn.set_label(&gettext("Install"));
731-
action.set(ToolAction::Install);
732-
}
692+
// Tool names live in the group description; the row carries state
693+
// only, with no version strings or optional-tool noise.
694+
if installed {
695+
row.set_subtitle(&gettext("Ready"));
696+
btn.set_label(&gettext("Check for Updates"));
697+
action.set(ToolAction::Check);
698+
} else {
699+
row.set_subtitle(&gettext("Not installed"));
700+
btn.set_label(&gettext("Install"));
701+
action.set(ToolAction::Install);
733702
}
734703
}
735704
fn refresh_video_tools(
@@ -739,14 +708,15 @@ pub fn show(
739708
action: &std::rc::Rc<std::cell::Cell<ToolAction>>,
740709
) {
741710
spin.set_visible(false);
742-
// Probe off the GTK thread (three subprocesses), then paint.
711+
btn.set_sensitive(false);
712+
// Presence-only probe off the GTK thread; no network calls or subprocesses.
743713
let (row, btn, action) = (row.clone(), btn.clone(), action.clone());
744714
gtk4::glib::spawn_future_local(async move {
745-
let probed = gio::spawn_blocking(installed_tool_versions)
715+
let installed = gio::spawn_blocking(core_tools_installed)
746716
.await
747-
.ok()
748-
.flatten();
749-
paint_tools_state(&row, &btn, &action, probed);
717+
.unwrap_or(false);
718+
paint_tools_state(&row, &btn, &action, installed);
719+
btn.set_sensitive(true);
750720
});
751721
}
752722
let video_page = adw::PreferencesPage::builder()
@@ -999,13 +969,15 @@ pub fn show(
999969
video_tools_group.add(&video_tools_row);
1000970
// Nothing is probed on open: versions and updates are checked only when
1001971
// the user presses Check for Updates.
1002-
let tool_action = std::rc::Rc::new(std::cell::Cell::new(ToolAction::Check));
972+
let tool_action = std::rc::Rc::new(std::cell::Cell::new(ToolAction::Install));
1003973
// Tools needing update (from the check); passed to install_progress::run
1004974
// so the popup only shows what's actually downloading.
1005975
let pending_updates: std::rc::Rc<std::cell::RefCell<Vec<String>>> =
1006976
std::rc::Rc::new(std::cell::RefCell::new(Vec::new()));
1007-
video_tools_btn.set_label(&gettext("Check for Updates"));
1008-
video_tools_row.set_subtitle(&gettext("Press Check for Updates to see versions"));
977+
// Start in the safe fresh-install state; the local presence probe below
978+
// switches to Check for Updates only when yt-dlp and ffmpeg are present.
979+
video_tools_btn.set_label(&gettext("Install"));
980+
video_tools_row.set_subtitle(&gettext("Not installed"));
1009981
{
1010982
let (row, btn, spin) = (
1011983
video_tools_row.clone(),
@@ -1067,24 +1039,23 @@ pub fn show(
10671039
// upstream for its latest release tag: yt-dlp/yt-dlp,
10681040
// BtbN/FFmpeg-Builds, quickjs-ng/quickjs, gdl-org/builds.
10691041
let installed = gio::spawn_blocking(|| {
1070-
crate::video::resolve_libraries().ok().map(|libs| {
1071-
let yt = tool_first_line(&libs.youtube, "--version");
1072-
let ff = tool_first_line(&libs.ffmpeg, "-version");
1073-
let qjs = crate::video_tools::find_quickjs()
1074-
.and_then(|p| tool_first_line(&p, "--version"));
1075-
let gdl = crate::gallery_tools::resolve_gallerydl()
1076-
.ok()
1077-
.and_then(|p| tool_first_line(&p, "--version"));
1078-
let ffprobe_available = crate::video_tools::toolchain_dir_in(
1079-
&crate::video_tools::tool_search_dirs(),
1080-
)
1081-
.is_some();
1082-
(yt, ff, qjs, gdl, ffprobe_available)
1083-
})
1042+
let dirs = crate::video_tools::tool_search_dirs();
1043+
let inspect = |name: &str, version_arg: &str| {
1044+
crate::video_tools::find_in_dirs(name, &dirs).map(|path| {
1045+
let version = tool_first_line(&path, version_arg);
1046+
(path, version)
1047+
})
1048+
};
1049+
let yt = inspect("yt-dlp", "--version");
1050+
let ff = inspect("ffmpeg", "-version");
1051+
let qjs = inspect("qjs", "--version");
1052+
let gdl = inspect("gallery-dl", "--version");
1053+
let ffprobe_available =
1054+
crate::video_tools::toolchain_dir_in(&dirs).is_some();
1055+
(yt, ff, qjs, gdl, ffprobe_available)
10841056
})
10851057
.await
1086-
.ok()
1087-
.flatten();
1058+
.ok();
10881059
// Upstream tags come from the update cache: a click reuses a
10891060
// check from the last 15 minutes. The lookups run on Grab's
10901061
// Tokio runtime inside update_cache, never on this GLib task.
@@ -1113,8 +1084,11 @@ pub fn show(
11131084
let mut current: Vec<String> = Vec::new();
11141085
let mut indeterminate = installed.is_none();
11151086
if let Some((yt, ff, qjs, gdl, ffprobe_available)) = installed {
1116-
match (yt.as_deref(), yt_tag.as_deref()) {
1117-
(Some(installed), Some(tag))
1087+
// Presence is independent for each binary. In particular,
1088+
// resolve_libraries() must not hide all missing tools just
1089+
// because a fresh profile has no yt-dlp yet.
1090+
match (yt.as_ref(), yt_tag.as_deref()) {
1091+
(Some((_, Some(installed))), Some(tag))
11181092
if crate::video_tools::tool_update_available(installed, tag) =>
11191093
{
11201094
updates.push(format!(
@@ -1123,48 +1097,46 @@ pub fn show(
11231097
tag.trim()
11241098
));
11251099
}
1126-
(Some(installed), Some(_)) => {
1100+
(Some((_, Some(installed))), Some(_)) => {
11271101
current.push(format!("yt-dlp {}", installed.trim()));
11281102
}
1129-
_ => indeterminate = true,
1103+
(Some(_), _) => indeterminate = true,
1104+
(None, _) => missing.push("yt-dlp".to_string()),
11301105
}
1131-
match (ff.as_deref(), ff_tag.as_deref()) {
1132-
(Some(first_line), Some(tag)) => {
1133-
let token = crate::video_tools::ffmpeg_version_token(first_line)
1134-
.unwrap_or(first_line);
1135-
if crate::video_tools::ffmpeg_needs_update(
1136-
first_line,
1137-
tag,
1138-
ffprobe_available,
1139-
) {
1140-
updates.push(if ffprobe_available {
1141-
format!("ffmpeg {} → {}", token, tag.trim())
1106+
if !ffprobe_available {
1107+
if let Some((_, version)) = ff.as_ref() {
1108+
let detail = version
1109+
.as_deref()
1110+
.and_then(crate::video_tools::ffmpeg_version_token)
1111+
.unwrap_or("repair required");
1112+
updates.push(format!("ffmpeg {detail} (ffprobe missing)"));
1113+
} else {
1114+
missing.push("ffmpeg".to_string());
1115+
}
1116+
} else {
1117+
match (ff.as_ref(), ff_tag.as_deref()) {
1118+
(Some((_, Some(first_line))), Some(tag)) => {
1119+
let token =
1120+
crate::video_tools::ffmpeg_version_token(first_line)
1121+
.unwrap_or(first_line);
1122+
if crate::video_tools::ffmpeg_needs_update(
1123+
first_line,
1124+
tag,
1125+
ffprobe_available,
1126+
) {
1127+
updates.push(format!("ffmpeg {} → {}", token, tag.trim()));
11421128
} else {
1143-
format!(
1144-
"ffmpeg {} → {} (ffprobe missing)",
1145-
token,
1146-
tag.trim()
1147-
)
1148-
});
1149-
} else {
1150-
current.push(format!("ffmpeg {token}"));
1129+
current.push(format!("ffmpeg {token}"));
1130+
}
11511131
}
1132+
(Some(_), _) => indeterminate = true,
1133+
(None, _) => missing.push("ffmpeg".to_string()),
11521134
}
1153-
(Some(first_line), None) if !ffprobe_available => {
1154-
let token = crate::video_tools::ffmpeg_version_token(first_line)
1155-
.unwrap_or(first_line);
1156-
updates.push(format!("ffmpeg {token} (ffprobe missing)"));
1157-
}
1158-
(None, _) if !ffprobe_available => {
1159-
updates.push("ffmpeg (ffprobe missing)".to_string());
1160-
}
1161-
_ => indeterminate = true,
11621135
}
1163-
// quickjs is optional (provisioned for YouTube only):
1164-
// judged when installed, never blocks the check.
1165-
// If missing, offer to install it.
1166-
match (qjs.as_deref(), qjs_tag.as_deref()) {
1167-
(Some(installed), Some(tag))
1136+
// Optional tools are offered for installation whenever
1137+
// absent, even if their latest-release lookup is offline.
1138+
match (qjs.as_ref(), qjs_tag.as_deref()) {
1139+
(Some((_, Some(installed))), Some(tag))
11681140
if crate::video_tools::tool_update_available(installed, tag) =>
11691141
{
11701142
updates.push(format!(
@@ -1173,33 +1145,25 @@ pub fn show(
11731145
tag.trim()
11741146
));
11751147
}
1176-
(Some(installed), Some(_)) => {
1148+
(Some((_, Some(installed))), Some(_)) => {
11771149
current.push(format!("quickjs {}", installed.trim()));
11781150
}
1179-
(None, Some(_)) => {
1180-
missing.push(TOOL_QUICKJS.to_string());
1181-
}
1182-
_ => indeterminate = true,
1151+
(Some(_), _) => indeterminate = true,
1152+
(None, _) => missing.push(TOOL_QUICKJS.to_string()),
11831153
}
1184-
// gallery-dl is optional (provisioned for galleries only):
1185-
// judged when installed, never blocks the check.
1186-
// If missing, offer to install it.
1187-
match (gdl.as_deref(), gdl_tag.as_deref()) {
1188-
(Some(_installed), Some(tag)) if gdl_needs_update => {
1154+
match (gdl.as_ref(), gdl_tag.as_deref()) {
1155+
(Some((_, Some(installed))), Some(tag)) if gdl_needs_update => {
11891156
updates.push(format!(
11901157
"gallery-dl {} → {}",
1191-
_installed.trim(),
1158+
installed.trim(),
11921159
tag.trim()
11931160
));
11941161
}
1195-
(Some(installed), Some(_)) => {
1162+
(Some((_, Some(installed))), Some(_)) => {
11961163
current.push(format!("gallery-dl {}", installed.trim()));
11971164
}
1198-
(None, Some(_)) => {
1199-
missing.push(TOOL_GALLERY_DL.to_string());
1200-
}
1201-
(None, _) => {}
1202-
_ => indeterminate = true,
1165+
(Some(_), _) => indeterminate = true,
1166+
(None, _) => missing.push(TOOL_GALLERY_DL.to_string()),
12031167
}
12041168
}
12051169
if !missing.is_empty() || !updates.is_empty() {
@@ -1280,6 +1244,14 @@ pub fn show(
12801244
);
12811245
});
12821246
}
1247+
// Do not make a fresh Flatpak-data profile ask for updates before its tools
1248+
// exist. This is a local executable-presence check, not an upstream lookup.
1249+
refresh_video_tools(
1250+
&video_tools_row,
1251+
&video_tools_btn,
1252+
&video_tools_spin,
1253+
&tool_action,
1254+
);
12831255
video_page.add(&video_quality_group);
12841256
video_page.add(&video_auth_group);
12851257
video_page.add(&video_tools_group);

0 commit comments

Comments
 (0)