Skip to content

Commit 0312001

Browse files
abelbriggs1abelbriggs1
authored
CLI: Fix silent interactive TUI hang when project is inferred from CWD (#414)
* core/jobs: Always wake after running a job If a job runs into an error case before calling `update_status()`, the job will never be considered 'completed' and the polling thread will never wake. Handle this by always waking at the end of the job. * cli/cmd/diff: Show job failures in the TUI Previously, if a job failed, the terminal would not give any feedback, showing a blank display. Handle this by printing the job status instead. * cli/cmd/diff: Route derived `project` dir to interactive TUI The CLI naturally infers the location of the project file/directory via either CLI arguments or the current working directory. `run_interactive()`, however, always populates the project dir from the `-p` argument. If the project dir is inferred via CWD, this fails. Route the inferred project dir from the caller instead.
1 parent 49b8698 commit 0312001

3 files changed

Lines changed: 58 additions & 13 deletions

File tree

‎objdiff-cli/src/cmd/diff.rs‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,12 @@ pub struct Args {
8181
}
8282

8383
pub fn run(args: Args) -> Result<()> {
84-
let (target_path, base_path, project_config, unit_options, symbol_mappings) =
84+
let (project_dir, target_path, base_path, project_config, unit_options, symbol_mappings) =
8585
match (&args.target, &args.base, &args.project, &args.unit) {
8686
(Some(_), Some(_), None, None)
8787
| (Some(_), None, None, None)
8888
| (None, Some(_), None, None) => {
89-
(args.target.clone(), args.base.clone(), None, None, BTreeMap::new())
89+
(None, args.target.clone(), args.base.clone(), None, None, BTreeMap::new())
9090
}
9191
(None, None, p, u) => {
9292
let project = match p {
@@ -168,7 +168,14 @@ pub fn run(args: Args) -> Result<()> {
168168
let target_path = object.target_path.clone();
169169
let base_path = object.base_path.clone();
170170
let symbol_mappings = object.symbol_mappings.clone();
171-
(target_path, base_path, Some(project_config), unit_options, symbol_mappings)
171+
(
172+
Some(project),
173+
target_path,
174+
base_path,
175+
Some(project_config),
176+
unit_options,
177+
symbol_mappings,
178+
)
172179
}
173180
_ => bail!("Either target and base or project and unit must be specified"),
174181
};
@@ -184,7 +191,15 @@ pub fn run(args: Args) -> Result<()> {
184191
&symbol_mappings,
185192
)
186193
} else {
187-
run_interactive(args, target_path, base_path, project_config, unit_options, symbol_mappings)
194+
run_interactive(
195+
args,
196+
project_dir,
197+
target_path,
198+
base_path,
199+
project_config,
200+
unit_options,
201+
symbol_mappings,
202+
)
188203
}
189204
}
190205

@@ -345,6 +360,28 @@ impl AppState {
345360
fn check_jobs(&mut self) -> Result<bool> {
346361
let mut redraw = false;
347362
self.jobs.collect_results();
363+
// Surface job errors (e.g. a failed build) instead of silently showing nothing.
364+
for job in self.jobs.iter_mut() {
365+
let Some((title, error)) = job
366+
.context
367+
.status
368+
.write()
369+
.ok()
370+
.and_then(|mut s| s.error.take().map(|e| (s.title.clone(), e)))
371+
else {
372+
continue;
373+
};
374+
let status = BuildStatus {
375+
success: false,
376+
stdout: format!("Job \"{title}\" failed"),
377+
stderr: format!("{error:#}"),
378+
..Default::default()
379+
};
380+
self.left_status = Some(status.clone());
381+
self.right_status = Some(status);
382+
redraw = true;
383+
}
384+
self.jobs.clear_finished();
348385
for result in mem::take(&mut self.jobs.results) {
349386
match result {
350387
JobResult::None => unreachable!("Unexpected JobResult::None"),
@@ -378,6 +415,7 @@ impl Wake for TermWaker {
378415

379416
fn run_interactive(
380417
args: Args,
418+
project_dir: Option<Utf8PlatformPathBuf>,
381419
target_path: Option<Utf8PlatformPathBuf>,
382420
base_path: Option<Utf8PlatformPathBuf>,
383421
project_config: Option<ProjectConfig>,
@@ -396,7 +434,7 @@ fn run_interactive(
396434
let mut state = AppState {
397435
jobs: Default::default(),
398436
waker: Default::default(),
399-
project_dir: args.project.clone(),
437+
project_dir,
400438
project_config,
401439
target_path,
402440
base_path,

‎objdiff-core/src/build/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::process::Command;
44

55
use typed_path::{Utf8PlatformPathBuf, Utf8UnixPath};
66

7+
#[derive(Clone)]
78
pub struct BuildStatus {
89
pub success: bool,
910
pub cmdline: String,

‎objdiff-core/src/jobs/mod.rs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -203,16 +203,22 @@ fn start_job(
203203
error: None,
204204
}));
205205
let context = JobContext { status: status.clone(), waker: waker.clone() };
206-
let context_inner = JobContext { status: status.clone(), waker };
206+
let context_inner = JobContext { status: status.clone(), waker: waker.clone() };
207207
let (tx, rx) = std::sync::mpsc::channel();
208-
let handle = std::thread::spawn(move || match run(context_inner, rx) {
209-
Ok(state) => state,
210-
Err(e) => {
211-
if let Ok(mut w) = status.write() {
212-
w.error = Some(e);
208+
let handle = std::thread::spawn(move || {
209+
let result = match run(context_inner, rx) {
210+
Ok(state) => state,
211+
Err(e) => {
212+
if let Ok(mut w) = status.write() {
213+
w.error = Some(e);
214+
}
215+
JobResult::None
213216
}
214-
JobResult::None
215-
}
217+
};
218+
// Always wake on completion, so the frontend notices jobs that finished
219+
// (or failed) without reporting any progress.
220+
waker.wake();
221+
result
216222
});
217223
let id = JOB_ID.fetch_add(1, Ordering::Relaxed);
218224
JobState { id, kind, handle: Some(handle), context, cancel: tx }

0 commit comments

Comments
 (0)