Add interactive mode to cross-util run

lumasepa · lumasepa · commit 1aab4cf83584 · 2023-06-26T09:08:11.000+01:00
diff --git a/.changes/1280.json b/.changes/1280.json
@@ -0,0 +1,6 @@
+{
+  "description": "Allow to run arbitrary commands in containers using `cross-util run ...`",
+  "issues": [1266],
+  "type": "added",
+  "breaking": false
+}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,8 +7,6 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased] - ReleaseDate
 
-- #1266 Allow to run arbitrary commands in containers using `cross-util run --target <target> --command <command>`
-
 ## [v0.2.5] - 2023-02-04
 
 ## Fixed
diff --git a/src/bin/commands/run.rs b/src/bin/commands/run.rs
@@ -1,9 +1,8 @@
 use clap::Args as ClapArgs;
 use cross::config::Config;
 use cross::shell::{MessageInfo, Verbosity};
-use cross::SafeCommand;
 use cross::{
-    cargo_metadata_with_args, cli::Args, docker, rustc, setup, toml, CargoVariant, CrossSetup,
+    cargo_metadata_with_args, cli::Args, docker, rustc, setup, toml, CommandVariant, CrossSetup,
     Target,
 };
 use eyre::Context;
@@ -22,11 +21,14 @@ pub struct Run {
     /// Container engine (such as docker or podman).
     #[clap(long)]
     pub engine: Option<String>,
-
+    /// Target
     #[clap(short, long)]
     pub target: String,
-
-    #[clap(short, long)]
+    /// Interactive session
+    #[clap(short, long, default_value = "false")]
+    pub interactive: bool,
+    /// Command to run, will be run in a shell
+    #[clap(last = true)]
     pub command: String,
 }
 
@@ -76,14 +78,20 @@ impl Run {
             let image = image.to_definite_with(&engine, msg_info);
 
             let paths = docker::DockerPaths::create(&engine, metadata, cwd, toolchain, msg_info)?;
-            let options =
-                docker::DockerOptions::new(engine, target, config, image, CargoVariant::None, None);
+            let options = docker::DockerOptions::new(
+                engine,
+                target,
+                config,
+                image,
+                CommandVariant::Shell,
+                None,
+                self.interactive,
+            );
 
-            let command = SafeCommand::new("sh");
             let mut args = vec![String::from("-c")];
             args.push(self.command.clone());
 
-            docker::run(options, paths, command, &args, None, msg_info)
+            docker::run(options, paths, &args, None, msg_info)
                 .wrap_err("could not run container")?;
         }
 
diff --git a/src/docker/custom.rs b/src/docker/custom.rs
@@ -85,7 +85,7 @@ impl<'a> Dockerfile<'a> {
         build_args: impl IntoIterator<Item = (impl AsRef<str>, impl AsRef<str>)>,
         msg_info: &mut MessageInfo,
     ) -> Result<String> {
-        let uses_zig = options.cargo_variant.uses_zig();
+        let uses_zig = options.command_variant.uses_zig();
         let mut docker_build = options.engine.command();
         docker_build.invoke_build_command();
         docker_build.disable_scan_suggest();
diff --git a/src/docker/local.rs b/src/docker/local.rs
@@ -5,7 +5,7 @@ use std::sync::atomic::Ordering;
 
 use super::shared::*;
 use crate::errors::Result;
-use crate::extensions::{CommandExt, SafeCommand};
+use crate::extensions::CommandExt;
 use crate::file::{PathExt, ToUtf8};
 use crate::shell::{MessageInfo, Stream};
 use eyre::Context;
@@ -29,14 +29,14 @@ fn mount(
 pub(crate) fn run(
     options: DockerOptions,
     paths: DockerPaths,
-    mut cmd: SafeCommand,
     args: &[String],
     msg_info: &mut MessageInfo,
 ) -> Result<ExitStatus> {
     let engine = &options.engine;
     let toolchain_dirs = paths.directories.toolchain_directories();
     let package_dirs = paths.directories.package_directories();
 
+    let mut cmd = options.command_variant.safe_command();
     cmd.args(args);
 
     let mut docker = engine.subcommand("run");
@@ -134,6 +134,11 @@ pub(crate) fn run(
     if io::Stdin::is_atty() && io::Stdout::is_atty() && io::Stderr::is_atty() {
         docker.arg("-t");
     }
+
+    if options.interactive {
+        docker.arg("-i");
+    }
+
     let mut image_name = options.image.name.clone();
     if options.needs_custom_image() {
         image_name = options
diff --git a/src/docker/mod.rs b/src/docker/mod.rs
@@ -17,7 +17,6 @@ pub use image::{Architecture, Image, ImagePlatform, Os as ContainerOs, PossibleI
 use std::process::ExitStatus;
 
 use crate::errors::*;
-use crate::extensions::SafeCommand;
 use crate::shell::MessageInfo;
 
 #[derive(Debug)]
@@ -45,7 +44,6 @@ pub fn image_name(target: &str, sub: Option<&str>, repository: &str, tag: &str)
 pub fn run(
     options: DockerOptions,
     paths: DockerPaths,
-    command: SafeCommand,
     args: &[String],
     subcommand: Option<crate::Subcommand>,
     msg_info: &mut MessageInfo,
@@ -57,9 +55,9 @@ pub fn run(
         );
     }
     if options.is_remote() {
-        remote::run(options, paths, command, args, subcommand, msg_info)
+        remote::run(options, paths, args, subcommand, msg_info)
             .wrap_err("could not complete remote run")
     } else {
-        local::run(options, paths, command, args, msg_info)
+        local::run(options, paths, args, msg_info)
     }
 }
diff --git a/src/docker/remote.rs b/src/docker/remote.rs
@@ -10,12 +10,12 @@ use super::engine::Engine;
 use super::shared::*;
 use crate::config::bool_from_envvar;
 use crate::errors::Result;
-use crate::extensions::{CommandExt, SafeCommand};
+use crate::extensions::CommandExt;
 use crate::file::{self, PathExt, ToUtf8};
 use crate::rustc::{self, QualifiedToolchain, VersionMetaExt};
 use crate::shell::{MessageInfo, Stream};
+use crate::temp;
 use crate::TargetTriple;
-use crate::{temp, CargoVariant};
 
 // prevent further commands from running if we handled
 // a signal earlier, and the volume is exited.
@@ -667,7 +667,6 @@ impl QualifiedToolchain {
 pub(crate) fn run(
     options: DockerOptions,
     paths: DockerPaths,
-    mut cmd: SafeCommand,
     args: &[String],
     subcommand: Option<crate::Subcommand>,
     msg_info: &mut MessageInfo,
@@ -896,7 +895,9 @@ pub(crate) fn run(
         }
     }
 
-    if options.cargo_variant != CargoVariant::None {
+    let mut cmd = options.command_variant.safe_command();
+
+    if !options.command_variant.is_shell() {
         // `clean` doesn't handle symlinks: it will just unlink the target
         // directory, so we should just substitute it our target directory
         // for it. we'll still have the same end behavior
@@ -976,6 +977,11 @@ symlink_recurse \"${{prefix}}\"
     docker.add_cwd(&paths)?;
     docker.arg(&container_id);
     docker.add_build_command(toolchain_dirs, &cmd);
+
+    if options.interactive {
+        docker.arg("-i");
+    }
+
     bail_container_exited!();
     let status = docker
         .run_and_get_status(msg_info, false)
diff --git a/src/docker/shared.rs b/src/docker/shared.rs
@@ -17,7 +17,7 @@ use crate::file::{self, write_file, PathExt, ToUtf8};
 use crate::id;
 use crate::rustc::QualifiedToolchain;
 use crate::shell::{ColorChoice, MessageInfo, Verbosity};
-use crate::{CargoVariant, OutputExt, Target, TargetTriple};
+use crate::{CommandVariant, OutputExt, Target, TargetTriple};
 
 use rustc_version::Version as RustcVersion;
 
@@ -33,9 +33,10 @@ pub struct DockerOptions {
     pub target: Target,
     pub config: Config,
     pub image: Image,
-    pub cargo_variant: CargoVariant,
+    pub command_variant: CommandVariant,
     // not all toolchains will provide this
     pub rustc_version: Option<RustcVersion>,
+    pub interactive: bool,
 }
 
 impl DockerOptions {
@@ -44,16 +45,18 @@ impl DockerOptions {
         target: Target,
         config: Config,
         image: Image,
-        cargo_variant: CargoVariant,
+        cargo_variant: CommandVariant,
         rustc_version: Option<RustcVersion>,
+        interactive: bool,
     ) -> DockerOptions {
         DockerOptions {
             engine,
             target,
             config,
             image,
-            cargo_variant,
+            command_variant: cargo_variant,
             rustc_version,
+            interactive,
         }
     }
 
@@ -928,8 +931,8 @@ fn validate_env_var<'a>(
     Ok((key, value))
 }
 
-impl CargoVariant {
-    pub(crate) fn safe_command(self) -> SafeCommand {
+impl CommandVariant {
+    pub(crate) fn safe_command(&self) -> SafeCommand {
         SafeCommand::new(self.to_str())
     }
 }
@@ -1041,7 +1044,7 @@ impl DockerCommandExt for Command {
             ])
             .args(["-e", "CARGO_TARGET_DIR=/target"])
             .args(["-e", &cross_runner]);
-        if options.cargo_variant.uses_zig() {
+        if options.command_variant.uses_zig() {
             // otherwise, zig has a permission error trying to create the cache
             self.args(["-e", "XDG_CACHE_HOME=/target/.zig-cache"]);
         }
diff --git a/src/lib.rs b/src/lib.rs
@@ -443,38 +443,42 @@ impl Serialize for Target {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum CargoVariant {
+pub enum CommandVariant {
     Cargo,
     Xargo,
     Zig,
-    None,
+    Shell,
 }
 
-impl CargoVariant {
-    pub fn create(uses_zig: bool, uses_xargo: bool) -> Result<CargoVariant> {
+impl CommandVariant {
+    pub fn create(uses_zig: bool, uses_xargo: bool) -> Result<CommandVariant> {
         match (uses_zig, uses_xargo) {
             (true, true) => eyre::bail!("cannot use both zig and xargo"),
-            (true, false) => Ok(CargoVariant::Zig),
-            (false, true) => Ok(CargoVariant::Xargo),
-            (false, false) => Ok(CargoVariant::Cargo),
+            (true, false) => Ok(CommandVariant::Zig),
+            (false, true) => Ok(CommandVariant::Xargo),
+            (false, false) => Ok(CommandVariant::Cargo),
         }
     }
 
     pub fn to_str(self) -> &'static str {
         match self {
-            CargoVariant::Cargo => "cargo",
-            CargoVariant::Xargo => "xargo",
-            CargoVariant::Zig => "cargo-zigbuild",
-            CargoVariant::None => "",
+            CommandVariant::Cargo => "cargo",
+            CommandVariant::Xargo => "xargo",
+            CommandVariant::Zig => "cargo-zigbuild",
+            CommandVariant::Shell => "sh",
         }
     }
 
     pub fn uses_xargo(self) -> bool {
-        self == CargoVariant::Xargo
+        self == CommandVariant::Xargo
     }
 
     pub fn uses_zig(self) -> bool {
-        self == CargoVariant::Zig
+        self == CommandVariant::Zig
+    }
+
+    pub(crate) fn is_shell(self) -> bool {
+        self == CommandVariant::Shell
     }
 }
 
@@ -606,8 +610,9 @@ pub fn run(
                     target.clone(),
                     config,
                     image,
-                    crate::CargoVariant::create(uses_zig, uses_xargo)?,
+                    crate::CommandVariant::create(uses_zig, uses_xargo)?,
                     rustc_version,
+                    false,
                 );
 
                 install_interpreter_if_needed(
@@ -617,16 +622,8 @@ pub fn run(
                     &options,
                     msg_info,
                 )?;
-                let cmd = options.cargo_variant.safe_command();
-                let status = docker::run(
-                    options,
-                    paths,
-                    cmd,
-                    &filtered_args,
-                    args.subcommand,
-                    msg_info,
-                )
-                .wrap_err("could not run container")?;
+                let status = docker::run(options, paths, &filtered_args, args.subcommand, msg_info)
+                    .wrap_err("could not run container")?;
                 let needs_host = args.subcommand.map_or(false, |sc| sc.needs_host(is_remote));
                 if !status.success() {
                     warn_on_failure(&target, &toolchain, msg_info)?;
