Skip to content

Commit d26f9cf

Browse files
authored
Merge pull request #2007 from cachix/fix-2006
devenv: fix execing into a shell with `devenv shell`
2 parents e2a9d0d + 6e08dcc commit d26f9cf

File tree

3 files changed

+34
-23
lines changed

3 files changed

+34
-23
lines changed

devenv-run-tests/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ async fn run_tests_in_directory(args: &Args) -> Result<Vec<TestResult>> {
207207
if PathBuf::from(setup_script).exists() {
208208
eprintln!(" Running {setup_script}");
209209
let output = devenv
210-
.exec_in_shell(format!("./{setup_script}"), &[])
210+
.run_in_shell(format!("./{setup_script}"), &[])
211211
.await?;
212212
if !output.status.success() {
213213
return Err(miette::miette!(

devenv/src/devenv.rs

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -380,21 +380,42 @@ impl Devenv {
380380
Ok(shell_cmd)
381381
}
382382

383-
/// Exec into the shell.
384-
/// This method does not return after calling `exec`.
385-
pub async fn shell(self) -> Result<()> {
386-
let shell_cmd = self.prepare_shell(&None, &[]).await?;
383+
/// Launch an interactive shell (uses exec, never returns).
384+
pub async fn shell(&self) -> Result<()> {
385+
self.exec_in_shell(None, &[]).await
386+
}
387+
388+
/// Execute a command by replacing the current process using exec.
389+
///
390+
/// This method accepts `Option<String>` for the command to support both:
391+
/// - Interactive shell: `exec_in_shell(None, &[])`
392+
/// - Command execution: `exec_in_shell(Some(cmd), args)`
393+
///
394+
/// **Important**: This function never returns `Ok(())` on success because `exec()`
395+
/// replaces the current process. The `Result<()>` return type only represents
396+
/// potential errors during setup or if `exec()` fails to start the new process.
397+
/// On successful exec, this function never returns.
398+
pub async fn exec_in_shell(&self, cmd: Option<String>, args: &[String]) -> Result<()> {
399+
let shell_cmd = self.prepare_shell(&cmd, args).await?;
387400
info!(devenv.is_user_message = true, "Entering shell");
388401
let err = shell_cmd.into_std().exec();
389-
bail!("Failed to execute shell: {}", err);
402+
403+
let cmd_context = match &cmd {
404+
Some(c) => format!("command '{}'", c),
405+
None => "interactive shell".to_string(),
406+
};
407+
bail!("Failed to exec into shell with {}: {}", cmd_context, err);
390408
}
391409

392-
pub async fn exec_in_shell(&self, cmd: String, args: &[String]) -> Result<Output> {
410+
/// Run a command and return the output.
411+
///
412+
/// This method accepts `String` (not `Option<String>`) because it's specifically
413+
/// designed for running commands and capturing their output. Unlike `exec_in_shell`,
414+
/// this method always requires a command and uses `spawn` + `wait_with_output`
415+
/// to return control to the caller with the command's output.
416+
pub async fn run_in_shell(&self, cmd: String, args: &[String]) -> Result<Output> {
393417
let mut shell_cmd = self.prepare_shell(&Some(cmd), args).await?;
394-
let span = info_span!(
395-
"executing_in_shell",
396-
devenv.user_message = "Executing in shell"
397-
);
418+
let span = info_span!("running_in_shell", devenv.user_message = "Running in shell");
398419
// Note that tokio's `output()` always configures stdout/stderr as pipes.
399420
// Use `spawn` + `wait_with_output` instead.
400421
let proc = shell_cmd

devenv/src/main.rs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@ use devenv::{
44
config, log, Devenv,
55
};
66
use miette::{IntoDiagnostic, Result, WrapErr};
7-
use std::env;
8-
use std::{
9-
os::unix::process::CommandExt,
10-
process::{self, Command},
11-
};
7+
use std::{env, os::unix::process::CommandExt, process::Command};
128
use tempfile::TempDir;
139
use tracing::{info, warn};
1410

@@ -100,13 +96,7 @@ async fn main() -> Result<()> {
10096

10197
match command {
10298
Commands::Shell { cmd, ref args } => match cmd {
103-
Some(cmd) => {
104-
let output = devenv.exec_in_shell(cmd, args).await?;
105-
if !output.status.success() {
106-
process::exit(output.status.code().unwrap_or(1));
107-
}
108-
Ok(())
109-
}
99+
Some(cmd) => devenv.exec_in_shell(Some(cmd), args).await,
110100
None => devenv.shell().await,
111101
},
112102
Commands::Test { .. } => devenv.test().await,

0 commit comments

Comments
 (0)