Skip to content

Commit 9e0b7a3

Browse files
joebonrichiejoebonrichie
committed
moss: Postblit code cleanup
Add common entry to execute triggers and split off common functionality into it's own function
1 parent 1929629 commit 9e0b7a3

File tree

2 files changed

+97
-133
lines changed

2 files changed

+97
-133
lines changed

‎moss/src/client/mod.rs‎

Lines changed: 33 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -422,74 +422,45 @@ impl Client {
422422
.progress_chars("■≡=- "),
423423
);
424424

425-
match &scope {
426-
TriggerScope::Transaction(install, scope) => {
425+
let phase_name = match &scope {
426+
TriggerScope::Transaction(_, _) => {
427427
progress_bar.set_message("Running transaction-scope triggers");
428-
429-
let timer = Instant::now();
430-
let phase_name = "transaction-scope-triggers";
431-
432-
info!(
433-
phase = phase_name,
434-
total_items = total_items,
435-
progress = 0.0,
436-
event_type = "progress_start",
437-
);
438-
439-
postblit::execute_transaction_triggers(install, scope, &triggers, |progress| {
440-
progress_bar.inc(progress.completed);
441-
info!(
442-
progress = (total_items + progress.completed) as f32 / total_items as f32,
443-
current = total_items + progress.in_progress as u64,
444-
total = total_items,
445-
event_type = "progress_update",
446-
"Executing {:?}",
447-
progress.items
448-
);
449-
})?;
450-
451-
info!(
452-
phase = phase_name,
453-
duration_ms = timer.elapsed().as_millis(),
454-
items_processed = triggers.len(),
455-
progress = 1.0,
456-
event_type = "progress_completed",
457-
);
428+
"transaction-scope-triggers"
458429
}
459-
TriggerScope::System(install, scope) => {
430+
TriggerScope::System(_, _) => {
460431
progress_bar.set_message("Running system-scope triggers");
432+
"system-scope-triggers"
433+
}
434+
};
461435

462-
let timer = Instant::now();
463-
let phase_name = "system-scope-triggers";
436+
let timer = Instant::now();
464437

465-
info!(
466-
phase = phase_name,
467-
total_items = total_items,
468-
progress = 0.0,
469-
event_type = "progress_start",
470-
);
438+
info!(
439+
phase = phase_name,
440+
total_items = total_items,
441+
progress = 0.0,
442+
event_type = "progress_start",
443+
);
471444

472-
postblit::execute_system_triggers(install, scope, &triggers, |progress| {
473-
progress_bar.inc(progress.completed);
474-
info!(
475-
progress = (total_items + progress.completed) as f32 / total_items as f32,
476-
current = total_items + progress.in_progress as u64,
477-
total = total_items,
478-
event_type = "progress_update",
479-
"Executing {:?}",
480-
progress.items
481-
);
482-
})?;
483-
484-
info!(
485-
phase = phase_name,
486-
duration_ms = timer.elapsed().as_millis(),
487-
items_processed = total_items,
488-
progress = 1.0,
489-
event_type = "progress_completed",
490-
);
491-
}
492-
};
445+
postblit::execute_triggers(scope, &triggers, |progress| {
446+
progress_bar.inc(progress.completed);
447+
info!(
448+
progress = (total_items + progress.completed) as f32 / total_items as f32,
449+
current = total_items + progress.in_progress as u64,
450+
total = total_items,
451+
event_type = "progress_update",
452+
"Executing {:?}",
453+
progress.items
454+
);
455+
})?;
456+
457+
info!(
458+
phase = phase_name,
459+
duration_ms = timer.elapsed().as_millis(),
460+
items_processed = total_items,
461+
progress = 1.0,
462+
event_type = "progress_completed",
463+
);
493464

494465
progress_bar.finish_and_clear();
495466

‎moss/src/client/postblit.rs‎

Lines changed: 64 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -161,49 +161,49 @@ pub(super) fn triggers<'a>(
161161
Ok(batches)
162162
}
163163

164+
/// Execute triggers based on TriggerScope
165+
///
166+
/// Execute either transaction or system scope triggers using container sandboxing as necessary
167+
pub fn execute_triggers(
168+
scope: TriggerScope<'_>,
169+
triggers: &[Vec<TriggerRunner>],
170+
on_progress: impl Fn(Progress) + Send + Sync,
171+
) -> Result<(), Error> {
172+
match scope {
173+
TriggerScope::Transaction(install, scope) => {
174+
execute_transaction_triggers(install, scope, triggers, &on_progress)?;
175+
}
176+
TriggerScope::System(install, scope) => {
177+
execute_system_triggers(install, scope, triggers, &on_progress)?;
178+
}
179+
};
180+
181+
Ok(())
182+
}
183+
164184
/// Execute transaction triggers
165185
///
166186
/// Transaction triggers are run via sandboxing ([`container::Container`]) to limit their
167187
/// system view, and limit write access. Each batch of triggers are executed in parallel
168188
/// to speed up execution time.
169-
pub fn execute_transaction_triggers(
189+
fn execute_transaction_triggers<P>(
170190
install: &Installation,
171191
scope: &super::Scope,
172192
triggers: &[Vec<TriggerRunner>],
173-
on_progress: impl Fn(Progress) + Send + Sync,
174-
) -> Result<(), Error> {
193+
on_progress: P,
194+
) -> Result<(), Error>
195+
where
196+
P: Fn(Progress) + Send + Sync,
197+
{
175198
let trigger_scope = TriggerScope::Transaction(install, scope);
199+
// TODO: Add caching support via /var/
176200
let isolation = Container::new(install.isolation_dir())
177201
.networking(false)
178202
.bind_ro(trigger_scope.host_path("etc"), "/etc")
179203
.bind_rw(trigger_scope.guest_path("usr"), "/usr")
180204
.work_dir("/");
181205

182-
let total = 0;
183-
184-
isolation.run(|| {
185-
// Ensure runtime threads get dropped
186-
let rayon_runtime = rayon::ThreadPoolBuilder::new().build().expect("rayon runtime");
187-
188-
rayon_runtime.install(|| {
189-
triggers.iter().try_for_each(|batch| {
190-
batch.par_iter().try_for_each(|trigger| {
191-
(on_progress)(Progress {
192-
in_progress: batch.len(),
193-
completed: total + 1,
194-
items: batch
195-
.iter()
196-
.map(|t| match t.handler() {
197-
Handler::Run { run, .. } => run.clone(),
198-
Handler::Delete { .. } => "delete operation".to_owned(),
199-
})
200-
.collect(),
201-
});
202-
execute_trigger_directly(trigger.trigger())
203-
})
204-
})
205-
})
206-
})?;
206+
isolation.run(|| execute_triggers_directly(triggers, &on_progress))?;
207207

208208
Ok(())
209209
}
@@ -214,63 +214,28 @@ pub fn execute_transaction_triggers(
214214
/// live root filesystem, and will force sandboxing when using a non-`/` root (such as using the
215215
/// `-D argument with `moss install`). Each batch of triggers is executed in parallel to speed up
216216
/// execution time.
217-
pub fn execute_system_triggers(
217+
fn execute_system_triggers<P>(
218218
install: &Installation,
219219
scope: &super::Scope,
220220
triggers: &[Vec<TriggerRunner>],
221-
on_progress: impl Fn(Progress) + Send + Sync,
222-
) -> Result<(), Error> {
221+
on_progress: P,
222+
) -> Result<(), Error>
223+
where
224+
P: Fn(Progress) + Send + Sync,
225+
{
223226
let trigger_scope = TriggerScope::System(install, scope);
224227

225-
let total = 0;
226-
227228
// OK, if the root == `/` then we can run directly, otherwise we need to containerise with RW.
228229
if install.root.to_string_lossy() == "/" {
229-
triggers.iter().try_for_each(|batch| {
230-
batch.par_iter().try_for_each(|trigger| {
231-
(on_progress)(Progress {
232-
in_progress: batch.len(),
233-
completed: total + 1,
234-
items: batch
235-
.iter()
236-
.map(|t| match t.handler() {
237-
Handler::Run { run, .. } => run.clone(),
238-
Handler::Delete { .. } => "delete operation".to_owned(),
239-
})
240-
.collect(),
241-
});
242-
execute_trigger_directly(trigger.trigger())
243-
})
244-
})?;
230+
execute_triggers_directly(triggers, on_progress)?;
245231
} else {
246232
let isolation = Container::new(install.isolation_dir())
247233
.networking(false)
248234
.bind_rw(trigger_scope.host_path("etc"), "/etc")
249235
.bind_rw(trigger_scope.guest_path("usr"), "/usr")
250236
.work_dir("/");
251237

252-
isolation.run(|| {
253-
// Ensure runtime threads get dropped
254-
let rayon_runtime = rayon::ThreadPoolBuilder::new().build().expect("rayon runtime");
255-
rayon_runtime.install(|| {
256-
triggers.iter().try_for_each(|batch| {
257-
batch.par_iter().try_for_each(|trigger| {
258-
(on_progress)(Progress {
259-
in_progress: batch.len(),
260-
completed: total + 1,
261-
items: batch
262-
.iter()
263-
.map(|t| match t.handler() {
264-
Handler::Run { run, .. } => run.clone(),
265-
Handler::Delete { .. } => "delete operation".to_owned(),
266-
})
267-
.collect(),
268-
});
269-
execute_trigger_directly(trigger.trigger())
270-
})
271-
})
272-
})
273-
})?;
238+
isolation.run(|| execute_triggers_directly(triggers, &on_progress))?;
274239
}
275240
Ok(())
276241
}
@@ -286,6 +251,34 @@ impl TriggerRunner {
286251
}
287252

288253
/// Internal executor for triggers.
254+
fn execute_triggers_directly<P>(triggers: &[Vec<TriggerRunner>], on_progress: P) -> Result<(), Error>
255+
where
256+
P: Fn(Progress) + Send + Sync,
257+
{
258+
let rayon_runtime = rayon::ThreadPoolBuilder::new().build().expect("rayon runtime");
259+
let total = 0;
260+
rayon_runtime.install(|| {
261+
triggers.iter().try_for_each(|batch| {
262+
batch.par_iter().try_for_each(|trigger| {
263+
(on_progress)(Progress {
264+
in_progress: batch.len(),
265+
completed: total + 1,
266+
items: batch
267+
.iter()
268+
.map(|t| match t.handler() {
269+
Handler::Run { run, .. } => run.clone(),
270+
Handler::Delete { .. } => "delete operation".to_owned(),
271+
})
272+
.collect(),
273+
});
274+
execute_trigger_directly(trigger.trigger())
275+
})
276+
})
277+
})?;
278+
Ok(())
279+
}
280+
281+
/// Internal executor for individual triggers.
289282
fn execute_trigger_directly(trigger: &CompiledHandler) -> Result<(), Error> {
290283
match trigger.handler() {
291284
Handler::Run { run, args } => {

0 commit comments

Comments
 (0)