Merge #939

939: Fix cross when using absolute paths to the current project. r=Emilgardis a=Alexhuszagh

Closes #938.

Co-authored-by: Alex Huszagh <ahuszagh@gmail.com>

Author: bors[bot]

.changes/939.json

@@ -0,0 +1,5 @@
+{
+    "type": "changed",
+    "description": "Remove `/project` mounting, instead always mount in the same path as on the host",
+    "issues": [938]
+}

src/docker/local.rs

@@ -24,7 +24,7 @@ pub(crate) fn run(
     docker_userns(&mut docker);
     docker_envvars(&mut docker, &options.config, &options.target, msg_info)?;
 
-    let mount_volumes = docker_mount(
+    docker_mount(
         &mut docker,
         &options,
         &paths,
@@ -43,18 +43,14 @@ pub(crate) fn run(
         .args(&["-v", &format!("{}:/cargo:Z", dirs.cargo.to_utf8()?)])
         // Prevent `bin` from being mounted inside the Docker container.
         .args(&["-v", "/cargo/bin"]);
-    if mount_volumes {
-        docker.args(&[
-            "-v",
-            &format!("{}:{}:Z", dirs.host_root.to_utf8()?, dirs.mount_root),
-        ]);
-    } else {
-        docker.args(&["-v", &format!("{}:/project:Z", dirs.host_root.to_utf8()?)]);
-    }
+    docker.args(&[
+        "-v",
+        &format!("{}:{}:Z", dirs.host_root.to_utf8()?, dirs.mount_root),
+    ]);
     docker
         .args(&["-v", &format!("{}:/rust:Z,ro", dirs.sysroot.to_utf8()?)])
         .args(&["-v", &format!("{}:/target:Z", dirs.target.to_utf8()?)]);
-    docker_cwd(&mut docker, &paths, mount_volumes)?;
+    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.

src/docker/remote.rs

@@ -915,7 +915,7 @@ pub(crate) fn run(
     docker.args(&["-v", &volume_mount]);
 
     let mut volumes = vec![];
-    let mount_volumes = docker_mount(
+    docker_mount(
         &mut docker,
         &options,
         &paths,
@@ -1018,28 +1018,23 @@ pub(crate) fn run(
         )
         .wrap_err("when copying rust target files")?;
     }
-    let mount_root = if mount_volumes {
-        // cannot panic: absolute unix path, must have root
-        let rel_mount_root = dirs
-            .mount_root
-            .strip_prefix('/')
-            .expect("mount root should be absolute");
-        let mount_root = mount_prefix_path.join(rel_mount_root);
-        if !rel_mount_root.is_empty() {
-            create_volume_dir(
-                engine,
-                &container,
-                mount_root
-                    .parent()
-                    .expect("mount root should have a parent directory"),
-                msg_info,
-            )
-            .wrap_err("when creating mount root")?;
-        }
-        mount_root
-    } else {
-        mount_prefix_path.join("project")
-    };
+    // cannot panic: absolute unix path, must have root
+    let rel_mount_root = dirs
+        .mount_root
+        .strip_prefix('/')
+        .expect("mount root should be absolute");
+    let mount_root = mount_prefix_path.join(rel_mount_root);
+    if !rel_mount_root.is_empty() {
+        create_volume_dir(
+            engine,
+            &container,
+            mount_root
+                .parent()
+                .expect("mount root should have a parent directory"),
+            msg_info,
+        )
+        .wrap_err("when creating mount root")?;
+    }
     copy_volume_container_project(
         engine,
         &container,
@@ -1175,7 +1170,7 @@ symlink_recurse \"${{prefix}}\"
     let mut docker = subcommand(engine, "exec");
     docker_user_id(&mut docker, engine.kind);
     docker_envvars(&mut docker, &options.config, target, msg_info)?;
-    docker_cwd(&mut docker, &paths, mount_volumes)?;
+    docker_cwd(&mut docker, &paths)?;
     docker.arg(&container);
     docker.args(&["sh", "-c", &format!("PATH=$PATH:/rust/bin {:?}", cmd)]);
     bail_container_exited!();

src/docker/shared.rs

@@ -9,12 +9,15 @@ use crate::cargo::{cargo_metadata_with_args, CargoMetadata};
 use crate::config::{bool_from_envvar, Config};
 use crate::errors::*;
 use crate::extensions::{CommandExt, SafeCommand};
-use crate::file::{self, write_file, PathExt, ToUtf8};
+use crate::file::{self, write_file, ToUtf8};
 use crate::id;
 use crate::rustc::{self, VersionMetaExt};
 use crate::shell::{MessageInfo, Verbosity};
 use crate::Target;
 
+#[cfg(target_os = "windows")]
+use crate::file::PathExt;
+
 pub use super::custom::CROSS_CUSTOM_DOCKERFILE_IMAGE_PREFIX;
 
 pub const CROSS_IMAGE: &str = "ghcr.io/cross-rs";
@@ -496,20 +499,8 @@ pub(crate) fn docker_envvars(
     Ok(())
 }
 
-pub(crate) fn docker_cwd(
-    docker: &mut Command,
-    paths: &DockerPaths,
-    mount_volumes: bool,
-) -> Result<()> {
-    if mount_volumes {
-        docker.args(&["-w", paths.mount_cwd()]);
-    } else if paths.mount_cwd() == paths.workspace_root().to_utf8()? {
-        docker.args(&["-w", "/project"]);
-    } else {
-        // We do this to avoid clashes with path separators. Windows uses `\` as a path separator on Path::join
-        let working_dir = Path::new("/project").join(paths.workspace_from_cwd()?);
-        docker.args(&["-w", &working_dir.as_posix()?]);
-    }
+pub(crate) fn docker_cwd(docker: &mut Command, paths: &DockerPaths) -> Result<()> {
+    docker.args(&["-w", paths.mount_cwd()]);
 
     Ok(())
 }
@@ -520,14 +511,7 @@ pub(crate) fn docker_mount(
     paths: &DockerPaths,
     mount_cb: impl Fn(&mut Command, &Path) -> Result<String>,
     mut store_cb: impl FnMut((String, String)),
-) -> Result<bool> {
-    let mut mount_volumes = false;
-    // FIXME(emilgardis 2022-04-07): This is a fallback so that if it's hard for us to do mounting logic, make it simple(r)
-    // Preferably we would not have to do this.
-    if !paths.in_workspace() {
-        mount_volumes = true;
-    }
-
+) -> Result<()> {
     for ref var in options
         .config
         .env_volumes(&options.target)?
@@ -545,7 +529,6 @@ pub(crate) fn docker_mount(
             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;
         }
     }
 
@@ -554,10 +537,9 @@ pub(crate) fn docker_mount(
         let host_path = paths.mount_finder.find_path(&canonical_path, true)?;
         let mount_path = mount_cb(docker, host_path.as_ref())?;
         store_cb((path.to_utf8()?.to_owned(), mount_path));
-        mount_volumes = true;
     }
 
-    Ok(mount_volumes)
+    Ok(())
 }
 
 pub(crate) fn canonicalize_mount_path(path: &Path) -> Result<String> {
@@ -806,6 +788,9 @@ mod tests {
     use super::*;
     use crate::id;
 
+    #[cfg(not(target_os = "windows"))]
+    use crate::file::PathExt;
+
     #[test]
     fn test_docker_user_id() {
         let var = "CROSS_ROOTLESS_CONTAINER_ENGINE";