Skip to content

Commit fe3e40a

Browse files
ranfdevCopilot
ranfdev
and
Copilot
committed
Refactor distrobox download task flow
Move bundled distrobox download task creation into RootStore::create_task and keep downloader focused on execution logic, preparing for future telemetry hooks without adding telemetry behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4306d2b commit fe3e40a

2 files changed

Lines changed: 120 additions & 106 deletions

File tree

‎src/distrobox_downloader.rs‎

Lines changed: 102 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use crate::models::DistroboxTask;
44
use crate::models::RootStore;
55
use anyhow::{Context, anyhow};
66
use gtk::glib;
7-
use gtk::prelude::*;
87
use std::path::PathBuf;
98

109
pub const DISTROBOX_VERSION: &str = "1.8.2.4";
@@ -110,116 +109,115 @@ fn log(task: &DistroboxTask, msg: &str) {
110109
task.append_output("\n");
111110
}
112111

113-
pub fn download_distrobox(root_store: &RootStore) -> DistroboxTask {
114-
let root_store_weak = root_store.downgrade();
112+
pub async fn download_distrobox(
113+
task: DistroboxTask,
114+
root_store_weak: glib::WeakRef<RootStore>,
115+
) -> anyhow::Result<()> {
115116
// We should be able to actually use a CommandRunner runs in the flatpak sandbox, because it has all the tools we need.
116117
// Also, the data folder is writable there and should be mapped to the host.
117118
let command_runner = CommandRunner::new_real();
119+
let download_dir = get_bundled_distrobox_dir();
120+
let tarball_path = download_dir.join("distrobox.tar.gz");
121+
let url = format!(
122+
"https://github.com/89luca89/distrobox/archive/refs/tags/{}.tar.gz",
123+
DISTROBOX_VERSION
124+
);
125+
126+
// Ensure directory exists
127+
std::fs::create_dir_all(&download_dir).context("Failed to create download directory")?;
128+
129+
log(
130+
&task,
131+
&format!("Using download directory: {:?}", download_dir),
132+
);
133+
134+
// 1. Download
135+
log(&task, &format!("Downloading {}...", url));
136+
let mut curl_cmd = Command::new("curl");
137+
curl_cmd.arg("-L");
138+
curl_cmd.arg("-o");
139+
curl_cmd.arg(&tarball_path);
140+
curl_cmd.arg(&url);
141+
curl_cmd.stdout = crate::fakers::FdMode::Pipe;
142+
curl_cmd.stderr = crate::fakers::FdMode::Pipe;
143+
144+
let child = command_runner
145+
.spawn(curl_cmd)
146+
.context("Failed to run curl")?;
147+
148+
task.handle_child_output(child).await?;
149+
150+
// 2. Verify SHA256
151+
log(&task, "Verifying checksum...");
152+
let mut sha_cmd = Command::new("sha256sum");
153+
sha_cmd.arg(&tarball_path);
154+
sha_cmd.stdout = crate::fakers::FdMode::Pipe;
155+
sha_cmd.stderr = crate::fakers::FdMode::Pipe;
156+
157+
let output = command_runner.output(sha_cmd).await?;
158+
if !output.status.success() {
159+
return Err(anyhow!("sha256sum failed"));
160+
}
118161

119-
DistroboxTask::new("system", "Downloading Distrobox", move |task| async move {
120-
let download_dir = get_bundled_distrobox_dir();
121-
let tarball_path = download_dir.join("distrobox.tar.gz");
122-
let url = format!(
123-
"https://github.com/89luca89/distrobox/archive/refs/tags/{}.tar.gz",
124-
DISTROBOX_VERSION
125-
);
126-
127-
// Ensure directory exists
128-
std::fs::create_dir_all(&download_dir).context("Failed to create download directory")?;
129-
130-
log(
131-
&task,
132-
&format!("Using download directory: {:?}", download_dir),
133-
);
134-
135-
// 1. Download
136-
log(&task, &format!("Downloading {}...", url));
137-
let mut curl_cmd = Command::new("curl");
138-
curl_cmd.arg("-L");
139-
curl_cmd.arg("-o");
140-
curl_cmd.arg(&tarball_path);
141-
curl_cmd.arg(&url);
142-
curl_cmd.stdout = crate::fakers::FdMode::Pipe;
143-
curl_cmd.stderr = crate::fakers::FdMode::Pipe;
144-
145-
let child = command_runner
146-
.spawn(curl_cmd)
147-
.context("Failed to run curl")?;
148-
149-
task.handle_child_output(child).await?;
150-
151-
// 2. Verify SHA256
152-
log(&task, "Verifying checksum...");
153-
let mut sha_cmd = Command::new("sha256sum");
154-
sha_cmd.arg(&tarball_path);
155-
sha_cmd.stdout = crate::fakers::FdMode::Pipe;
156-
sha_cmd.stderr = crate::fakers::FdMode::Pipe;
157-
158-
let output = command_runner.output(sha_cmd).await?;
159-
if !output.status.success() {
160-
return Err(anyhow!("sha256sum failed"));
161-
}
162-
163-
let stdout = String::from_utf8_lossy(&output.stdout);
164-
let calculated_hash = stdout.split_whitespace().next().unwrap_or_default();
162+
let stdout = String::from_utf8_lossy(&output.stdout);
163+
let calculated_hash = stdout.split_whitespace().next().unwrap_or_default();
165164

166-
if calculated_hash != DISTROBOX_SHA256 {
167-
return Err(anyhow!(
168-
"Checksum mismatch. Expected {}, got {}",
169-
DISTROBOX_SHA256,
170-
calculated_hash
171-
));
172-
}
173-
log(&task, "Checksum verified.");
174-
175-
// 3. Extract
176-
log(&task, "Extracting...");
177-
let mut tar_cmd = Command::new("tar");
178-
tar_cmd.arg("xzf");
179-
tar_cmd.arg(&tarball_path);
180-
tar_cmd.arg("-C");
181-
tar_cmd.arg(&download_dir);
182-
tar_cmd.stdout = crate::fakers::FdMode::Pipe;
183-
tar_cmd.stderr = crate::fakers::FdMode::Pipe;
184-
185-
let child = command_runner.spawn(tar_cmd).context("Failed to run tar")?;
186-
187-
task.handle_child_output(child).await?;
188-
189-
// 3b. Clean up tarball
190-
log(&task, "Removing tarball...");
191-
std::fs::remove_file(&tarball_path).context("Failed to remove tarball")?;
192-
193-
// 4. Make executable (it should be already, but just in case)
194-
let binary_path = get_bundled_distrobox_path();
195-
log(
196-
&task,
197-
&format!("Setting executable permissions on {:?}...", binary_path),
198-
);
199-
200-
let mut chmod_cmd = Command::new("chmod");
201-
chmod_cmd.arg("+x");
202-
chmod_cmd.arg(&binary_path);
203-
chmod_cmd.stdout = crate::fakers::FdMode::Pipe;
204-
chmod_cmd.stderr = crate::fakers::FdMode::Pipe;
205-
206-
let output = command_runner.output(chmod_cmd).await?;
207-
if !output.status.success() {
208-
return Err(anyhow!("chmod failed"));
209-
}
165+
if calculated_hash != DISTROBOX_SHA256 {
166+
return Err(anyhow!(
167+
"Checksum mismatch. Expected {}, got {}",
168+
DISTROBOX_SHA256,
169+
calculated_hash
170+
));
171+
}
172+
log(&task, "Checksum verified.");
173+
174+
// 3. Extract
175+
log(&task, "Extracting...");
176+
let mut tar_cmd = Command::new("tar");
177+
tar_cmd.arg("xzf");
178+
tar_cmd.arg(&tarball_path);
179+
tar_cmd.arg("-C");
180+
tar_cmd.arg(&download_dir);
181+
tar_cmd.stdout = crate::fakers::FdMode::Pipe;
182+
tar_cmd.stderr = crate::fakers::FdMode::Pipe;
183+
184+
let child = command_runner.spawn(tar_cmd).context("Failed to run tar")?;
185+
186+
task.handle_child_output(child).await?;
187+
188+
// 3b. Clean up tarball
189+
log(&task, "Removing tarball...");
190+
std::fs::remove_file(&tarball_path).context("Failed to remove tarball")?;
191+
192+
// 4. Make executable (it should be already, but just in case)
193+
let binary_path = get_bundled_distrobox_path();
194+
log(
195+
&task,
196+
&format!("Setting executable permissions on {:?}...", binary_path),
197+
);
198+
199+
let mut chmod_cmd = Command::new("chmod");
200+
chmod_cmd.arg("+x");
201+
chmod_cmd.arg(&binary_path);
202+
chmod_cmd.stdout = crate::fakers::FdMode::Pipe;
203+
chmod_cmd.stderr = crate::fakers::FdMode::Pipe;
204+
205+
let output = command_runner.output(chmod_cmd).await?;
206+
if !output.status.success() {
207+
return Err(anyhow!("chmod failed"));
208+
}
210209

211-
log(&task, "Distrobox installed successfully.");
210+
log(&task, "Distrobox installed successfully.");
212211

213-
// Clean up old bundled versions
214-
log(&task, "Cleaning up old bundled versions...");
215-
cleanup_old_bundled_versions();
212+
// Clean up old bundled versions
213+
log(&task, "Cleaning up old bundled versions...");
214+
cleanup_old_bundled_versions();
216215

217-
if let Some(root_store) = root_store_weak.upgrade() {
218-
root_store.distrobox_version().refetch();
219-
root_store.update_bundled_update_available();
220-
root_store.set_current_dialog(crate::models::DialogType::None);
221-
}
216+
if let Some(root_store) = root_store_weak.upgrade() {
217+
root_store.distrobox_version().refetch();
218+
root_store.update_bundled_update_available();
219+
root_store.set_current_dialog(crate::models::DialogType::None);
220+
}
222221

223-
Ok(())
224-
})
222+
Ok(())
225223
}

‎src/models/root_store.rs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,8 +451,10 @@ impl RootStore {
451451
return task;
452452
}
453453
}
454-
let task = crate::distrobox_downloader::download_distrobox(self);
455-
self.tasks().append(&task);
454+
let root_store_weak = self.downgrade();
455+
let task = self.create_task("system", "Downloading Distrobox", move |task| async move {
456+
crate::distrobox_downloader::download_distrobox(task, root_store_weak).await
457+
});
456458
self.set_selected_task(Some(task.clone()));
457459
task
458460
}
@@ -1163,4 +1165,18 @@ mod tests {
11631165
_ => panic!("expected selected terminal to resolve by legacy program name"),
11641166
}
11651167
}
1168+
1169+
#[gtk::test]
1170+
fn test_download_distrobox_returns_existing_active_task() {
1171+
let store = RootStore::new(NullCommandRunnerBuilder::new().build());
1172+
let existing_task = DistroboxTask::new("system", "Downloading Distrobox", |_task| async {
1173+
pending::<anyhow::Result<()>>().await
1174+
});
1175+
store.tasks().append(&existing_task);
1176+
1177+
let returned_task = store.download_distrobox();
1178+
1179+
assert_eq!(returned_task, existing_task);
1180+
assert_eq!(store.tasks().iter().count(), 1);
1181+
}
11661182
}

0 commit comments

Comments
 (0)