Merge #1007

1007: volume improvements r=Alexhuszagh a=Emilgardis

<img width="998" alt="image" src="https://user-images.githubusercontent.com/1502855/188205513-8d95139e-d839-4540-a763-e66cf6803ee0.png">

Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>

Author: bors[bot]

src/bin/commands/containers.rs

@@ -1,12 +1,10 @@
 use std::io;
 
 use clap::{Args, Subcommand};
+use cross::docker::ImagePlatform;
+use cross::rustc::{QualifiedToolchain, Toolchain};
 use cross::shell::{MessageInfo, Stream};
 use cross::{docker, CommandExt, TargetTriple};
-use cross::{
-    docker::ImagePlatform,
-    rustc::{QualifiedToolchain, Toolchain},
-};
 
 #[derive(Args, Debug)]
 pub struct ListVolumes {
@@ -105,7 +103,7 @@ pub struct CreateVolume {
     pub engine: Option<String>,
     /// Toolchain to create a volume for
     #[clap(long, default_value = TargetTriple::DEFAULT.triple(), )]
-    pub toolchain: TargetTriple,
+    pub toolchain: String,
 }
 
 impl CreateVolume {
@@ -141,7 +139,7 @@ pub struct RemoveVolume {
     pub engine: Option<String>,
     /// Toolchain to remove the volume for
     #[clap(long, default_value = TargetTriple::DEFAULT.triple(), )]
-    pub toolchain: TargetTriple,
+    pub toolchain: String,
 }
 
 impl RemoveVolume {
@@ -321,11 +319,12 @@ fn get_cross_volumes(
     engine: &docker::Engine,
     msg_info: &mut MessageInfo,
 ) -> cross::Result<Vec<String>> {
+    use cross::docker::remote::VOLUME_PREFIX;
     let stdout = docker::subcommand(engine, "volume")
         .arg("list")
-        .args(&["--format", "{{.Name}}"])
+        .args(["--format", "{{.Name}}"])
         // handles simple regex: ^ for start of line.
-        .args(&["--filter", "name=^cross-"])
+        .args(["--filter", &format!("name=^{VOLUME_PREFIX}")])
         .run_and_get_stdout(msg_info)?;
 
     let mut volumes: Vec<String> = stdout.lines().map(|s| s.to_string()).collect();
@@ -372,7 +371,7 @@ pub fn prune_volumes(
     msg_info: &mut MessageInfo,
 ) -> cross::Result<()> {
     let mut command = docker::subcommand(engine, "volume");
-    command.args(&["prune", "--force"]);
+    command.args(["prune", "--force"]);
     if execute {
         command.run(msg_info, false).map_err(Into::into)
     } else {
@@ -392,25 +391,21 @@ pub fn create_persistent_volume(
     channel: Option<&Toolchain>,
     msg_info: &mut MessageInfo,
 ) -> cross::Result<()> {
-    let config = cross::config::Config::new(None);
-    let toolchain_host: cross::Target = toolchain.into();
-    let mut toolchain = QualifiedToolchain::default(&config, msg_info)?;
-    toolchain.replace_host(&ImagePlatform::from_target(
-        toolchain_host.target().clone(),
-    )?);
+    let mut toolchain = toolchain_or_target(&toolchain, msg_info)?;
     if let Some(channel) = channel {
-        toolchain = toolchain.with_picked(&config, channel.clone(), msg_info)?;
+        toolchain.channel = channel.channel.clone();
     };
     let (dirs, metadata) = docker::get_package_info(engine, toolchain.clone(), msg_info)?;
-    let container = docker::remote::unique_container_identifier(&toolchain_host, &metadata, &dirs)?;
+    let container =
+        docker::remote::unique_container_identifier(&toolchain.host().target, &metadata, &dirs)?;
     let volume = dirs.toolchain.unique_toolchain_identifier()?;
 
     if docker::remote::volume_exists(engine, &volume, msg_info)? {
         eyre::bail!("Error: volume {volume} already exists.");
     }
 
     docker::subcommand(engine, "volume")
-        .args(&["create", &volume])
+        .args(["create", &volume])
         .run_and_get_status(msg_info, false)?;
 
     // stop the container if it's already running
@@ -427,9 +422,9 @@ pub fn create_persistent_volume(
     // create a dummy running container to copy data over
     let mount_prefix = docker::remote::MOUNT_PREFIX;
     let mut docker = docker::subcommand(engine, "run");
-    docker.args(&["--name", &container]);
+    docker.args(["--name", &container]);
     docker.arg("--rm");
-    docker.args(&["-v", &format!("{}:{}", volume, mount_prefix)]);
+    docker.args(["-v", &format!("{}:{}", volume, mount_prefix)]);
     docker.arg("-d");
     let is_tty = io::Stdin::is_atty() && io::Stdout::is_atty() && io::Stderr::is_atty();
     if is_tty {
@@ -442,7 +437,7 @@ pub fn create_persistent_volume(
         // a TTY. this has a few issues though: now, the
         // container no longer responds to signals, so the
         // container will need to be sig-killed.
-        docker.args(&["sh", "-c", "sleep infinity"]);
+        docker.args(["sh", "-c", "sleep infinity"]);
     }
     // store first, since failing to non-existing container is fine
     docker::remote::create_container_deleter(engine.clone(), container.clone());
@@ -483,14 +478,11 @@ pub fn remove_persistent_volume(
     channel: Option<&Toolchain>,
     msg_info: &mut MessageInfo,
 ) -> cross::Result<()> {
-    let config = cross::config::Config::new(None);
-    let target_host: cross::Target = toolchain.into();
-    let mut toolchain = QualifiedToolchain::default(&config, msg_info)?;
-    toolchain.replace_host(&ImagePlatform::from_target(target_host.target().clone())?);
+    let mut toolchain = toolchain_or_target(&toolchain, msg_info)?;
     if let Some(channel) = channel {
-        toolchain = toolchain.with_picked(&config, channel.clone(), msg_info)?;
+        toolchain.channel = channel.channel.clone();
     };
-    let (dirs, _) = docker::get_package_info(engine, toolchain.clone(), msg_info)?;
+    let (dirs, _) = docker::get_package_info(engine, toolchain, msg_info)?;
     let volume = dirs.toolchain.unique_toolchain_identifier()?;
 
     if !docker::remote::volume_exists(engine, &volume, msg_info)? {
@@ -506,11 +498,12 @@ fn get_cross_containers(
     engine: &docker::Engine,
     msg_info: &mut MessageInfo,
 ) -> cross::Result<Vec<String>> {
+    use cross::docker::remote::VOLUME_PREFIX;
     let stdout = docker::subcommand(engine, "ps")
         .arg("-a")
-        .args(&["--format", "{{.Names}}: {{.State}}"])
+        .args(["--format", "{{.Names}}: {{.State}}"])
         // handles simple regex: ^ for start of line.
-        .args(&["--filter", "name=^cross-"])
+        .args(["--filter", &format!("name=^{VOLUME_PREFIX}")])
         .run_and_get_stdout(msg_info)?;
 
     let mut containers: Vec<String> = stdout.lines().map(|s| s.to_string()).collect();
@@ -576,3 +569,20 @@ pub fn remove_all_containers(
 
     Ok(())
 }
+
+fn toolchain_or_target(
+    s: &str,
+    msg_info: &mut MessageInfo,
+) -> Result<QualifiedToolchain, color_eyre::Report> {
+    let config = cross::config::Config::new(None);
+    let mut toolchain = QualifiedToolchain::default(&config, msg_info)?;
+    let target_list = cross::rustc::target_list(msg_info)?;
+    if target_list.contains(s) {
+        toolchain.replace_host(&ImagePlatform::from_target(s.into())?);
+    } else {
+        let picked: Toolchain = s.parse()?;
+        toolchain = toolchain.with_picked(picked)?;
+    }
+
+    Ok(toolchain)
+}

src/bin/commands/images.rs

@@ -167,8 +167,8 @@ fn get_cross_images(
     local: bool,
 ) -> cross::Result<Vec<Image>> {
     let mut images: BTreeSet<_> = cross::docker::subcommand(engine, "images")
-        .args(&["--format", "{{.Repository}}:{{.Tag}} {{.ID}}"])
-        .args(&[
+        .args(["--format", "{{.Repository}}:{{.Tag}} {{.ID}}"])
+        .args([
             "--filter",
             &format!("label={}.for-cross-target", cross::CROSS_LABEL_DOMAIN),
         ])
@@ -178,7 +178,7 @@ fn get_cross_images(
         .collect();
 
     let stdout = cross::docker::subcommand(engine, "images")
-        .args(&["--format", "{{.Repository}}:{{.Tag}} {{.ID}}"])
+        .args(["--format", "{{.Repository}}:{{.Tag}} {{.ID}}"])
         .run_and_get_stdout(msg_info)?;
     let ids: Vec<_> = images.iter().map(|i| i.id.to_string()).collect();
     images.extend(
@@ -239,7 +239,7 @@ fn get_image_target(
         }
     }
     let mut command = cross::docker::subcommand(engine, "inspect");
-    command.args(&[
+    command.args([
         "--format",
         &format!(
             r#"{{{{index .Config.Labels "{}.for-cross-target"}}}}"#,
@@ -377,7 +377,7 @@ pub fn remove_target_images(
     let target_list = msg_info.as_quiet(cross::rustc::target_list)?;
     let mut images = vec![];
     for image in cross_images {
-        let target = dbg!(get_image_target(engine, &image, &target_list, msg_info)?);
+        let target = get_image_target(engine, &image, &target_list, msg_info)?;
         if targets.contains(&target) {
             images.push(image);
         }

src/bin/cross-util.rs

@@ -38,9 +38,9 @@ enum Commands {
     Clean(commands::Clean),
 }
 
-fn is_toolchain(toolchain: &str) -> cross::Result<String> {
+fn is_toolchain(toolchain: &str) -> cross::Result<Toolchain> {
     if toolchain.starts_with('+') {
-        Ok(toolchain.chars().skip(1).collect())
+        Ok(toolchain.chars().skip(1).collect::<String>().parse()?)
     } else {
         let _ = <CliHidden as CommandFactory>::command().get_matches();
         unreachable!();

src/build.rs

@@ -39,7 +39,7 @@ fn commit_info() -> String {
 
 fn commit_hash() -> Result<String, Some> {
     let output = Command::new("git")
-        .args(&["rev-parse", "--short", "HEAD"])
+        .args(["rev-parse", "--short", "HEAD"])
         .output()?;
 
     if output.status.success() {
@@ -51,7 +51,7 @@ fn commit_hash() -> Result<String, Some> {
 
 fn commit_date() -> Result<String, Some> {
     let output = Command::new("git")
-        .args(&["log", "-1", "--date=short", "--pretty=format:%cd"])
+        .args(["log", "-1", "--date=short", "--pretty=format:%cd"])
         .output()?;
 
     if output.status.success() {

src/cargo.rs

@@ -132,7 +132,7 @@ pub fn cargo_metadata_with_args(
     if let Some(channel) = args.and_then(|x| x.channel.as_deref()) {
         command.arg(format!("+{channel}"));
     }
-    command.arg("metadata").args(&["--format-version", "1"]);
+    command.arg("metadata").args(["--format-version", "1"]);
     if let Some(cd) = cd {
         command.current_dir(cd);
     }

src/docker/custom.rs

@@ -151,11 +151,11 @@ impl<'a> Dockerfile<'a> {
             opts.contains("--load") || opts.contains("--output")
         });
         if options.engine.kind.is_docker() && !has_output {
-            docker_build.args(&["--output", "type=docker"]);
+            docker_build.args(["--output", "type=docker"]);
         };
 
         if let Some(context) = self.context() {
-            docker_build.arg(&context);
+            docker_build.arg(context);
         } else {
             docker_build.arg(paths.host_root());
         }

src/docker/image.rs

@@ -374,7 +374,7 @@ impl ImagePlatform {
             && Some(&self.os) == engine.os.as_ref()
         {
         } else {
-            cmd.args(&["--platform", &self.docker_platform()]);
+            cmd.args(["--platform", &self.docker_platform()]);
         }
     }
 }

src/docker/local.rs

@@ -12,7 +12,7 @@ use eyre::Context;
 // NOTE: host path must be absolute
 fn mount(docker: &mut Command, host_path: &Path, absolute_path: &Path, prefix: &str) -> Result<()> {
     let mount_path = absolute_path.as_posix_absolute()?;
-    docker.args(&[
+    docker.args([
         "-v",
         &format!("{}:{prefix}{}:z", host_path.to_utf8()?, mount_path),
     ]);
@@ -62,36 +62,36 @@ pub(crate) fn run(
     docker_user_id(&mut docker, engine.kind);
 
     docker
-        .args(&[
+        .args([
             "-v",
             &format!("{}:{}:z", dirs.xargo.to_utf8()?, dirs.xargo_mount_path()),
         ])
-        .args(&[
+        .args([
             "-v",
             &format!("{}:{}:z", dirs.cargo.to_utf8()?, dirs.cargo_mount_path()),
         ])
         // Prevent `bin` from being mounted inside the Docker container.
-        .args(&["-v", &format!("{}/bin", dirs.cargo_mount_path())]);
-    docker.args(&[
+        .args(["-v", &format!("{}/bin", dirs.cargo_mount_path())]);
+    docker.args([
         "-v",
         &format!("{}:{}:z", dirs.host_root.to_utf8()?, dirs.mount_root),
     ]);
     docker
-        .args(&[
+        .args([
             "-v",
             &format!(
                 "{}:{}:z,ro",
                 dirs.get_sysroot().to_utf8()?,
                 dirs.sysroot_mount_path()
             ),
         ])
-        .args(&["-v", &format!("{}:/target:z", dirs.target.to_utf8()?)]);
+        .args(["-v", &format!("{}:/target:z", dirs.target.to_utf8()?)]);
     docker_cwd(&mut docker, &paths)?;
 
     // When running inside NixOS or using Nix packaging we need to add the Nix
     // Store to the running container so it can load the needed binaries.
     if let Some(ref nix_store) = dirs.nix_store {
-        docker.args(&[
+        docker.args([
             "-v",
             &format!(
                 "{}:{}:z",
@@ -113,7 +113,7 @@ pub(crate) fn run(
 
     docker
         .arg(&image_name)
-        .args(&["sh", "-c", &build_command(dirs, &cmd)])
+        .args(["sh", "-c", &build_command(dirs, &cmd)])
         .run_and_get_status(msg_info, false)
         .map_err(Into::into)
 }