diff --git a/.changes/947.json b/.changes/947.json new file mode 100644 index 000000000..ec5243eb1 --- /dev/null +++ b/.changes/947.json @@ -0,0 +1,12 @@ +[ + { + "type": "internal", + "description": "resolve symlinks to cargo and xargo home before mounting", + "issues": [373] + }, + { + "type": "fixed", + "description": "mount cargo, xargo, and the sysroot at the same path as on the host to avoid unnecessary recompilation.", + "issues": [551] + } +] diff --git a/src/bin/commands/containers.rs b/src/bin/commands/containers.rs index f602b59c8..973609d9e 100644 --- a/src/bin/commands/containers.rs +++ b/src/bin/commands/containers.rs @@ -451,15 +451,14 @@ pub fn create_persistent_volume( docker::remote::copy_volume_container_xargo( engine, &container, - &dirs.xargo, - &toolchain_host, + &dirs, mount_prefix.as_ref(), msg_info, )?; docker::remote::copy_volume_container_cargo( engine, &container, - &dirs.cargo, + &dirs, mount_prefix.as_ref(), copy_registry, msg_info, @@ -467,7 +466,7 @@ pub fn create_persistent_volume( docker::remote::copy_volume_container_rust( engine, &container, - &toolchain, + &dirs, None, mount_prefix.as_ref(), msg_info, diff --git a/src/docker/image.rs b/src/docker/image.rs index 7bfd5b9f5..266c5813c 100644 --- a/src/docker/image.rs +++ b/src/docker/image.rs @@ -135,6 +135,12 @@ impl ImagePlatform { } } +impl Default for ImagePlatform { + fn default() -> ImagePlatform { + ImagePlatform::DEFAULT + } +} + impl TryFrom<&str> for ImagePlatform { type Error = ::Err; diff --git a/src/docker/local.rs b/src/docker/local.rs index e11b2bb8c..665bb385e 100644 --- a/src/docker/local.rs +++ b/src/docker/local.rs @@ -27,7 +27,13 @@ pub(crate) fn run( .image .platform .specify_platform(&options.engine, &mut docker); - docker_envvars(&mut docker, &options.config, &options.target, msg_info)?; + docker_envvars( + &mut docker, + &options.config, + dirs, + &options.target, + msg_info, + )?; docker_mount( &mut docker, @@ -44,10 +50,16 @@ pub(crate) fn run( docker_user_id(&mut docker, engine.kind); docker - .args(&["-v", &format!("{}:/xargo:Z", dirs.xargo.to_utf8()?)]) - .args(&["-v", &format!("{}:/cargo:Z", dirs.cargo.to_utf8()?)]) + .args(&[ + "-v", + &format!("{}:{}:Z", dirs.xargo.to_utf8()?, dirs.xargo_mount_path()), + ]) + .args(&[ + "-v", + &format!("{}:{}:Z", dirs.cargo.to_utf8()?, dirs.cargo_mount_path()), + ]) // Prevent `bin` from being mounted inside the Docker container. - .args(&["-v", "/cargo/bin"]); + .args(&["-v", &format!("{}/bin", dirs.cargo_mount_path())]); docker.args(&[ "-v", &format!("{}:{}:Z", dirs.host_root.to_utf8()?, dirs.mount_root), @@ -55,7 +67,11 @@ pub(crate) fn run( docker .args(&[ "-v", - &format!("{}:/rust:Z,ro", paths.get_sysroot().to_utf8()?), + &format!( + "{}:{}:Z,ro", + dirs.get_sysroot().to_utf8()?, + dirs.sysroot_mount_path() + ), ]) .args(&["-v", &format!("{}:/target:Z", dirs.target.to_utf8()?)]); docker_cwd(&mut docker, &paths)?; @@ -84,7 +100,7 @@ pub(crate) fn run( docker .arg(&image_name) - .args(&["sh", "-c", &format!("PATH=$PATH:/rust/bin {:?}", cmd)]) + .args(&["sh", "-c", &build_command(dirs, &cmd)]) .run_and_get_status(msg_info, false) .map_err(Into::into) } diff --git a/src/docker/remote.rs b/src/docker/remote.rs index 5a97a807c..bdb2926e8 100644 --- a/src/docker/remote.rs +++ b/src/docker/remote.rs @@ -241,24 +241,19 @@ fn copy_volume_files_nocache( pub fn copy_volume_container_xargo( engine: &Engine, container: &str, - xargo_dir: &Path, - target: &Target, + dirs: &Directories, mount_prefix: &Path, msg_info: &mut MessageInfo, ) -> Result<()> { - // only need to copy the rustlib files for our current target. - let triple = target.triple(); - let relpath = Path::new("lib").join("rustlib").join(&triple); - let src = xargo_dir.join(&relpath); - let dst = mount_prefix.join("xargo").join(&relpath); - if Path::new(&src).exists() { + let dst = mount_prefix.join(&dirs.xargo_mount_path_relative()?); + if dirs.xargo.exists() { create_volume_dir( engine, container, dst.parent().expect("destination should have a parent"), msg_info, )?; - copy_volume_files(engine, container, &src, &dst, msg_info)?; + copy_volume_files(engine, container, &dirs.xargo, &dst, msg_info)?; } Ok(()) @@ -267,23 +262,23 @@ pub fn copy_volume_container_xargo( pub fn copy_volume_container_cargo( engine: &Engine, container: &str, - cargo_dir: &Path, + dirs: &Directories, mount_prefix: &Path, copy_registry: bool, msg_info: &mut MessageInfo, ) -> Result<()> { - let dst = mount_prefix.join("cargo"); + let dst = mount_prefix.join(&dirs.cargo_mount_path_relative()?); let copy_registry = env::var("CROSS_REMOTE_COPY_REGISTRY") .map(|s| bool_from_envvar(&s)) .unwrap_or(copy_registry); if copy_registry { - copy_volume_files(engine, container, cargo_dir, &dst, msg_info)?; + copy_volume_files(engine, container, &dirs.cargo, &dst, msg_info)?; } else { // can copy a limit subset of files: the rest is present. create_volume_dir(engine, container, &dst, msg_info)?; - for entry in fs::read_dir(cargo_dir) - .wrap_err_with(|| format!("when reading directory {cargo_dir:?}"))? + for entry in fs::read_dir(&dirs.cargo) + .wrap_err_with(|| format!("when reading directory {:?}", dirs.cargo))? { let file = entry?; let basename = file @@ -371,17 +366,17 @@ fn warn_symlinks(had_symlinks: bool, msg_info: &mut MessageInfo) -> Result<()> { fn copy_volume_container_rust_base( engine: &Engine, container: &str, - sysroot: &Path, + dirs: &Directories, mount_prefix: &Path, msg_info: &mut MessageInfo, ) -> Result<()> { // the rust toolchain is quite large, but most of it isn't needed // we need the bin, libexec, and etc directories, and part of the lib directory. - let dst = mount_prefix.join("rust"); + let dst = mount_prefix.join(&dirs.sysroot_mount_path_relative()?); let rustlib = Path::new("lib").join("rustlib"); create_volume_dir(engine, container, &dst.join(&rustlib), msg_info)?; for basename in ["bin", "libexec", "etc"] { - let file = sysroot.join(basename); + let file = dirs.get_sysroot().join(basename); copy_volume_files(engine, container, &file, &dst, msg_info)?; } @@ -393,9 +388,9 @@ fn copy_volume_container_rust_base( // SAFETY: safe, single-threaded execution. let tempdir = unsafe { temp::TempDir::new()? }; let temppath = tempdir.path(); - fs::create_dir_all(&temppath.join(&rustlib))?; + file::create_dir_all(&temppath.join(&rustlib))?; let mut had_symlinks = copy_dir( - &sysroot.join("lib"), + &dirs.get_sysroot().join("lib"), &temppath.join("lib"), true, 0, @@ -404,7 +399,7 @@ fn copy_volume_container_rust_base( // next, copy the src/etc directories inside rustlib had_symlinks |= copy_dir( - &sysroot.join(&rustlib), + &dirs.get_sysroot().join(&rustlib), &temppath.join(&rustlib), true, 0, @@ -418,21 +413,21 @@ fn copy_volume_container_rust_base( fn copy_volume_container_rust_manifest( engine: &Engine, container: &str, - sysroot: &Path, + dirs: &Directories, mount_prefix: &Path, msg_info: &mut MessageInfo, ) -> Result<()> { // copy over all the manifest files in rustlib // these are small text files containing names/paths to toolchains - let dst = mount_prefix.join("rust"); + let dst = mount_prefix.join(&dirs.sysroot_mount_path_relative()?); let rustlib = Path::new("lib").join("rustlib"); // SAFETY: safe, single-threaded execution. let tempdir = unsafe { temp::TempDir::new()? }; let temppath = tempdir.path(); - fs::create_dir_all(&temppath.join(&rustlib))?; + file::create_dir_all(&temppath.join(&rustlib))?; let had_symlinks = copy_dir( - &sysroot.join(&rustlib), + &dirs.get_sysroot().join(&rustlib), &temppath.join(&rustlib), true, 0, @@ -447,18 +442,20 @@ fn copy_volume_container_rust_manifest( pub fn copy_volume_container_rust_triple( engine: &Engine, container: &str, - toolchain: &QualifiedToolchain, + dirs: &Directories, target_triple: &TargetTriple, mount_prefix: &Path, skip_exists: bool, msg_info: &mut MessageInfo, ) -> Result<()> { - let sysroot = toolchain.get_sysroot(); // copy over the files for a specific triple - let dst = mount_prefix.join("rust"); + let dst = mount_prefix.join(&dirs.sysroot_mount_path_relative()?); let rustlib = Path::new("lib").join("rustlib"); let dst_rustlib = dst.join(&rustlib); - let src_toolchain = sysroot.join(&rustlib).join(target_triple.triple()); + let src_toolchain = dirs + .get_sysroot() + .join(&rustlib) + .join(target_triple.triple()); let dst_toolchain = dst_rustlib.join(target_triple.triple()); // skip if the toolchain target component already exists. for the host toolchain @@ -473,7 +470,7 @@ pub fn copy_volume_container_rust_triple( if !skip && skip_exists { // this means we have a persistent data volume and we have a // new target, meaning we might have new manifests as well. - copy_volume_container_rust_manifest(engine, container, sysroot, mount_prefix, msg_info)?; + copy_volume_container_rust_manifest(engine, container, dirs, mount_prefix, msg_info)?; } Ok(()) @@ -482,41 +479,28 @@ pub fn copy_volume_container_rust_triple( pub fn copy_volume_container_rust( engine: &Engine, container: &str, - toolchain: &QualifiedToolchain, + dirs: &Directories, target_triple: Option<&TargetTriple>, mount_prefix: &Path, msg_info: &mut MessageInfo, ) -> Result<()> { - copy_volume_container_rust_base( - engine, - container, - toolchain.get_sysroot(), - mount_prefix, - msg_info, - )?; - copy_volume_container_rust_manifest( - engine, - container, - toolchain.get_sysroot(), - mount_prefix, - msg_info, - )?; + copy_volume_container_rust_base(engine, container, dirs, mount_prefix, msg_info)?; + copy_volume_container_rust_manifest(engine, container, dirs, mount_prefix, msg_info)?; copy_volume_container_rust_triple( engine, container, - toolchain, - &toolchain.host().target, + dirs, + &dirs.toolchain.host().target, mount_prefix, false, msg_info, )?; - // TODO: impl Eq if let Some(target_triple) = target_triple { - if target_triple.triple() != toolchain.host().target.triple() { + if target_triple.triple() != dirs.toolchain.host().target.triple() { copy_volume_container_rust_triple( engine, container, - toolchain, + dirs, target_triple, mount_prefix, false, @@ -631,7 +615,7 @@ fn copy_volume_file_list( for file in files { let src_path = src.join(file); let dst_path = temppath.join(file); - fs::create_dir_all(dst_path.parent().expect("must have parent"))?; + file::create_dir_all(dst_path.parent().expect("must have parent"))?; fs::copy(&src_path, &dst_path)?; } copy_volume_files(engine, container, temppath, dst, msg_info) @@ -698,7 +682,7 @@ fn copy_volume_container_project( match volume { VolumeId::Keep(_) => { let parent = temp::dir()?; - fs::create_dir_all(&parent)?; + file::create_dir_all(&parent)?; let fingerprint = parent.join(container); let current = get_project_fingerprint(src, copy_cache)?; // need to check if the container path exists, otherwise we might @@ -997,28 +981,14 @@ pub(crate) fn run( }; let mount_prefix_path = mount_prefix.as_ref(); if let VolumeId::Discard = volume { - copy_volume_container_xargo( - engine, - &container, - &dirs.xargo, - target, - mount_prefix_path, - msg_info, - ) - .wrap_err("when copying xargo")?; - copy_volume_container_cargo( - engine, - &container, - &dirs.cargo, - mount_prefix_path, - false, - msg_info, - ) - .wrap_err("when copying cargo")?; + copy_volume_container_xargo(engine, &container, dirs, mount_prefix_path, msg_info) + .wrap_err("when copying xargo")?; + copy_volume_container_cargo(engine, &container, dirs, mount_prefix_path, false, msg_info) + .wrap_err("when copying cargo")?; copy_volume_container_rust( engine, &container, - &dirs.toolchain, + dirs, Some(target.target()), mount_prefix_path, msg_info, @@ -1029,7 +999,7 @@ pub(crate) fn run( copy_volume_container_rust_triple( engine, &container, - &dirs.toolchain, + dirs, target.target(), mount_prefix_path, true, @@ -1066,9 +1036,18 @@ pub(crate) fn run( .wrap_err("when copying project")?; let sysroot = dirs.get_sysroot().to_owned(); let mut copied = vec![ - (&dirs.xargo, mount_prefix_path.join("xargo")), - (&dirs.cargo, mount_prefix_path.join("cargo")), - (&sysroot, mount_prefix_path.join("rust")), + ( + &dirs.xargo, + mount_prefix_path.join(&dirs.xargo_mount_path_relative()?), + ), + ( + &dirs.cargo, + mount_prefix_path.join(&dirs.cargo_mount_path_relative()?), + ), + ( + &sysroot, + mount_prefix_path.join(&dirs.sysroot_mount_path_relative()?), + ), (&dirs.host_root, mount_root.clone()), ]; let mut to_symlink = vec![]; @@ -1188,10 +1167,10 @@ symlink_recurse \"${{prefix}}\" // 6. execute our cargo command inside the container let mut docker = subcommand(engine, "exec"); docker_user_id(&mut docker, engine.kind); - docker_envvars(&mut docker, &options.config, target, msg_info)?; + docker_envvars(&mut docker, &options.config, dirs, target, msg_info)?; docker_cwd(&mut docker, &paths)?; docker.arg(&container); - docker.args(&["sh", "-c", &format!("PATH=$PATH:/rust/bin {:?}", cmd)]); + docker.args(&["sh", "-c", &build_command(dirs, &cmd)]); bail_container_exited!(); let status = docker .run_and_get_status(msg_info, false) diff --git a/src/docker/shared.rs b/src/docker/shared.rs index 7e1459b93..1e572b095 100644 --- a/src/docker/shared.rs +++ b/src/docker/shared.rs @@ -256,6 +256,9 @@ pub struct Directories { pub mount_root: String, pub mount_cwd: String, pub toolchain: QualifiedToolchain, + pub cargo_mount_path: String, + pub xargo_mount_path: String, + pub sysroot_mount_path: String, } impl Directories { @@ -277,10 +280,22 @@ impl Directories { // otherwise `docker` will create them but they will be owned by `root` // cargo builds all intermediate directories, but fails // if it has other issues (such as permission errors). - fs::create_dir_all(&cargo)?; - fs::create_dir_all(&xargo)?; + file::create_dir_all(&cargo)?; + file::create_dir_all(&xargo)?; + if let Some(ref nix_store) = nix_store { + file::create_dir_all(nix_store)?; + } create_target_dir(target)?; + // now that we know the paths exist, canonicalize them. this avoids creating + // directories after failed canonicalization into a shared directory. + let cargo = file::canonicalize(&cargo)?; + let xargo = file::canonicalize(&xargo)?; + let nix_store = match nix_store { + Some(store) => Some(file::canonicalize(&store)?), + None => None, + }; + let cargo = mount_finder.find_mount_path(cargo); let xargo = mount_finder.find_mount_path(xargo); let target = mount_finder.find_mount_path(target); @@ -293,21 +308,17 @@ impl Directories { }); // root is either workspace_root, or, if we're outside the workspace root, the current directory - let mount_root: 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_root = host_root.as_wslpath()?; - } - #[cfg(not(target_os = "windows"))] - { - // NOTE: host root has already found the mount path - mount_root = host_root.to_utf8()?.to_owned(); - } + // NOTE: host root has already found the mount path + let mount_root = canonicalize_mount_path(&host_root)?; let mount_cwd = mount_finder.find_path(cwd, false)?; toolchain.set_sysroot(|p| mount_finder.find_mount_path(p)); + // canonicalize these once to avoid syscalls + let cargo_mount_path = canonicalize_mount_path(&cargo)?; + let xargo_mount_path = canonicalize_mount_path(&xargo)?; + let sysroot_mount_path = canonicalize_mount_path(toolchain.get_sysroot())?; + Ok(Directories { cargo, xargo, @@ -317,12 +328,48 @@ impl Directories { mount_root, mount_cwd, toolchain, + cargo_mount_path, + xargo_mount_path, + sysroot_mount_path, }) } pub fn get_sysroot(&self) -> &Path { self.toolchain.get_sysroot() } + + pub fn cargo_mount_path(&self) -> &str { + &self.cargo_mount_path + } + + pub fn xargo_mount_path(&self) -> &str { + &self.xargo_mount_path + } + + pub fn sysroot_mount_path(&self) -> &str { + &self.sysroot_mount_path + } + + pub fn cargo_mount_path_relative(&self) -> Result { + self.cargo_mount_path() + .strip_prefix('/') + .map(ToOwned::to_owned) + .ok_or_else(|| eyre::eyre!("cargo directory must be relative to root")) + } + + pub fn xargo_mount_path_relative(&self) -> Result { + self.xargo_mount_path() + .strip_prefix('/') + .map(ToOwned::to_owned) + .ok_or_else(|| eyre::eyre!("xargo directory must be relative to root")) + } + + pub fn sysroot_mount_path_relative(&self) -> Result { + self.sysroot_mount_path() + .strip_prefix('/') + .map(ToOwned::to_owned) + .ok_or_else(|| eyre::eyre!("sysroot directory must be relative to root")) + } } const CACHEDIR_TAG: &str = "Signature: 8a477f597d28d172789f06886806bc55 @@ -333,7 +380,7 @@ fn create_target_dir(path: &Path) -> Result<()> { // cargo creates all paths to the target directory, and writes // a cache dir tag only if the path doesn't previously exist. if !path.exists() { - fs::create_dir_all(&path)?; + file::create_dir_all(&path)?; fs::OpenOptions::new() .write(true) .create_new(true) @@ -467,6 +514,7 @@ pub(crate) fn mount(docker: &mut Command, host_path: &Path, prefix: &str) -> Res pub(crate) fn docker_envvars( docker: &mut Command, config: &Config, + dirs: &Directories, target: &Target, msg_info: &mut MessageInfo, ) -> Result<()> { @@ -482,8 +530,8 @@ pub(crate) fn docker_envvars( let cross_runner = format!("CROSS_RUNNER={}", runner.unwrap_or_default()); docker .args(&["-e", "PKG_CONFIG_ALLOW_CROSS=1"]) - .args(&["-e", "XARGO_HOME=/xargo"]) - .args(&["-e", "CARGO_HOME=/cargo"]) + .args(&["-e", &format!("XARGO_HOME={}", dirs.xargo_mount_path())]) + .args(&["-e", &format!("CARGO_HOME={}", dirs.cargo_mount_path())]) .args(&["-e", "CARGO_TARGET_DIR=/target"]) .args(&["-e", &cross_runner]); add_cargo_configuration_envvars(docker); @@ -513,6 +561,14 @@ pub(crate) fn docker_envvars( Ok(()) } +pub(crate) fn build_command(dirs: &Directories, cmd: &SafeCommand) -> String { + format!( + "PATH=\"$PATH\":\"{}/bin\" {:?}", + dirs.sysroot_mount_path(), + cmd + ) +} + pub(crate) fn docker_cwd(docker: &mut Command, paths: &DockerPaths) -> Result<()> { docker.args(&["-w", paths.mount_cwd()]); @@ -961,6 +1017,7 @@ mod tests { mod directories { use super::*; use crate::cargo::cargo_metadata_with_args; + use crate::rustc::{self, VersionMetaExt}; use crate::temp; fn unset_env() -> Vec<(&'static str, Option)> { @@ -1020,14 +1077,18 @@ mod tests { } fn get_toolchain() -> Result { + let host_version_meta = rustc::version_meta()?; + let host = host_version_meta.host(); + let image_platform = + crate::docker::ImagePlatform::from_const_target(host.triple().into()); let sysroot = home()? .join(".rustup") .join("toolchains") - .join("stable-x86_64-unknown-linux-gnu"); + .join(host.triple()); Ok(QualifiedToolchain::new( "stable", &None, - &crate::docker::ImagePlatform::X86_64_UNKNOWN_LINUX_GNU, + &image_platform, &sysroot, )) } diff --git a/src/file.rs b/src/file.rs index c2c976cfd..bebf2050b 100644 --- a/src/file.rs +++ b/src/file.rs @@ -143,6 +143,11 @@ where read_(path.as_ref()) } +pub fn create_dir_all(path: impl AsRef) -> Result<()> { + fs::create_dir_all(path.as_ref()) + .wrap_err_with(|| format!("couldn't create directory {:?}", path.as_ref())) +} + fn read_(path: &Path) -> Result { let mut s = String::new(); File::open(path) @@ -222,12 +227,11 @@ pub fn maybe_canonicalize(path: &Path) -> Cow<'_, OsStr> { pub fn write_file(path: impl AsRef, overwrite: bool) -> Result { let path = path.as_ref(); - fs::create_dir_all( + create_dir_all( &path .parent() .ok_or_else(|| eyre::eyre!("could not find parent directory for `{path:?}`"))?, - ) - .wrap_err_with(|| format!("couldn't create directory `{path:?}`"))?; + )?; let mut open = fs::OpenOptions::new(); open.write(true); diff --git a/src/lib.rs b/src/lib.rs index 58dd6dd12..b147006b8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -235,6 +235,12 @@ impl<'a> From<&'a str> for TargetTriple { } } +impl Default for TargetTriple { + fn default() -> TargetTriple { + TargetTriple::DEFAULT + } +} + impl std::str::FromStr for TargetTriple { type Err = std::convert::Infallible; @@ -367,6 +373,12 @@ impl Target { } } +impl Default for Target { + fn default() -> Target { + Target::DEFAULT + } +} + impl std::fmt::Display for Target { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(self.triple())