some improvements

* fixes wrong handling of `cross-util +toolchain`
* removes a trailing dbg!
* makes `cross-utils volumes create --toolchain` actually parse a toolchain
  this may be a bit surprising as with `+custom` only the channel will be picked

Author: Emilgardis

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}}"])
         // 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();
@@ -392,17 +391,13 @@ 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)? {
@@ -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}}"])
         // 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

@@ -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/docker/remote.rs

@@ -16,11 +16,13 @@ use crate::extensions::CommandExt;
 use crate::file::{self, PathExt, ToUtf8};
 use crate::rustc::{self, QualifiedToolchain, VersionMetaExt};
 use crate::shell::{ColorChoice, MessageInfo, Stream, Verbosity};
-use crate::temp;
-use crate::{Target, TargetTriple};
+use crate::TargetTriple;
+use crate::{temp, OutputExt};
 
 // the mount directory for the data volume.
 pub const MOUNT_PREFIX: &str = "/cross";
+// the prefix used when naming volumes
+pub const VOLUME_PREFIX: &str = "cross-";
 // default timeout to stop a container (in seconds)
 pub const DEFAULT_TIMEOUT: u32 = 2;
 // instant kill in case of a non-graceful exit
@@ -94,16 +96,6 @@ enum VolumeId {
     Discard,
 }
 
-impl VolumeId {
-    fn create(engine: &Engine, toolchain: &str, msg_info: &mut MessageInfo) -> Result<Self> {
-        if volume_exists(engine, toolchain, msg_info)? {
-            Ok(Self::Keep(toolchain.to_owned()))
-        } else {
-            Ok(Self::Discard)
-        }
-    }
-}
-
 // prevent further commands from running if we handled
 // a signal earlier, and the volume is exited.
 // this isn't required, but avoids unnecessary
@@ -754,6 +746,32 @@ pub fn volume_exists(engine: &Engine, volume: &str, msg_info: &mut MessageInfo)
         .map(|output| output.status.success())
 }
 
+pub fn existing_volumes(
+    engine: &Engine,
+    toolchain: &QualifiedToolchain,
+    msg_info: &mut MessageInfo,
+) -> Result<Vec<String>> {
+    let list = run_and_get_output(
+        engine,
+        &[
+            "volume",
+            "list",
+            "--format",
+            "{{.Name}}",
+            "--filter",
+            &format!("name=^{VOLUME_PREFIX}{}", toolchain),
+        ],
+        msg_info,
+    )?
+    .stdout()?;
+
+    if list.is_empty() {
+        Ok(vec![])
+    } else {
+        Ok(list.split('\n').map(ToOwned::to_owned).collect())
+    }
+}
+
 pub fn container_stop(
     engine: &Engine,
     container: &str,
@@ -818,14 +836,14 @@ impl QualifiedToolchain {
             .to_utf8()?;
         let toolchain_hash = path_hash(self.get_sysroot())?;
         Ok(format!(
-            "cross-{toolchain_name}-{toolchain_hash}-{commit_hash}"
+            "{VOLUME_PREFIX}{toolchain_name}-{toolchain_hash}-{commit_hash}"
         ))
     }
 }
 
 // unique identifier for a given project
 pub fn unique_container_identifier(
-    target: &Target,
+    triple: &TargetTriple,
     metadata: &CargoMetadata,
     dirs: &Directories,
 ) -> Result<String> {
@@ -847,7 +865,6 @@ pub fn unique_container_identifier(
         });
 
     let name = &package.name;
-    let triple = target.triple();
     let toolchain_id = dirs.toolchain.unique_toolchain_identifier()?;
     let project_hash = path_hash(&package.manifest_path)?;
     Ok(format!("{toolchain_id}-{triple}-{name}-{project_hash}"))
@@ -892,8 +909,22 @@ pub(crate) fn run(
     // note that since we use `docker run --rm`, it's very
     // unlikely the container state existed before.
     let toolchain_id = dirs.toolchain.unique_toolchain_identifier()?;
-    let container = unique_container_identifier(target, &paths.metadata, dirs)?;
-    let volume = VolumeId::create(engine, &toolchain_id, msg_info)?;
+    let container = unique_container_identifier(target.target(), &paths.metadata, dirs)?;
+    let volume = {
+        let existing = existing_volumes(engine, &dirs.toolchain, msg_info)?;
+        if existing.iter().any(|v| v == &toolchain_id) {
+            VolumeId::Keep(toolchain_id)
+        } else {
+            let partial = format!("{VOLUME_PREFIX}{}", dirs.toolchain);
+            if existing.iter().any(|v| v.starts_with(&partial)) {
+                msg_info.warn(format_args!(
+                    "a persistent volume does not exists for `{0}`, but there is a volume for a different version.\n > Create a new volume with `cross-util volumes create --toolchain {0}`",
+                    dirs.toolchain
+                ))?;
+            }
+            VolumeId::Discard
+        }
+    };
     let state = container_state(engine, &container, msg_info)?;
     if !state.is_stopped() {
         msg_info.warn(format_args!("container {container} was running."))?;

src/lib.rs

@@ -556,7 +556,7 @@ r#"Overriding the toolchain in cross is only possible in CLI by specifying a cha
 To override the toolchain mounted in the image, set `target.{}.image.toolchain = "{picked_host}"`"#, target).header("Note:".bright_cyan()));
             }
 
-            default_toolchain.with_picked(&config, picked_toolchain, msg_info)?
+            default_toolchain.with_picked(picked_toolchain)?
         } else {
             default_toolchain
         };

src/rustc.rs

@@ -205,13 +205,7 @@ impl QualifiedToolchain {
     }
 
     /// Merge a "picked" toolchain, overriding set fields.
-    pub fn with_picked(
-        self,
-        config: &crate::config::Config,
-        picked: Toolchain,
-        msg_info: &mut MessageInfo,
-    ) -> Result<Self> {
-        let toolchain = Self::default(config, msg_info)?;
+    pub fn with_picked(self, picked: Toolchain) -> Result<Self> {
         let date = picked.date.or(self.date);
         let host = picked
             .host
@@ -222,7 +216,7 @@ impl QualifiedToolchain {
             &channel,
             &date,
             &host,
-            &toolchain.sysroot,
+            &self.sysroot,
         ))
     }