Skip to content

Commit

Permalink
fix(tui): replay all logs sent to tui with forced crlf (vercel#9962)
Browse files Browse the repository at this point in the history
### Description

If a task is run without a TTY (e.g. in CI) the logs will be created
with LF (`\n`) line endings. The TUI assumes that logs are produced
hooked up to a TTY where CRLF (`\r\n`) is used to return the cursor to
the first column and move to the next row (`\n` behavior in non-TTY).

This PR adds a log replay method that replays logs with CRLF line
endings regardless of what is in the actual file.

Future PR is to refactor this to share code with the standard log replay
as I didn't have time to tackle this right at this moment.

### Testing Instructions

Added quick unit test to verify this swaps out LF for CRLF and is a noop
if the logs already use CRLF.

Manual Test:
First populate the logs with a non-TTY run:
```
[0 olszewski@macbookpro] /tmp/tui-test $ turbo @repo/ui#build > /dev/null
```

Using `turbo@2.4.2`:
<img width="797" alt="Screenshot 2025-02-13 at 6 24 47 PM"
src="https://github.com/user-attachments/assets/45439888-0098-4bb7-92b8-65eb2789cfbf"
/>

Using `turbo_dev` from this PR
<img width="762" alt="Screenshot 2025-02-13 at 6 26 04 PM"
src="https://github.com/user-attachments/assets/bd00c5dd-ebd8-41eb-9bd8-4f597925676f"
/>
  • Loading branch information
chris-olszewski authored and joshnuss committed Feb 15, 2025
1 parent d322f36 commit e75eb2a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 3 deletions.
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/task_graph/visitor/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl<W: Write> CacheOutput for TaskCacheOutput<W> {
let writer = direct.output_prefixed_writer();
turborepo_ui::replay_logs(writer, log_file)
}
TaskCacheOutput::UI(task) => turborepo_ui::replay_logs(task, log_file),
TaskCacheOutput::UI(task) => turborepo_ui::replay_logs_with_crlf(task, log_file),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use thiserror::Error;
pub use crate::{
color_selector::ColorSelector,
line::LineWriter,
logs::{replay_logs, LogWriter},
logs::{replay_logs, replay_logs_with_crlf, LogWriter},
output::{OutputClient, OutputClientBehavior, OutputSink, OutputWriter},
prefixed::{PrefixedUI, PrefixedWriter},
tui::{TaskTable, TerminalPane},
Expand Down
75 changes: 74 additions & 1 deletion crates/turborepo-ui/src/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,48 @@ pub fn replay_logs<W: Write>(
Ok(())
}

/// Replay logs, but enforce crlf line endings
// TODO: refactor to share code with `replay_logs`
pub fn replay_logs_with_crlf<W: Write>(
mut output: W,
log_file_name: &AbsoluteSystemPath,
) -> Result<(), Error> {
debug!("start replaying logs");

let log_file = File::open(log_file_name).map_err(|err| {
warn!("error opening log file: {:?}", err);
Error::CannotReadLogs(err)
})?;

let mut log_reader = BufReader::new(log_file);

let mut buffer = Vec::new();
loop {
let num_bytes = log_reader
.read_until(b'\n', &mut buffer)
.map_err(Error::CannotReadLogs)?;
if num_bytes == 0 {
break;
}

let line_without_lf = buffer.strip_suffix(b"\n").unwrap_or(&buffer);
let line_without_crlf = line_without_lf
.strip_suffix(b"\r")
.unwrap_or(line_without_lf);

output
.write_all(line_without_crlf)
.map_err(Error::CannotReadLogs)?;
output.write_all(b"\r\n").map_err(Error::CannotReadLogs)?;

buffer.clear();
}

debug!("finish replaying logs");

Ok(())
}

#[cfg(test)]
mod tests {
use std::{fs, io::Write};
Expand All @@ -128,7 +170,8 @@ mod tests {
use turbopath::AbsoluteSystemPathBuf;

use crate::{
logs::replay_logs, ColorConfig, LogWriter, PrefixedUI, PrefixedWriter, BOLD, CYAN,
logs::replay_logs, replay_logs_with_crlf, ColorConfig, LogWriter, PrefixedUI,
PrefixedWriter, BOLD, CYAN,
};

#[test]
Expand Down Expand Up @@ -207,4 +250,34 @@ mod tests {
assert_eq!(output, [b'>', 0, 159, 146, 150, b'\n']);
Ok(())
}

#[test]
fn test_replay_logs_crlf() -> Result<()> {
let dir = tempdir()?;
let log_file_path = AbsoluteSystemPathBuf::try_from(dir.path().join("test.txt"))?;
fs::write(&log_file_path, "\none fish\ntwo fish\nred fish\nblue fish")?;
let mut output = Vec::new();
replay_logs_with_crlf(&mut output, &log_file_path)?;
let output_str = std::str::from_utf8(&output)?;
assert_eq!(
output_str,
"\r\none fish\r\ntwo fish\r\nred fish\r\nblue fish\r\n"
);

Ok(())
}

#[test]
fn test_replay_logs_crlf_noop() -> Result<()> {
let dir = tempdir()?;
let log_file_path = AbsoluteSystemPathBuf::try_from(dir.path().join("test.txt"))?;
let contents = "\r\none fish\r\ntwo fish\r\nred fish\r\nblue fish\r\n";
fs::write(&log_file_path, contents)?;
let mut output = Vec::new();
replay_logs_with_crlf(&mut output, &log_file_path)?;
let output_str = std::str::from_utf8(&output)?;
assert_eq!(output_str, contents,);

Ok(())
}
}

0 comments on commit e75eb2a

Please sign in to comment.