Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with 0.2.2 and CROSS_DOCKER_IN_DOCKER #893

Closed
4 of 11 tasks
alexet opened this issue Jul 1, 2022 · 5 comments · Fixed by #898
Closed
4 of 11 tasks

Problem with 0.2.2 and CROSS_DOCKER_IN_DOCKER #893

alexet opened this issue Jul 1, 2022 · 5 comments · Fixed by #898
Assignees
Labels
A-container-engine Area: container engines bug
Milestone

Comments

@alexet
Copy link

alexet commented Jul 1, 2022

Checklist

Describe your issue

Cross no longer seems to work with our project with docker in docker.

Not using docker or downgrading fixes the issue. Not using a path for a dependency also works.

The error is error: could not find Cargo.tomlin/home/runner/work/cross-bug-reduced/cross-bug-reduced or any parent directory

What target(s) are you cross-compiling for?

x86_64-unknown-linux-gnu

Which operating system is the host (e.g computer cross is on) running?

  • macOS
  • Windows
  • Linux / BSD
  • other OS (specify in description)

What architecture is the host?

  • x86_64 / AMD64
  • arm32
  • arm64 (including Mac M1)

What container engine is cross using?

  • docker
  • podman
  • other container engine (specify in description)

cross version

cross 0.2.2

Example

The example is reduced from an internal case and is therefore pointless.

Inside a docker image do the following steps:

Additional information / notes

Working run with cross 0.2.1 : https://github.com/alexet/cross-bug-reduced/runs/7151280983
Broken run with 0. 2.2: https://github.com/alexet/cross-bug-reduced/actions/runs/2597098367
Working run with 0.2.2 without docker: https://github.com/alexet/cross-bug-reduced/actions/runs/2597098367
Working run without dependency: https://github.com/alexet/cross-bug-reduced/runs/7149368098

@Alexhuszagh Alexhuszagh added bug A-container-engine Area: container engines labels Jul 1, 2022
@Emilgardis
Copy link
Member

Emilgardis commented Jul 1, 2022

I think there's something wrong with the mountfinder and path deps or env.volumes in container in container mode

Working mount (0.2.1, pre #684 )

-v /home/runner/work/cross-bug-reduced/cross-bug-reduced:/project:Z

Failed mount (0.2.2)

-v /home/runner/work/cross-bug-reduced/cross-bug-reduced:/var/lib/docker/overlay2/800098c600aa830941b6dbb41b4cd64252d2018d17e74fd4d8b39f34a95b65cc/merged/home/runner/work/cross-bug-reduced/cross-bug-reduced:Z

@Alexhuszagh
Copy link
Contributor

Interesting, I wonder if this has anything to do with the introduction of Directories (I believe introduced with remote docker support).

@Alexhuszagh
Copy link
Contributor

Alexhuszagh commented Jul 1, 2022

After bisecting, it looks like the offending commit with the mount paths was 96246b2. However, it didn't work for me due to inspect issues for 4257c41, and that issue was introduced in c59aa93. See if it works on 150c892 (unrelated issue, possibly not a real issue) and 1f07053 for you.

Looks like it was an issue in refactoring I introduced. I'll try to fix this.

That commit (for remote Docker infra) made changes to how the mount directories worked (just in restructuring), so that was likely it:

96246b2#diff-b0f71fc97387d26459b3d22f08eaae7b431074839cce7cb6aacf3cd154db9800R36-R109

@Alexhuszagh Alexhuszagh self-assigned this Jul 1, 2022
@Alexhuszagh
Copy link
Contributor

Alexhuszagh commented Jul 1, 2022

Ok changing the mount paths for the project, which for some reason seems to "double-up" for the mount paths.

/usr/bin/docker run --userns host -e 'PKG_CONFIG_ALLOW_CROSS=1' -e 'XARGO_HOME=/xargo' -e 'CARGO_HOME=/cargo' -e 'CARGO_TARGET_DIR=/target' -e 'CROSS_RUNNER=' -e TERM -e 'USER=root' \
    -v /cross-bug-reduced/dependency:/cross-bug-reduced/dependency --rm --user 0:0 \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/root/.xargo:/xargo:Z \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/root/.cargo:/cargo:Z \
    -v /cargo/bin \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced:/var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced:Z \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu:/rust:Z,ro \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced/target:/target:Z -w /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced -i -t centos/devtoolset-7-toolchain-centos7 sh -c 'PATH=$PATH:/rust/bin cargo build --target x86_64-unknown-linux-gnu --verbose'

This does work:

/usr/bin/docker run --userns host -e 'PKG_CONFIG_ALLOW_CROSS=1' -e 'XARGO_HOME=/xargo' -e 'CARGO_HOME=/cargo' -e 'CARGO_TARGET_DIR=/target' -e 'CROSS_RUNNER=' -e TERM -e 'USER=root' \
    -v /cross-bug-reduced/dependency:/cross-bug-reduced/dependency --rm --user 0:0 \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/root/.xargo:/xargo:Z \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/root/.cargo:/cargo:Z \
    -v /cargo/bin \
    -v "/var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced:/project:Z" \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu:/rust:Z,ro \
    -v /var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced/target:/target:Z \
    -w /project -i -t centos/devtoolset-7-toolchain-centos7 sh -c 'PATH=$PATH:/rust/bin cargo build --target x86_64-unknown-linux-gnu --verbose'

If we set PREFIX=/var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged, then the only difference is we have:

- "$PREFIX/cross-bug-reduced:/project:Z"
+ $PREFIX/cross-bug-reduced:$PREFIX/var/lib/docker/overlay2/ef1b7713a2d64f694cb589f8802b70af301485955850d3a0083ca1fb4817de3e/merged/cross-bug-reduced

So we're getting double the prefix for the mount path.

Update

The following diff fixes the issue. Going to see if this was an accident or an intentional change.

diff --git a/src/docker/shared.rs b/src/docker/shared.rs
index fb3f6b3..57a4db9 100644
--- a/src/docker/shared.rs
+++ b/src/docker/shared.rs
@@ -89,10 +89,7 @@ impl Directories {
         }
         #[cfg(not(target_os = "windows"))]
         {
-            mount_root = mount_finder
-                .find_mount_path(host_root.clone())
-                .to_utf8()?
-                .to_string();
+            mount_root = host_root.to_utf8()?.to_string();
         }
         let mount_cwd: String;
         #[cfg(target_os = "windows")]

@Alexhuszagh Alexhuszagh added this to the v0.2.3 milestone Jul 1, 2022
@Alexhuszagh
Copy link
Contributor

It looks like this was the offending commit, and the issue was a weird rebase or something:

A new function was introduced, which exhibits the correct behavior, but

#[allow(unused_variables)]
pub fn mount(cmd: &mut Command, val: &Path, verbose: bool) -> Result<PathBuf> {
    let host_path =
        file::canonicalize(&val).wrap_err_with(|| format!("when canonicalizing path `{val:?}`"))?;
    let mount_path: PathBuf;
    #[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_path = wslpath(&host_path, verbose)?;
    }
    #[cfg(not(target_os = "windows"))]
    {
        mount_path = host_path.clone();
    }
    cmd.args(&[
        "-v",
        &format!("{}:{}", host_path.display(), mount_path.display()),
    ]);
    Ok(mount_path)
}
     let mount_root: PathBuf;
     #[cfg(target_os = "windows")]
     {
@@ -122,7 +147,17 @@ pub fn run(
     }
     #[cfg(not(target_os = "windows"))]
     {
-        mount_root = host_root.clone();
+        mount_root = mount_finder.find_mount_path(host_root.clone());
+    }

This means we didn't notice it until I removed the redundant mount code later, which uncovered the bad rebase or whatever it was here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-container-engine Area: container engines bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants