Skip to content
Draft
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
3 changes: 3 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2025-05-15 - Redundant data cloning in command execution
**Learning:** In the command execution module, cache hits were causing redundant clones of the entire captured stdout/stderr. By using `Arc<[u8]>` instead of `Vec<u8>` in `CommandOutput`, we can avoid copying large outputs when retrieving from the cache or when cloning the output object. Additionally, `fingerprint()` was being called redundantly on the failure path, which involves cloning all arguments and environment variables.
**Action:** Use `Arc` for shared data in performance-critical structures that are frequently cloned. Avoid redundant calculations of complex objects like command fingerprints by passing them down the call stack.
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2025-05-15 - No security issues found
**Vulnerability:** None identified in this session.
**Learning:** The bootstrap build system is relatively self-contained and doesn't handle sensitive user data or external network requests in a way that exposed common vulnerabilities.
**Prevention:** Continue to monitor for insecure use of `Command` and potential path traversal in build steps.
44 changes: 29 additions & 15 deletions src/bootstrap/src/utils/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,8 @@ pub fn command<S: AsRef<OsStr>>(program: S) -> BootstrapCommand {
#[derive(Clone, PartialEq)]
pub struct CommandOutput {
status: CommandStatus,
stdout: Option<Vec<u8>>,
stderr: Option<Vec<u8>>,
stdout: Option<Arc<[u8]>>,
stderr: Option<Arc<[u8]>>,
}

impl CommandOutput {
Expand All @@ -456,11 +456,11 @@ impl CommandOutput {
status: CommandStatus::DidNotStartOrFinish,
stdout: match stdout {
OutputMode::Print => None,
OutputMode::Capture => Some(vec![]),
OutputMode::Capture => Some(Arc::from(vec![])),
},
stderr: match stderr {
OutputMode::Print => None,
OutputMode::Capture => Some(vec![]),
OutputMode::Capture => Some(Arc::from(vec![])),
},
}
}
Expand All @@ -471,11 +471,11 @@ impl CommandOutput {
status: CommandStatus::Finished(output.status),
stdout: match stdout {
OutputMode::Print => None,
OutputMode::Capture => Some(output.stdout),
OutputMode::Capture => Some(Arc::from(output.stdout)),
},
stderr: match stderr {
OutputMode::Print => None,
OutputMode::Capture => Some(output.stderr),
OutputMode::Capture => Some(Arc::from(output.stderr)),
},
}
}
Expand Down Expand Up @@ -503,14 +503,17 @@ impl CommandOutput {
#[must_use]
pub fn stdout(&self) -> String {
String::from_utf8(
self.stdout.clone().expect("Accessing stdout of a command that did not capture stdout"),
self.stdout
.as_ref()
.expect("Accessing stdout of a command that did not capture stdout")
.to_vec(),
)
.expect("Cannot parse process stdout as UTF-8")
}

#[must_use]
pub fn stdout_if_present(&self) -> Option<String> {
self.stdout.as_ref().and_then(|s| String::from_utf8(s.clone()).ok())
self.stdout.as_ref().and_then(|s| String::from_utf8(s.to_vec()).ok())
}

#[must_use]
Expand All @@ -521,23 +524,26 @@ impl CommandOutput {
#[must_use]
pub fn stderr(&self) -> String {
String::from_utf8(
self.stderr.clone().expect("Accessing stderr of a command that did not capture stderr"),
self.stderr
.as_ref()
.expect("Accessing stderr of a command that did not capture stderr")
.to_vec(),
)
.expect("Cannot parse process stderr as UTF-8")
}

#[must_use]
pub fn stderr_if_present(&self) -> Option<String> {
self.stderr.as_ref().and_then(|s| String::from_utf8(s.clone()).ok())
self.stderr.as_ref().and_then(|s| String::from_utf8(s.to_vec()).ok())
}
}

impl Default for CommandOutput {
fn default() -> Self {
Self {
status: CommandStatus::Finished(ExitStatus::default()),
stdout: Some(vec![]),
stderr: Some(vec![]),
stdout: Some(Arc::from(vec![])),
stderr: Some(Arc::from(vec![])),
}
}
}
Expand Down Expand Up @@ -826,8 +832,15 @@ impl<'a> DeferredCommand<'a> {
} => {
let exec_ctx = exec_ctx.as_ref();

let output =
Self::finish_process(process, command, stdout, stderr, executed_at, exec_ctx);
let output = Self::finish_process(
process,
command,
stdout,
stderr,
executed_at,
exec_ctx,
&fingerprint,
);

#[cfg(feature = "tracing")]
drop(_span_guard);
Expand All @@ -852,6 +865,7 @@ impl<'a> DeferredCommand<'a> {
stderr: OutputMode,
executed_at: &'a std::panic::Location<'a>,
exec_ctx: &ExecutionContext,
fingerprint: &CommandFingerprint,
) -> CommandOutput {
use std::fmt::Write;

Expand Down Expand Up @@ -904,7 +918,7 @@ impl<'a> DeferredCommand<'a> {
let command_str = if exec_ctx.is_verbose() {
format!("{command:?}")
} else {
command.fingerprint().format_short_cmd()
fingerprint.format_short_cmd()
};
let action = match fail_reason {
FailureReason::FailedAtRuntime(e) => {
Expand Down