Skip to content

Commit b46e62a

Browse files
ranfdevranfdev
committed
Properly abstract the factory of the embedded distrobox cmd
1 parent 556ede5 commit b46e62a

File tree

5 files changed

+64
-49
lines changed

5 files changed

+64
-49
lines changed

‎src/backends/distrobox/command.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
use crate::fakers::Command;
2+
3+
/// A factory that returns a base `Command` value for running `distrobox`.
4+
/// Invariant: the factory must return the base program only (e.g. `Command::new("distrobox")`).
5+
/// Any Flatpak wrapping should be applied by `CommandRunner` implementations.
6+
pub type CmdFactory = Box<dyn Fn() -> Command + 'static>;
7+
8+
pub fn default_cmd_factory() -> CmdFactory {
9+
Box::new(|| Command::new("distrobox"))
10+
}

‎src/backends/distrobox/distrobox.rs‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::{
1616
use tracing::{debug, error, info, warn};
1717

1818
use crate::backends::desktop_file::*;
19+
use crate::backends::distrobox::command::{CmdFactory, default_cmd_factory};
1920

2021
const POSIX_FIND_AND_CONCAT_DESKTOP_FILES: &str =
2122
include_str!("POSIX_FIND_AND_CONCAT_DESKTOP_FILES.sh");
@@ -101,6 +102,7 @@ impl DesktopFiles {
101102

102103
pub struct Distrobox {
103104
cmd_runner: CommandRunner,
105+
cmd_factory: CmdFactory,
104106
}
105107

106108
#[derive(Clone, Debug, PartialEq, Hash)]
@@ -427,13 +429,13 @@ impl DistroboxCommandRunnerResponse {
427429
}
428430

429431
fn build_version_response() -> (Command, String) {
430-
let mut cmd = Command::new("distrobox");
432+
let mut cmd = default_cmd_factory()();
431433
cmd.arg("version");
432434
(cmd, "distrobox: 1.7.2.1".to_string())
433435
}
434436

435437
fn build_no_version_response() -> (Command, Rc<dyn Fn() -> io::Result<String>>) {
436-
let mut cmd = Command::new("distrobox");
438+
let mut cmd = default_cmd_factory()();
437439
cmd.arg("version");
438440
(cmd, Rc::new(|| Err(io::Error::from_raw_os_error(0))))
439441
}
@@ -451,14 +453,14 @@ impl DistroboxCommandRunnerResponse {
451453
output.push_str(&container.image);
452454
output.push('\n');
453455
}
454-
let mut cmd = Command::new("distrobox");
456+
let mut cmd = default_cmd_factory()();
455457
cmd.arg("ls").arg("--no-color");
456458
(cmd, output.clone())
457459
}
458460

459461
fn build_compatibility_response(images: &[String]) -> (Command, String) {
460462
let output = images.join("\n");
461-
let mut cmd = Command::new("distrobox");
463+
let mut cmd = default_cmd_factory()();
462464
cmd.arg("create").arg("--compatibility");
463465
(cmd, output)
464466
}
@@ -517,20 +519,16 @@ impl DistroboxCommandRunnerResponse {
517519

518520
toml.push_str("[user]\n");
519521

520-
commands.push((
521-
Command::new_with_args(
522-
"distrobox",
523-
[
524-
"enter",
525-
box_name,
526-
"--",
527-
"sh",
528-
"-c",
529-
POSIX_FIND_AND_CONCAT_DESKTOP_FILES,
530-
],
531-
),
532-
toml,
533-
));
522+
let mut db_cmd = default_cmd_factory()();
523+
db_cmd.args([
524+
"enter",
525+
box_name,
526+
"--",
527+
"sh",
528+
"-c",
529+
POSIX_FIND_AND_CONCAT_DESKTOP_FILES,
530+
]);
531+
commands.push((db_cmd, toml));
534532

535533
commands
536534
}
@@ -565,12 +563,16 @@ impl DistroboxCommandRunnerResponse {
565563
}
566564

567565
impl Distrobox {
568-
pub fn new(cmd_runner: CommandRunner) -> Self {
569-
Self { cmd_runner }
566+
// The command factory ensures we can customize the distrobox executable path, e.g. to use a bundled version.
567+
pub fn new(cmd_runner: CommandRunner, cmd_factory: CmdFactory) -> Self {
568+
Self {
569+
cmd_runner,
570+
cmd_factory,
571+
}
570572
}
571573

572574
fn dbcmd(&self) -> Command {
573-
Command::new("distrobox")
575+
(self.cmd_factory)()
574576
}
575577

576578
pub fn null_command_runner(responses: &[DistroboxCommandRunnerResponse]) -> CommandRunner {
@@ -1083,7 +1085,7 @@ impl Distrobox {
10831085

10841086
impl Default for Distrobox {
10851087
fn default() -> Self {
1086-
Self::new(CommandRunner::new_null())
1088+
Self::new(CommandRunner::new_null(), default_cmd_factory())
10871089
}
10881090
}
10891091

@@ -1122,6 +1124,7 @@ d24405b14180 | ubuntu | Created | ghcr.io/ublue-os/ubun
11221124
NullCommandRunnerBuilder::new()
11231125
.cmd(&["distrobox", "ls", "--no-color"], output)
11241126
.build(),
1127+
default_cmd_factory()
11251128
);
11261129
assert_eq!(
11271130
db.list().await?,
@@ -1147,6 +1150,8 @@ d24405b14180 | ubuntu | Created | ghcr.io/ublue-os/ubun
11471150
NullCommandRunnerBuilder::new()
11481151
.cmd(&["distrobox", "version"], output)
11491152
.build(),
1153+
default_cmd_factory()
1154+
11501155
);
11511156
assert_eq!(db.version().await?, "1.7.2.1".to_string(),);
11521157
Ok(())
@@ -1201,6 +1206,8 @@ Categories=Utility;Network;";
12011206
&desktop_files_toml,
12021207
)
12031208
.build(),
1209+
default_cmd_factory()
1210+
12041211
);
12051212

12061213
let apps = block_on(db.list_apps("ubuntu"))?;
@@ -1253,6 +1260,8 @@ Categories=Utility;Security;";
12531260
&desktop_files_toml,
12541261
)
12551262
.build(),
1263+
default_cmd_factory()
1264+
12561265
);
12571266

12581267
let apps = block_on(db.list_apps("ubuntu"))?;
@@ -1270,7 +1279,7 @@ Categories=Utility;Security;";
12701279
#[test]
12711280
fn create() -> Result<(), Error> {
12721281
let _ = tracing_subscriber::fmt().with_test_writer().try_init();
1273-
let db = Distrobox::new(CommandRunner::new_null());
1282+
let db = Distrobox::new(CommandRunner::new_null(), default_cmd_factory());
12741283
let output_tracker = db.cmd_runner.output_tracker();
12751284
debug!("Testing container creation");
12761285
let args = CreateArgs {
@@ -1294,7 +1303,7 @@ Categories=Utility;Security;";
12941303
}
12951304
#[test]
12961305
fn assemble() -> Result<(), Error> {
1297-
let db = Distrobox::new(CommandRunner::new_null());
1306+
let db = Distrobox::new(CommandRunner::new_null(), default_cmd_factory());
12981307
let output_tracker = db.cmd_runner.output_tracker();
12991308
db.assemble("/path/to/assemble.yml")?;
13001309
assert_eq!(
@@ -1306,7 +1315,7 @@ Categories=Utility;Security;";
13061315

13071316
#[test]
13081317
fn remove() -> Result<(), Error> {
1309-
let db = Distrobox::new(CommandRunner::new_null());
1318+
let db = Distrobox::new(CommandRunner::new_null(), default_cmd_factory());
13101319
let output_tracker = db.cmd_runner.output_tracker();
13111320
block_on(db.remove("ubuntu"))?;
13121321
assert_eq!(

‎src/backends/distrobox/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub mod command;
12
mod distrobox;
23

34
pub use distrobox::*;

‎src/models/root_store.rs‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -166,23 +166,24 @@ impl RootStore {
166166
.terminal_repository
167167
.replace(TerminalRepository::new(command_runner.clone()));
168168

169-
// Create a mapped command runner for Distrobox that handles the executable path
170-
let settings = this.settings();
171-
let distrobox_cmd_runner = command_runner.clone().map_cmd(move |mut cmd| {
172-
// Only map commands that are trying to run "distrobox"
173-
if cmd.program == "distrobox" {
174-
let distrobox_executable = settings.string("distrobox-executable");
175-
if distrobox_executable == "bundled" {
176-
let bundled_path = crate::distrobox_downloader::get_bundled_distrobox_path();
177-
cmd.program = bundled_path.into();
178-
}
179-
}
180-
cmd
169+
// Build a CmdFactory that will be injected into the Distrobox backend. The factory
170+
// is created here (root_store) so the distrobox module does not depend on `gio::Settings`.
171+
let this_clone = this.clone();
172+
let cmd_factory: crate::backends::distrobox::command::CmdFactory = Box::new(move || {
173+
let distrobox_executable_val = this_clone.settings().string("distrobox-executable");
174+
let selected_program: String = if distrobox_executable_val == "bundled" {
175+
crate::distrobox_downloader::get_bundled_distrobox_path()
176+
.to_string_lossy()
177+
.into_owned()
178+
} else {
179+
"distrobox".into()
180+
};
181+
crate::fakers::Command::new(selected_program.clone())
181182
});
182183

183184
this.imp()
184185
.distrobox
185-
.set(Distrobox::new(distrobox_cmd_runner))
186+
.set(Distrobox::new(command_runner.clone(), cmd_factory))
186187
.or(Err("distrobox already set"))
187188
.unwrap();
188189

‎src/widgets/integrated_terminal.rs‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use gtk::{
77
use vte4::prelude::*;
88

99
use crate::i18n::gettext;
10-
use crate::{fakers::Command, models::Container};
10+
use crate::models::Container;
1111

1212
mod imp {
1313
use std::cell::OnceCell;
@@ -152,16 +152,10 @@ impl IntegratedTerminal {
152152
imp.reload_button.set_visible(false);
153153
let root_store = self.container().root_store();
154154

155-
// Prepare the shell command
156-
let shell = root_store
157-
.command_runner()
158-
.wrap_command(
159-
Command::new("distrobox")
160-
.arg("enter")
161-
.arg(self.container().name())
162-
.clone(),
163-
)
164-
.to_vec();
155+
// Prepare the shell command via the Distrobox backend (uses injected factory)
156+
let name = self.container().name();
157+
let enter_cmd = root_store.distrobox().enter_cmd(&name);
158+
let shell = root_store.command_runner().wrap_command(enter_cmd).to_vec();
165159

166160
let fut = imp.terminal.spawn_future(
167161
vte4::PtyFlags::DEFAULT,

0 commit comments

Comments
 (0)