Merge #904

904: Various bug fixes for cross. r=Emilgardis a=Alexhuszagh

Fixed `cargo metadata` failing with unstable arguments in rustflags if the default toolchain was not nightly. We also log any warnings that occurred if we cannot get metadata. Also fixed a lack of formatting in messages with remote docker support. The mount paths are also correctly processed if using docker-in-docker for mounted volumes.

Closes #901.
Closes #903.
Required for #873.


Co-authored-by: Alex Huszagh <[email protected]>

Author: bors[bot]

CHANGELOG.md

@@ -20,6 +20,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 
+- #904 - ensure `cargo metadata` works by using the same channel.
+- #904 - fixed the path for workspace volumes and passthrough volumes with docker-in-docker.
 - #898 - fix the path to the mount root with docker-in-docker if mounting volumes.
 - #897 - ensure `target.$(...)` config options override `build` ones when parsing strings and vecs.
 - #895 - convert filenames in docker tags to ASCII lowercase and ignore invalid characters

src/cargo.rs

@@ -5,7 +5,7 @@ use std::process::{Command, ExitStatus};
 use crate::cli::Args;
 use crate::errors::*;
 use crate::extensions::CommandExt;
-use crate::shell::MessageInfo;
+use crate::shell::{self, MessageInfo};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Subcommand {
@@ -123,6 +123,9 @@ pub fn cargo_metadata_with_args(
     msg_info: MessageInfo,
 ) -> Result<Option<CargoMetadata>> {
     let mut command = cargo_command();
+    if let Some(channel) = args.and_then(|x| x.channel.as_deref()) {
+        command.arg(format!("+{channel}"));
+    }
     command.arg("metadata").args(&["--format-version", "1"]);
     if let Some(cd) = cd {
         command.current_dir(cd);
@@ -142,7 +145,9 @@ pub fn cargo_metadata_with_args(
     }
     let output = command.run_and_get_output(msg_info)?;
     if !output.status.success() {
-        // TODO: logging
+        shell::warn("unable to get metadata for package", msg_info)?;
+        let indented = shell::indent(&String::from_utf8(output.stderr)?, shell::default_ident());
+        shell::debug(indented, msg_info)?;
         return Ok(None);
     }
     let manifest: Option<CargoMetadata> = serde_json::from_slice(&output.stdout)?;

src/docker/local.rs

@@ -25,7 +25,8 @@ pub(crate) fn run(
     docker_in_docker: bool,
     cwd: &Path,
 ) -> Result<ExitStatus> {
-    let dirs = Directories::create(engine, metadata, cwd, sysroot, docker_in_docker)?;
+    let mount_finder = MountFinder::create(engine, docker_in_docker)?;
+    let dirs = Directories::create(&mount_finder, metadata, cwd, sysroot)?;
 
     let mut cmd = cargo_safe_command(uses_xargo);
     cmd.args(args);
@@ -37,6 +38,7 @@ pub(crate) fn run(
     let mount_volumes = docker_mount(
         &mut docker,
         metadata,
+        &mount_finder,
         config,
         target,
         cwd,

src/docker/mod.rs

@@ -12,7 +12,7 @@ use std::process::ExitStatus;
 
 use crate::cargo::CargoMetadata;
 use crate::errors::*;
-use crate::shell::MessageInfo;
+use crate::shell::{self, MessageInfo};
 use crate::{Config, Target};
 
 #[allow(clippy::too_many_arguments)] // TODO: refactor
@@ -28,6 +28,13 @@ pub fn run(
     docker_in_docker: bool,
     cwd: &Path,
 ) -> Result<ExitStatus> {
+    if cfg!(target_os = "windows") && docker_in_docker {
+        shell::fatal(
+            "running cross insider a container running windows is currently unsupported",
+            msg_info,
+            1,
+        );
+    }
     if engine.is_remote {
         remote::run(
             engine,

src/docker/remote.rs

@@ -764,7 +764,8 @@ pub(crate) fn run(
     docker_in_docker: bool,
     cwd: &Path,
 ) -> Result<ExitStatus> {
-    let dirs = Directories::create(engine, metadata, cwd, sysroot, docker_in_docker)?;
+    let mount_finder = MountFinder::create(engine, docker_in_docker)?;
+    let dirs = Directories::create(&mount_finder, metadata, cwd, sysroot)?;
 
     let mount_prefix = MOUNT_PREFIX;
 
@@ -793,16 +794,16 @@ pub(crate) fn run(
     let volume = VolumeId::create(engine, &toolchain_id, &container, msg_info)?;
     let state = container_state(engine, &container, msg_info)?;
     if !state.is_stopped() {
-        shell::warn("container {container} was running.", msg_info)?;
+        shell::warn(format_args!("container {container} was running."), msg_info)?;
         container_stop(engine, &container, msg_info)?;
     }
     if state.exists() {
-        shell::warn("container {container} was exited.", msg_info)?;
+        shell::warn(format_args!("container {container} was exited."), msg_info)?;
         container_rm(engine, &container, msg_info)?;
     }
     if let VolumeId::Discard(ref id) = volume {
         if volume_exists(engine, id, msg_info)? {
-            shell::warn("temporary volume {container} existed.", msg_info)?;
+            shell::warn(format_args!("temporary volume {id} existed."), msg_info)?;
             volume_rm(engine, id, msg_info)?;
         }
     }
@@ -824,6 +825,7 @@ pub(crate) fn run(
     let mount_volumes = docker_mount(
         &mut docker,
         metadata,
+        &mount_finder,
         config,
         target,
         cwd,

src/docker/shared.rs

@@ -43,17 +43,11 @@ pub struct Directories {
 
 impl Directories {
     pub fn create(
-        engine: &Engine,
+        mount_finder: &MountFinder,
         metadata: &CargoMetadata,
         cwd: &Path,
         sysroot: &Path,
-        docker_in_docker: bool,
     ) -> Result<Self> {
-        let mount_finder = if docker_in_docker {
-            MountFinder::new(docker_read_mount_paths(engine)?)
-        } else {
-            MountFinder::default()
-        };
         let home_dir =
             home::home_dir().ok_or_else(|| eyre::eyre!("could not find home directory"))?;
         let cargo = home::cargo_home()?;
@@ -89,18 +83,10 @@ impl Directories {
         }
         #[cfg(not(target_os = "windows"))]
         {
+            // NOTE: host root has already found the mount path
             mount_root = host_root.to_utf8()?.to_string();
         }
-        let mount_cwd: String;
-        #[cfg(target_os = "windows")]
-        {
-            // On Windows, we can not mount the directory name directly. Instead, we use wslpath to convert the path to a linux compatible path.
-            mount_cwd = cwd.as_wslpath()?;
-        }
-        #[cfg(not(target_os = "windows"))]
-        {
-            mount_cwd = mount_finder.find_mount_path(cwd).to_utf8()?.to_string();
-        }
+        let mount_cwd = mount_finder.find_path(cwd, false)?;
         let sysroot = mount_finder.find_mount_path(sysroot);
 
         Ok(Directories {
@@ -163,8 +149,10 @@ pub fn get_package_info(
     let cwd = std::env::current_dir()?;
     let host_meta = rustc::version_meta()?;
     let host = host_meta.host();
+
     let sysroot = rustc::get_sysroot(&host, &target, channel, msg_info)?.1;
-    let dirs = Directories::create(engine, &metadata, &cwd, &sysroot, docker_in_docker)?;
+    let mount_finder = MountFinder::create(engine, docker_in_docker)?;
+    let dirs = Directories::create(&mount_finder, &metadata, &cwd, &sysroot)?;
 
     Ok((target, metadata, dirs))
 }
@@ -251,9 +239,9 @@ fn add_cargo_configuration_envvars(docker: &mut Command) {
     }
 }
 
-pub(crate) fn mount(docker: &mut Command, val: &Path, prefix: &str) -> Result<String> {
-    let host_path = file::canonicalize(val)?;
-    let mount_path = canonicalize_mount_path(&host_path)?;
+// NOTE: host path must be canonical
+pub(crate) fn mount(docker: &mut Command, host_path: &Path, prefix: &str) -> Result<String> {
+    let mount_path = canonicalize_mount_path(host_path)?;
     docker.args(&[
         "-v",
         &format!("{}:{prefix}{}", host_path.to_utf8()?, mount_path),
@@ -338,6 +326,7 @@ pub(crate) fn docker_cwd(
 pub(crate) fn docker_mount(
     docker: &mut Command,
     metadata: &CargoMetadata,
+    mount_finder: &MountFinder,
     config: &Config,
     target: &Target,
     cwd: &Path,
@@ -359,15 +348,19 @@ pub(crate) fn docker_mount(
         };
 
         if let Ok(val) = value {
-            let mount_path = mount_cb(docker, val.as_ref())?;
-            docker.args(&["-e", &format!("{}={}", var, mount_path)]);
+            let canonical_val = file::canonicalize(&val)?;
+            let host_path = mount_finder.find_path(&canonical_val, true)?;
+            let mount_path = mount_cb(docker, host_path.as_ref())?;
+            docker.args(&["-e", &format!("{}={}", host_path, mount_path)]);
             store_cb((val, mount_path));
             mount_volumes = true;
         }
     }
 
     for path in metadata.path_dependencies() {
-        let mount_path = mount_cb(docker, path)?;
+        let canonical_path = file::canonicalize(path)?;
+        let host_path = mount_finder.find_path(&canonical_path, true)?;
+        let mount_path = mount_cb(docker, host_path.as_ref())?;
         store_cb((path.to_utf8()?.to_string(), mount_path));
         mount_volumes = true;
     }
@@ -614,7 +607,7 @@ fn dockerinfo_parse_user_mounts(info: &serde_json::Value) -> Vec<MountDetail> {
 }
 
 #[derive(Debug, Default)]
-struct MountFinder {
+pub struct MountFinder {
     mounts: Vec<MountDetail>,
 }
 
@@ -636,7 +629,15 @@ impl MountFinder {
         MountFinder { mounts }
     }
 
-    fn find_mount_path(&self, path: impl AsRef<Path>) -> PathBuf {
+    pub fn create(engine: &Engine, docker_in_docker: bool) -> Result<MountFinder> {
+        Ok(if docker_in_docker {
+            MountFinder::new(docker_read_mount_paths(engine)?)
+        } else {
+            MountFinder::default()
+        })
+    }
+
+    pub fn find_mount_path(&self, path: impl AsRef<Path>) -> PathBuf {
         let path = path.as_ref();
 
         for info in &self.mounts {
@@ -647,6 +648,23 @@ impl MountFinder {
 
         path.to_path_buf()
     }
+
+    #[allow(unused_variables, clippy::needless_return)]
+    fn find_path(&self, path: &Path, host: bool) -> Result<String> {
+        #[cfg(target_os = "windows")]
+        {
+            // On Windows, we can not mount the directory name directly. Instead, we use wslpath to convert the path to a linux compatible path.
+            if host {
+                return Ok(path.to_utf8()?.to_string());
+            } else {
+                return path.as_wslpath();
+            }
+        }
+        #[cfg(not(target_os = "windows"))]
+        {
+            return Ok(self.find_mount_path(path).to_utf8()?.to_string());
+        }
+    }
 }
 
 fn path_digest(path: &Path) -> Result<const_sha1::Digest> {

src/shell.rs

@@ -308,3 +308,14 @@ impl Stream for io::Stderr {
     const TTY: atty::Stream = atty::Stream::Stderr;
     const OWO: owo_colors::Stream = owo_colors::Stream::Stderr;
 }
+
+pub fn default_ident() -> usize {
+    cross_prefix!("").len()
+}
+
+pub fn indent(message: &str, spaces: usize) -> String {
+    message
+        .lines()
+        .map(|s| format!("{:spaces$}{s}", ""))
+        .collect()
+}