fix(ui): respect --output-logs and outputLogs for persisting logs…

… after TUI exits (#8612) ### Description This hooks up the `--output-logs` flag and `outputLogs` in `turbo.json` to the logic that persists task output. Previously these values were not respected and task output was always printed. ### Testing Instructions `new-only` should only print cache status on a cache hit, but everything on a miss. ``` [0 olszewski@chriss-mbp] /tmp/tui-output-logs $ turbo_dev --skip-infer build --output-logs=new-only --no-daemon 12:46:14 [2/570] • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running build in 5 packages • Remote caching disabled ┌ web#build > cache miss, executing e45fd0a9bad40e92 │ │ > web@0.1.0 build /private/tmp/tui-output-logs/apps/web │ > next build │ │ ▲ Next.js 15.0.0-rc.0 │ │ Creating an optimized production build ... │ ✓ Compiled successfully │ ✓ Linting and checking validity of types │ ✓ Collecting page data │ ✓ Generating static pages (5/5) │ ✓ Collecting build traces │ ✓ Finalizing page optimization │ │ Route (app) Size First Load JS │ ┌ ○ / 5.53 kB 94.4 kB │ └ ○ /_not-found 895 B 89.8 kB │ + First Load JS shared by all 88.9 kB │ ├ chunks/207a5ba7-ac24752941b7dbc9.js 51.5 kB │ ├ chunks/479-8568a017162d290a.js 35.5 kB │ └ other shared chunks (total) 1.88 kB │ │ │ ○ (Static) prerendered as static content └────> ┌ docs#build > cache miss, executing 5176bc1351e4bb6b │ │ > docs@0.1.0 build /private/tmp/tui-output-logs/apps/docs │ > next build │ │ ▲ Next.js 15.0.0-rc.0 │ │ Creating an optimized production build ... │ ✓ Compiled successfully │ ✓ Linting and checking validity of types │ ✓ Collecting page data │ ✓ Generating static pages (5/5) │ ✓ Collecting build traces │ ✓ Finalizing page optimization │ │ Route (app) Size First Load JS │ ┌ ○ / 5.53 kB 94.4 kB │ └ ○ /_not-found 895 B 89.8 kB │ + First Load JS shared by all 88.9 kB │ ├ chunks/207a5ba7-ac24752941b7dbc9.js 51.5 kB │ ├ chunks/479-8568a017162d290a.js 35.5 kB │ └ other shared chunks (total) 1.88 kB │ │ │ ○ (Static) prerendered as static content └────> Tasks: 2 successful, 2 total Cached: 0 cached, 2 total Time: 8.692s [0 olszewski@chriss-mbp] /tmp/tui-output-logs $ turbo_dev --skip-infer build --output-logs=new-only --no-daemon • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running build in 5 packages • Remote caching disabled web#build > cache hit, suppressing logs e45fd0a9bad40e92 docs#build > cache hit, suppressing logs 5176bc1351e4bb6b Tasks: 2 successful, 2 total Cached: 2 cached, 2 total Time: 110ms >>> FULL TURBO ``` `hash-only` should only persist the status line: ``` [0 olszewski@chriss-mbp] /tmp/tui-output-logs $ turbo_dev --skip-infer build --output-logs=hash-only --no-daemon • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running build in 5 packages • Remote caching disabled docs#build > cache hit, suppressing logs 5176bc1351e4bb6b web#build > cache hit, suppressing logs e45fd0a9bad40e92 Tasks: 2 successful, 2 total Cached: 2 cached, 2 total Time: 138ms >>> FULL TURBO [0 olszewski@chriss-mbp] /tmp/tui-output-logs $ turbo_dev --skip-infer build --output-logs=hash-only --no-daemon --force • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running build in 5 packages • Remote caching disabled web#build > cache bypass, force executing e45fd0a9bad40e92 docs#build > cache bypass, force executing 5176bc1351e4bb6b Tasks: 2 successful, 2 total Cached: 0 cached, 2 total Time: 10.807s ``` `errors-only` should only output errors: ``` [0 olszewski@chriss-mbp] /tmp/tui-output-logs $ turbo_dev --skip-infer build --output-logs=errors-only --no-daemon --force --continue • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running build in 5 packages • Remote caching disabled ┌ web#build > cache miss, executing a9ef756a1783f051 │ │ > web@0.1.0 build /private/tmp/tui-output-logs/apps/web │ > sleep 1; echo oops; exit 1 │ │ oops │ ELIFECYCLE Command failed with exit code 1. │ │ command finished with error, but continuing... └────> web#build: command (/private/tmp/tui-output-logs/apps/web) /Users/olszewski/.nvm/versions/node/v20.11.1/bin/pnpm run build exited (1) Tasks: 1 successful, 2 total Cached: 0 cached, 2 total Time: 8.07s Failed: web#build ```
vercel · Jul 9, 2024 · b9df74c · b9df74c
1 parent adcb85c
commit b9df74c
Show file tree

Hide file tree

Showing 7 changed files with 189 additions and 62 deletions.
diff --git a/crates/turborepo-lib/src/cli/mod.rs b/crates/turborepo-lib/src/cli/mod.rs
@@ -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")]

diff --git a/crates/turborepo-lib/src/run/cache.rs b/crates/turborepo-lib/src/run/cache.rs
@@ -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,
@@ -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>;
 }
@@ -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() {
@@ -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)?;
         }
 
@@ -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);
@@ -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);
@@ -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

diff --git a/crates/turborepo-lib/src/task_graph/visitor.rs b/crates/turborepo-lib/src/task_graph/visitor.rs
@@ -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;
@@ -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);
             }
         }
 
@@ -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),
         }
     }
 

diff --git a/crates/turborepo-ui/src/tui/app.rs b/crates/turborepo-ui/src/tui/app.rs
@@ -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},
@@ -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
@@ -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 {
@@ -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(());
@@ -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(())
     }
 }
@@ -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;
@@ -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() {
@@ -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();
@@ -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",
@@ -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),
@@ -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),
@@ -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
@@ -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();
 
@@ -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!(

diff --git a/crates/turborepo-ui/src/tui/event.rs b/crates/turborepo-ui/src/tui/event.rs
@@ -1,6 +1,7 @@
 pub enum Event {
     StartTask {
         task: String,
+        output_logs: OutputLogs,
     },
     TaskOutput {
         task: String,
@@ -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
@@ -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::*;