Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): respect --output-logs and outputLogs for persisting logs after TUI exits #8612

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ impl Display for OutputLogsMode {
}
}

impl From<OutputLogsMode> for turborepo_ui::tui::event::OutputLogs {
fn from(value: OutputLogsMode) -> Self {
match value {
OutputLogsMode::Full => turborepo_ui::tui::event::OutputLogs::Full,
OutputLogsMode::None => turborepo_ui::tui::event::OutputLogs::None,
OutputLogsMode::HashOnly => turborepo_ui::tui::event::OutputLogs::HashOnly,
OutputLogsMode::NewOnly => turborepo_ui::tui::event::OutputLogs::NewOnly,
OutputLogsMode::ErrorsOnly => turborepo_ui::tui::event::OutputLogs::ErrorsOnly,
}
}
}

#[derive(Copy, Clone, Debug, PartialEq, Serialize, ValueEnum)]
pub enum LogOrder {
#[serde(rename = "auto")]
Expand Down
67 changes: 43 additions & 24 deletions crates/turborepo-lib/src/run/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use turborepo_cache::{http::UploadMap, AsyncCache, CacheError, CacheHitMetadata,
use turborepo_repository::package_graph::PackageInfo;
use turborepo_scm::SCM;
use turborepo_telemetry::events::{task::PackageTaskEventBuilder, TrackedErrors};
use turborepo_ui::{color, ColorSelector, LogWriter, GREY, UI};
use turborepo_ui::{color, tui::event::CacheResult, ColorSelector, LogWriter, GREY, UI};

use crate::{
cli::OutputLogsMode,
Expand Down Expand Up @@ -57,7 +57,7 @@ pub struct RunCache {

/// Trait used to output cache information to user
pub trait CacheOutput {
fn status(&mut self, message: &str);
fn status(&mut self, message: &str, result: CacheResult);
fn error(&mut self, message: &str);
fn replay_logs(&mut self, log_file: &AbsoluteSystemPath) -> Result<(), turborepo_ui::Error>;
}
Expand Down Expand Up @@ -147,6 +147,10 @@ pub struct TaskCache {
}

impl TaskCache {
pub fn output_logs(&self) -> OutputLogsMode {
self.task_output_logs
}

/// Will read log file and write to output a line at a time
pub fn replay_log_file(&self, output: &mut impl CacheOutput) -> Result<(), Error> {
if self.log_file_path.exists() {
Expand All @@ -158,10 +162,13 @@ impl TaskCache {

pub fn on_error(&self, terminal_output: &mut impl CacheOutput) -> Result<(), Error> {
if self.task_output_logs == OutputLogsMode::ErrorsOnly {
terminal_output.status(&format!(
"cache miss, executing {}",
color!(self.ui, GREY, "{}", self.hash)
));
terminal_output.status(
&format!(
"cache miss, executing {}",
color!(self.ui, GREY, "{}", self.hash)
),
CacheResult::Miss,
);
self.replay_log_file(terminal_output)?;
}

Expand Down Expand Up @@ -202,10 +209,13 @@ impl TaskCache {
self.task_output_logs,
OutputLogsMode::None | OutputLogsMode::ErrorsOnly
) {
terminal_output.status(&format!(
"cache bypass, force executing {}",
color!(self.ui, GREY, "{}", self.hash)
));
terminal_output.status(
&format!(
"cache bypass, force executing {}",
color!(self.ui, GREY, "{}", self.hash)
),
CacheResult::Miss,
);
}

return Ok(None);
Expand Down Expand Up @@ -250,10 +260,13 @@ impl TaskCache {
self.task_output_logs,
OutputLogsMode::None | OutputLogsMode::ErrorsOnly
) {
terminal_output.status(&format!(
"cache miss, executing {}",
color!(self.ui, GREY, "{}", self.hash)
));
terminal_output.status(
&format!(
"cache miss, executing {}",
color!(self.ui, GREY, "{}", self.hash)
),
CacheResult::Miss,
);
}

return Ok(None);
Expand Down Expand Up @@ -298,19 +311,25 @@ impl TaskCache {

match self.task_output_logs {
OutputLogsMode::HashOnly | OutputLogsMode::NewOnly => {
terminal_output.status(&format!(
"cache hit{}, suppressing logs {}",
more_context,
color!(self.ui, GREY, "{}", self.hash)
));
terminal_output.status(
&format!(
"cache hit{}, suppressing logs {}",
more_context,
color!(self.ui, GREY, "{}", self.hash)
),
CacheResult::Hit,
);
}
OutputLogsMode::Full => {
debug!("log file path: {}", self.log_file_path);
terminal_output.status(&format!(
"cache hit{}, replaying logs {}",
more_context,
color!(self.ui, GREY, "{}", self.hash)
));
terminal_output.status(
&format!(
"cache hit{}, replaying logs {}",
more_context,
color!(self.ui, GREY, "{}", self.hash)
),
CacheResult::Hit,
);
self.replay_log_file(terminal_output)?;
}
// Note that if we're restoring from cache, the task succeeded
Expand Down
9 changes: 5 additions & 4 deletions crates/turborepo-lib/src/task_graph/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use turborepo_telemetry::events::{
generic::GenericEventBuilder, task::PackageTaskEventBuilder, EventBuilder, TrackedErrors,
};
use turborepo_ui::{
tui::{self, AppSender, TuiTask},
tui::{self, event::CacheResult, AppSender, TuiTask},
ColorSelector, OutputClient, OutputSink, OutputWriter, PrefixedUI, UI,
};
use which::which;
Expand Down Expand Up @@ -853,7 +853,8 @@ impl ExecContext {

if self.experimental_ui {
if let TaskOutput::UI(task) = output_client {
task.start();
let output_logs = self.task_cache.output_logs().into();
task.start(output_logs);
}
}

Expand Down Expand Up @@ -1088,10 +1089,10 @@ impl<W: Write> TaskCacheOutput<W> {
}

impl<W: Write> CacheOutput for TaskCacheOutput<W> {
fn status(&mut self, message: &str) {
fn status(&mut self, message: &str, result: CacheResult) {
match self {
TaskCacheOutput::Direct(direct) => direct.output(message),
TaskCacheOutput::UI(task) => task.status(message),
TaskCacheOutput::UI(task) => task.status(message, result),
}
}

Expand Down
59 changes: 40 additions & 19 deletions crates/turborepo-ui/src/tui/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const PANE_SIZE_RATIO: f32 = 3.0 / 4.0;
const FRAMERATE: Duration = Duration::from_millis(3);

use super::{
event::TaskResult, input, AppReceiver, Error, Event, InputOptions, TaskTable, TerminalPane,
event::{CacheResult, OutputLogs, TaskResult},
input, AppReceiver, Error, Event, InputOptions, TaskTable, TerminalPane,
};
use crate::tui::{
task::{Task, TasksByStatus},
Expand Down Expand Up @@ -133,7 +134,7 @@ impl<W> App<W> {
/// Mark the given task as started.
/// If planned, pulls it from planned tasks and starts it.
/// If finished, removes from finished and starts again as new task.
pub fn start_task(&mut self, task: &str) -> Result<(), Error> {
pub fn start_task(&mut self, task: &str, output_logs: OutputLogs) -> Result<(), Error> {
// Name of currently highlighted task.
// We will use this after the order switches.
let highlighted_task = self
Expand Down Expand Up @@ -171,6 +172,10 @@ impl<W> App<W> {
if !found_task {
return Err(Error::TaskNotFound { name: task.into() });
}
self.tasks
.get_mut(task)
.ok_or_else(|| Error::TaskNotFound { name: task.into() })?
.output_logs = Some(output_logs);

// If user hasn't interacted, keep highlighting top-most task in list.
if !self.has_user_scrolled {
Expand Down Expand Up @@ -213,6 +218,11 @@ impl<W> App<W> {
let running = self.tasks_by_status.running.remove(running_idx);
self.tasks_by_status.finished.push(running.finish(result));

self.tasks
.get_mut(task)
.ok_or_else(|| Error::TaskNotFound { name: task.into() })?
.task_result = Some(result);

// If user hasn't interacted, keep highlighting top-most task in list.
if !self.has_user_scrolled {
return Ok(());
Expand Down Expand Up @@ -278,14 +288,20 @@ impl<W> App<W> {
Ok(())
}

pub fn set_status(&mut self, task: String, status: String) -> Result<(), Error> {
pub fn set_status(
&mut self,
task: String,
status: String,
result: CacheResult,
) -> Result<(), Error> {
let task = self
.tasks
.get_mut(&task)
.ok_or_else(|| Error::TaskNotFound {
name: task.to_owned(),
})?;
task.status = Some(status);
task.cache_result = Some(result);
Ok(())
}
}
Expand Down Expand Up @@ -449,14 +465,18 @@ fn update(
event: Event,
) -> Result<Option<mpsc::SyncSender<()>>, Error> {
match event {
Event::StartTask { task } => {
app.start_task(&task)?;
Event::StartTask { task, output_logs } => {
app.start_task(&task, output_logs)?;
}
Event::TaskOutput { task, output } => {
app.process_output(&task, &output)?;
}
Event::Status { task, status } => {
app.set_status(task, status)?;
Event::Status {
task,
status,
result,
} => {
app.set_status(task, status, result)?;
}
Event::InternalStop => {
app.done = true;
Expand Down Expand Up @@ -526,6 +546,7 @@ fn view<W>(app: &mut App<W>, f: &mut Frame, cols: u16) {
#[cfg(test)]
mod test {
use super::*;
use crate::tui::event::CacheResult;

#[test]
fn test_scroll() {
Expand Down Expand Up @@ -564,10 +585,10 @@ mod test {
app.next();
assert_eq!(app.scroll.selected(), Some(1), "selected b");
assert_eq!(app.active_task(), "b", "selected b");
app.start_task("b").unwrap();
app.start_task("b", OutputLogs::Full).unwrap();
assert_eq!(app.scroll.selected(), Some(0), "b stays selected");
assert_eq!(app.active_task(), "b", "selected b");
app.start_task("a").unwrap();
app.start_task("a", OutputLogs::Full).unwrap();
assert_eq!(app.scroll.selected(), Some(0), "b stays selected");
assert_eq!(app.active_task(), "b", "selected b");
app.finish_task("a", TaskResult::Success).unwrap();
Expand All @@ -585,9 +606,9 @@ mod test {
app.next();
app.next();
// Start all tasks
app.start_task("b").unwrap();
app.start_task("a").unwrap();
app.start_task("c").unwrap();
app.start_task("b", OutputLogs::Full).unwrap();
app.start_task("a", OutputLogs::Full).unwrap();
app.start_task("c", OutputLogs::Full).unwrap();
assert_eq!(
app.tasks_by_status.task_name(0),
"b",
Expand All @@ -614,7 +635,7 @@ mod test {
);

// Restart b
app.start_task("b").unwrap();
app.start_task("b", OutputLogs::Full).unwrap();
assert_eq!(
(
app.tasks_by_status.task_name(1),
Expand All @@ -625,7 +646,7 @@ mod test {
);

// Restart a
app.start_task("a").unwrap();
app.start_task("a", OutputLogs::Full).unwrap();
assert_eq!(
(
app.tasks_by_status.task_name(0),
Expand All @@ -650,10 +671,10 @@ mod test {
assert_eq!(app.scroll.selected(), Some(2), "selected c");
assert_eq!(app.tasks_by_status.task_name(2), "c", "selected c");
// start c which moves it to "running" which is before "planned"
app.start_task("c").unwrap();
app.start_task("c", OutputLogs::Full).unwrap();
assert_eq!(app.scroll.selected(), Some(0), "selection stays on c");
assert_eq!(app.tasks_by_status.task_name(0), "c", "selected c");
app.start_task("a").unwrap();
app.start_task("a", OutputLogs::Full).unwrap();
assert_eq!(app.scroll.selected(), Some(0), "selection stays on c");
assert_eq!(app.tasks_by_status.task_name(0), "c", "selected c");
// c
Expand Down Expand Up @@ -682,8 +703,8 @@ mod test {
assert_eq!(app.scroll.selected(), Some(1), "selected b");
assert_eq!(app.tasks_by_status.task_name(1), "b", "selected b");
// start c which moves it to "running" which is before "planned"
app.start_task("a").unwrap();
app.start_task("b").unwrap();
app.start_task("a", OutputLogs::Full).unwrap();
app.start_task("b", OutputLogs::Full).unwrap();
app.insert_stdin("a", Some(Vec::new())).unwrap();
app.insert_stdin("b", Some(Vec::new())).unwrap();

Expand Down Expand Up @@ -735,7 +756,7 @@ mod test {
assert_eq!(app.scroll.selected(), Some(1), "selected b");
assert_eq!(app.tasks_by_status.task_name(1), "b", "selected b");
// set status for a
app.set_status("a".to_string(), "building".to_string())
app.set_status("a".to_string(), "building".to_string(), CacheResult::Hit)
.unwrap();

assert_eq!(
Expand Down
22 changes: 22 additions & 0 deletions crates/turborepo-ui/src/tui/event.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub enum Event {
StartTask {
task: String,
output_logs: OutputLogs,
},
TaskOutput {
task: String,
Expand All @@ -13,6 +14,7 @@ pub enum Event {
Status {
task: String,
status: String,
result: CacheResult,
},
Stop(std::sync::mpsc::SyncSender<()>),
// Stop initiated by the TUI itself
Expand Down Expand Up @@ -42,6 +44,26 @@ pub enum TaskResult {
Failure,
}

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
pub enum CacheResult {
Hit,
Miss,
}

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
pub enum OutputLogs {
// Entire task output is persisted after run
Full,
// None of a task output is persisted after run
None,
// Only the status line of a task is persisted
HashOnly,
// Output is only persisted if it is a cache miss
NewOnly,
// Output is only persisted if the task failed
ErrorsOnly,
}

#[cfg(test)]
mod test {
use super::*;
Expand Down
Loading
Loading