Skip to content

Commit

Permalink
Fix paths when using CROSS_CONTAINER_IN_CONTAINER (#1483)
Browse files Browse the repository at this point in the history
Currently, `MountFinder` is used for almost all paths, rather than only
for the host path in bind/volume mounts.

Because of this, `docker build` invocations use the wrong working
directory (`/var/lib/docker/.../workdir-path` instead of
`/workdir-path`), causing failures for builds that use `pre-build` with
a relative file path, and `docker run` invocations use incorrect `-v`
arguments (e.g.
`/var/lib/docker/.../toolchain-dir:/var/lib/docker/.../toolchain-dir`
instead of `/var/lib/docker/.../toolchain-dir:/toolchain-dir`).

This PR fixes those issues by only using `find_path` and
`find_mount_path` when necessary.
  • Loading branch information
Emilgardis authored May 6, 2024
2 parents 22fcf3d + 109737f commit 8b27866
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 29 deletions.
4 changes: 4 additions & 0 deletions .changes/1483.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"description": "Fix paths when using `CROSS_CONTAINER_IN_CONTAINER`",
"type": "fixed"
}
9 changes: 7 additions & 2 deletions src/docker/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,25 @@ pub(crate) fn run(
// Prevent `bin` from being mounted inside the Docker container.
.args(["-v", &format!("{}/bin", toolchain_dirs.cargo_mount_path())]);

let host_root = paths.mount_finder.find_mount_path(package_dirs.host_root());
docker.args([
"-v",
&format!(
"{}:{}{selinux}",
package_dirs.host_root().to_utf8()?,
host_root.to_utf8()?,
package_dirs.mount_root()
),
]);

let sysroot = paths
.mount_finder
.find_mount_path(toolchain_dirs.get_sysroot());
docker
.args([
"-v",
&format!(
"{}:{}{selinux_ro}",
toolchain_dirs.get_sysroot().to_utf8()?,
sysroot.to_utf8()?,
toolchain_dirs.sysroot_mount_path()
),
])
Expand Down
39 changes: 12 additions & 27 deletions src/docker/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ pub struct ToolchainDirectories {
}

impl ToolchainDirectories {
pub fn assemble(mount_finder: &MountFinder, mut toolchain: QualifiedToolchain) -> Result<Self> {
pub fn assemble(mount_finder: &MountFinder, toolchain: QualifiedToolchain) -> Result<Self> {
let home_dir =
home::home_dir().ok_or_else(|| eyre::eyre!("could not find home directory"))?;
let cargo = home::cargo_home()?;
Expand Down Expand Up @@ -311,8 +311,6 @@ impl ToolchainDirectories {
let cargo = mount_finder.find_mount_path(cargo);
let xargo = mount_finder.find_mount_path(xargo);

toolchain.set_sysroot(|p| mount_finder.find_mount_path(p));

// canonicalize these once to avoid syscalls
let sysroot_mount_path = toolchain.get_sysroot().as_posix_absolute()?;

Expand Down Expand Up @@ -413,30 +411,29 @@ pub struct PackageDirectories {
impl PackageDirectories {
pub fn assemble(
mount_finder: &MountFinder,
mut metadata: CargoMetadata,
metadata: CargoMetadata,
cwd: &Path,
) -> Result<(Self, CargoMetadata)> {
let target = &metadata.target_directory;
// see ToolchainDirectories::assemble for creating directories
create_target_dir(target)?;

metadata.target_directory = mount_finder.find_mount_path(target);

// root is either workspace_root, or, if we're outside the workspace root, the current directory
let host_root = mount_finder.find_mount_path(if metadata.workspace_root.starts_with(cwd) {
let host_root = if metadata.workspace_root.starts_with(cwd) {
cwd
} else {
&metadata.workspace_root
});
}
.to_path_buf();

// on Windows, we can not mount the directory name directly. Instead, we use wslpath to convert the path to a linux compatible path.
// NOTE: on unix, host root has already found the mount path
let mount_root = host_root.as_posix_absolute()?;
let mount_cwd = mount_finder.find_path(cwd, false)?;
let mount_cwd = cwd.as_posix_absolute()?;

Ok((
PackageDirectories {
target: metadata.target_directory.clone(),
target: mount_finder.find_mount_path(target),
host_root,
mount_root,
mount_cwd,
Expand Down Expand Up @@ -1193,10 +1190,7 @@ impl DockerCommandExt for Command {
if let Ok(val) = value {
let canonical_path = file::canonicalize(&val)?;
let host_path = paths.mount_finder.find_path(&canonical_path, true)?;
let absolute_path = Path::new(&val).as_posix_absolute()?;
let mount_path = paths
.mount_finder
.find_path(Path::new(&absolute_path), true)?;
let mount_path = Path::new(&val).as_posix_absolute()?;
mount_cb(self, host_path.as_ref(), mount_path.as_ref())?;
self.args(["-e", &format!("{}={}", var, mount_path)]);
store_cb((val, mount_path));
Expand All @@ -1209,10 +1203,7 @@ impl DockerCommandExt for Command {
// to the mounted project directory.
let canonical_path = file::canonicalize(path)?;
let host_path = paths.mount_finder.find_path(&canonical_path, true)?;
let absolute_path = Path::new(path).as_posix_absolute()?;
let mount_path = paths
.mount_finder
.find_path(Path::new(&absolute_path), true)?;
let mount_path = path.as_posix_absolute()?;
mount_cb(self, host_path.as_ref(), mount_path.as_ref())?;
store_cb((path.to_utf8()?.to_owned(), mount_path));
}
Expand Down Expand Up @@ -1716,15 +1707,9 @@ mod tests {

paths_equal(toolchain_dirs.cargo(), &mount_path(home()?.join(".cargo")))?;
paths_equal(toolchain_dirs.xargo(), &mount_path(home()?.join(".xargo")))?;
paths_equal(package_dirs.host_root(), &mount_path(get_cwd()?))?;
assert_eq!(
package_dirs.mount_root(),
&mount_path(get_cwd()?).as_posix_absolute()?
);
assert_eq!(
package_dirs.mount_cwd(),
&mount_path(get_cwd()?).as_posix_absolute()?
);
paths_equal(package_dirs.host_root(), &get_cwd()?)?;
assert_eq!(package_dirs.mount_root(), &get_cwd()?.as_posix_absolute()?);
assert_eq!(package_dirs.mount_cwd(), &get_cwd()?.as_posix_absolute()?);

reset_env(vars);
Ok(())
Expand Down

0 comments on commit 8b27866

Please sign in to comment.