Skip to content

Commit

Permalink
fix: respect output mode 'none' even when caching is disabled (#9670)
Browse files Browse the repository at this point in the history
### Description

This is a fresh PR for #7900, which sat too long and got stale. I
accidentally closed the PR with no commits after force-pushing so I'm
opening this for simplicity's sake.

@aholland deserves all credit here for finding this issue and proposing
a fix.

### Testing Instructions

- Existing CI should catch me on regressions 🙏
- Added a test
- Did some manual testing. Things look to be doing what we want.

---------

Co-authored-by: Chris Olszewski <[email protected]>
  • Loading branch information
anthonyshew and chris-olszewski authored Jan 10, 2025
1 parent a21c30f commit 1a40277
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
17 changes: 7 additions & 10 deletions crates/turborepo-lib/src/run/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,15 @@ impl TaskCache {
pub fn output_writer<W: Write>(&self, writer: W) -> Result<LogWriter<W>, Error> {
let mut log_writer = LogWriter::default();

if self.caching_disabled || self.run_cache.writes_disabled {
log_writer.with_writer(writer);
return Ok(log_writer);
if !self.caching_disabled && !self.run_cache.writes_disabled {
log_writer.with_log_file(&self.log_file_path)?;
}

log_writer.with_log_file(&self.log_file_path)?;

if !matches!(
self.task_output_logs,
OutputLogsMode::None | OutputLogsMode::HashOnly | OutputLogsMode::ErrorsOnly
) {
log_writer.with_writer(writer);
match self.task_output_logs {
OutputLogsMode::None | OutputLogsMode::HashOnly | OutputLogsMode::ErrorsOnly => {}
OutputLogsMode::Full | OutputLogsMode::NewOnly => {
log_writer.with_writer(writer);
}
}

Ok(log_writer)
Expand Down
11 changes: 8 additions & 3 deletions crates/turborepo-ui/src/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,14 @@ impl<W: Write> Write for LogWriter<W> {
(Some(log_file), None) => log_file.write(buf),
(None, Some(prefixed_writer)) => prefixed_writer.write(buf),
(None, None) => {
// Should this be an error or even a panic?
debug!("no log file or prefixed writer");
Ok(0)
debug!(
"No log file or prefixed writer to write to. This should only happen when \
both caching is disabled and output logs are set to none."
);

// Returning the buffer's length so callers don't think this is a failure to
// create the buffer
Ok(buf.len())
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Setup
$ . ${TESTDIR}/../../../helpers/setup_integration_test.sh run_logging

$ ${TURBO} run build --cache=local:,remote: --output-logs=none
WARNING no caches are enabled
\xe2\x80\xa2 Packages in scope: app-a (esc)
\xe2\x80\xa2 Running build in 1 packages (esc)
\xe2\x80\xa2 Remote caching disabled (esc)

Tasks: 1 successful, 1 total
Cached: 0 cached, 1 total
Time:\s*[\.0-9]+m?s (re)

0 comments on commit 1a40277

Please sign in to comment.