Add support for cargo clean on remote hosts.

Required to patch #724 without deleting the entire volume for persistent
data volumes. A few changes were required: the entire `/cross` mount
prefix must be owned by the user so `/cross/target` can be removed.
Next, we use the full path to the mounted target directory, rather than
the symlink, since `cargo clean` would just delete the symlink. Finally,
we've added `cargo clean` to a list of known subcommands, and it only
needs docker if we use a remote host.

Author: Alexhuszagh

src/bin/cross-util.rs

@@ -44,7 +44,7 @@ fn get_container_engine(engine: Option<&str>, verbose: bool) -> cross::Result<do
     } else {
         docker::get_container_engine()?
     };
-    docker::Engine::from_path(engine, verbose)
+    docker::Engine::from_path(engine, None, verbose)
 }
 
 pub fn main() -> cross::Result<()> {

src/cargo.rs

@@ -19,11 +19,20 @@ pub enum Subcommand {
     Clippy,
     Metadata,
     List,
+    Clean,
 }
 
 impl Subcommand {
-    pub fn needs_docker(self) -> bool {
-        !matches!(self, Subcommand::Other | Subcommand::List)
+    pub fn needs_docker(self, is_remote: bool) -> bool {
+        match self {
+            Subcommand::Other | Subcommand::List => false,
+            Subcommand::Clean if !is_remote => false,
+            _ => true,
+        }
+    }
+
+    pub fn needs_host(self, is_remote: bool) -> bool {
+        self == Subcommand::Clean && is_remote
     }
 
     pub fn needs_interpreter(self) -> bool {
@@ -40,6 +49,7 @@ impl<'a> From<&'a str> for Subcommand {
         match s {
             "b" | "build" => Subcommand::Build,
             "c" | "check" => Subcommand::Check,
+            "clean" => Subcommand::Clean,
             "doc" => Subcommand::Doc,
             "r" | "run" => Subcommand::Run,
             "rustc" => Subcommand::Rustc,

src/docker/engine.rs

@@ -25,18 +25,16 @@ pub struct Engine {
 }
 
 impl Engine {
-    pub fn new(verbose: bool) -> Result<Engine> {
+    pub fn new(is_remote: Option<bool>, verbose: bool) -> Result<Engine> {
         let path = get_container_engine()
             .map_err(|_| eyre::eyre!("no container engine found"))
             .with_suggestion(|| "is docker or podman installed?")?;
-        Self::from_path(path, verbose)
+        Self::from_path(path, is_remote, verbose)
     }
 
-    pub fn from_path(path: PathBuf, verbose: bool) -> Result<Engine> {
+    pub fn from_path(path: PathBuf, is_remote: Option<bool>, verbose: bool) -> Result<Engine> {
         let kind = get_engine_type(&path, verbose)?;
-        let is_remote = env::var("CROSS_REMOTE")
-            .map(|s| bool_from_envvar(&s))
-            .unwrap_or_default();
+        let is_remote = is_remote.unwrap_or_else(Self::is_remote);
         Ok(Engine {
             path,
             kind,
@@ -47,6 +45,12 @@ impl Engine {
     pub fn needs_remote(&self) -> bool {
         self.is_remote && self.kind == EngineType::Podman
     }
+
+    pub fn is_remote() -> bool {
+        env::var("CROSS_REMOTE")
+            .map(|s| bool_from_envvar(&s))
+            .unwrap_or_default()
+    }
 }
 
 // determine if the container engine is docker. this fixes issues with

src/docker/remote.rs

@@ -146,6 +146,19 @@ fn is_cachedir(entry: &fs::DirEntry) -> bool {
     }
 }
 
+fn container_path_exists(
+    engine: &Engine,
+    container: &str,
+    path: &Path,
+    verbose: bool,
+) -> Result<bool> {
+    Ok(subcommand(engine, "exec")
+        .arg(container)
+        .args(&["bash", "-c", &format!("[[ -d '{}' ]]", path.as_posix()?)])
+        .run_and_get_status(verbose, true)?
+        .success())
+}
+
 // copy files for a docker volume, for remote host support
 fn copy_volume_files_nocache(
     engine: &Engine,
@@ -329,15 +342,7 @@ pub fn copy_volume_container_rust_triple(
     // or the first run of the target toolchain, we know it doesn't exist.
     let mut skip = false;
     if skip_exists {
-        skip = subcommand(engine, "exec")
-            .arg(container)
-            .args(&[
-                "bash",
-                "-c",
-                &format!("[[ -d '{}' ]]", dst_toolchain.as_posix()?),
-            ])
-            .run_and_get_status(verbose, true)?
-            .success();
+        skip = container_path_exists(engine, container, &dst_toolchain, verbose)?;
     }
     if !skip {
         copy_volume_files(engine, container, &src_toolchain, &dst_rustlib, verbose)?;
@@ -490,9 +495,6 @@ pub(crate) fn run(
 ) -> Result<ExitStatus> {
     let dirs = Directories::create(engine, metadata, cwd, sysroot, docker_in_docker, verbose)?;
 
-    let mut cmd = cargo_safe_command(uses_xargo);
-    cmd.args(args);
-
     let mount_prefix = MOUNT_PREFIX;
 
     // the logic is broken into the following steps
@@ -654,10 +656,7 @@ pub(crate) fn run(
     let mut to_symlink = vec![];
     let target_dir = file::canonicalize(&dirs.target)?;
     let target_dir = if let Ok(relpath) = target_dir.strip_prefix(&dirs.host_root) {
-        // target dir is in the project, just symlink it in
-        let target_dir = mount_root.join(relpath);
-        to_symlink.push((target_dir.clone(), "/target".to_string()));
-        target_dir
+        mount_root.join(relpath)
     } else {
         // outside project, need to copy the target data over
         // only do if we're copying over cached files.
@@ -687,13 +686,43 @@ pub(crate) fn run(
         }
     }
 
+    // `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
+    let mut final_args = vec![];
+    let mut iter = args.iter().cloned();
+    let mut has_target_dir = false;
+    let target_dir_string = target_dir.to_utf8()?.to_string();
+    while let Some(arg) = iter.next() {
+        if arg == "--target-dir" {
+            has_target_dir = true;
+            final_args.push(arg);
+            if iter.next().is_some() {
+                final_args.push(target_dir_string.clone());
+            }
+        } else if arg.starts_with("--target-dir=") {
+            has_target_dir = true;
+            if arg.split_once('=').is_some() {
+                final_args.push(format!("--target-dir={target_dir_string}"));
+            }
+        } else {
+            final_args.push(arg);
+        }
+    }
+    if !has_target_dir {
+        final_args.push("--target-dir".to_string());
+        final_args.push(target_dir_string);
+    }
+    let mut cmd = cargo_safe_command(uses_xargo);
+    cmd.args(final_args);
+
     // 5. create symlinks for copied data
     let mut symlink = vec!["set -e pipefail".to_string()];
     if verbose {
         symlink.push("set -x".to_string());
     }
     symlink.push(format!(
-        "chown -R {uid}:{gid} {mount_prefix}/*",
+        "chown -R {uid}:{gid} {mount_prefix}",
         uid = user_id(),
         gid = group_id(),
     ));
@@ -738,12 +767,15 @@ symlink_recurse \"${{prefix}}\"
         .map_err(Into::into);
 
     // 7. copy data from our target dir back to host
-    subcommand(engine, "cp")
-        .arg("-a")
-        .arg(&format!("{container}:{}", target_dir.as_posix()?))
-        .arg(&dirs.target.parent().unwrap())
-        .run_and_get_status(verbose, false)
-        .map_err::<eyre::ErrReport, _>(Into::into)?;
+    // this might not exist if we ran `clean`.
+    if container_path_exists(engine, &container, &target_dir, verbose)? {
+        subcommand(engine, "cp")
+            .arg("-a")
+            .arg(&format!("{container}:{}", target_dir.as_posix()?))
+            .arg(&dirs.target.parent().unwrap())
+            .run_and_get_status(verbose, false)
+            .map_err::<eyre::ErrReport, _>(Into::into)?;
+    }
 
     status
 }

src/lib.rs

@@ -478,9 +478,13 @@ pub fn run() -> Result<ExitStatus> {
                 filtered_args.push("-Zbuild-std".to_string());
             }
 
-            if target.needs_docker() && args.subcommand.map(|sc| sc.needs_docker()).unwrap_or(false)
-            {
-                let engine = docker::Engine::new(verbose)?;
+            let is_remote = docker::Engine::is_remote();
+            let needs_docker = args
+                .subcommand
+                .map(|sc| sc.needs_docker(is_remote))
+                .unwrap_or(false);
+            if target.needs_docker() && needs_docker {
+                let engine = docker::Engine::new(Some(is_remote), verbose)?;
                 if host_version_meta.needs_interpreter()
                     && needs_interpreter
                     && target.needs_interpreter()
@@ -489,7 +493,7 @@ pub fn run() -> Result<ExitStatus> {
                     docker::register(&engine, &target, verbose)?
                 }
 
-                return docker::run(
+                let status = docker::run(
                     &engine,
                     &target,
                     &filtered_args,
@@ -500,7 +504,14 @@ pub fn run() -> Result<ExitStatus> {
                     verbose,
                     args.docker_in_docker,
                     &cwd,
-                );
+                )?;
+                let needs_host = args
+                    .subcommand
+                    .map(|sc| sc.needs_host(is_remote))
+                    .unwrap_or(false);
+                if !(status.success() && needs_host) {
+                    return Ok(status);
+                }
             }
         }
     }

xtask/src/main.rs

@@ -85,5 +85,5 @@ fn get_container_engine(engine: Option<&str>, verbose: bool) -> cross::Result<do
     } else {
         docker::get_container_engine()?
     };
-    docker::Engine::from_path(engine, verbose)
+    docker::Engine::from_path(engine, None, verbose)
 }