address code review

src/bin/commands/containers.rs

@@ -3,7 +3,7 @@ use std::io;
 use clap::{Args, Subcommand};
 use cross::shell::{self, MessageInfo, Stream};
 use cross::{
- docker::{self, ImageArchitecture},
+ docker::{self, ImagePlatform},
  rustc::{QualifiedToolchain, Toolchain},
  CommandExt,
 };
@@ -408,7 +408,7 @@ pub fn create_persistent_volume(
  triple: cross::TargetTriple::X86_64UnknownLinuxGnu,
  };
  let mut toolchain = QualifiedToolchain::default(msg_info)?;
- toolchain.replace_host(&ImageArchitecture::from_target(target.target().clone()));
+ toolchain.replace_host(&ImagePlatform::from_target(target.target().clone())?);
  if let Some(channel) = channel {
  toolchain = toolchain.with_picked(channel.clone(), msg_info)?;
  };
@@ -469,7 +469,7 @@ pub fn create_persistent_volume(
  docker::remote::copy_volume_container_rust(
  engine,
  &container,
- &dirs.sysroot,
+ &toolchain,
  target.target(),
  mount_prefix.as_ref(),
  true,
@@ -499,7 +499,7 @@ pub fn remove_persistent_volume(
  triple: cross::TargetTriple::X86_64UnknownLinuxGnu,
  };
  let mut toolchain = QualifiedToolchain::default(msg_info)?;
- toolchain.replace_host(&ImageArchitecture::from_target(target.target().clone()));
+ toolchain.replace_host(&ImagePlatform::from_target(target.target().clone())?);
  if let Some(channel) = channel {
  toolchain = toolchain.with_picked(channel.clone(), msg_info)?;
  };

src/config.rs

@@ -1,4 +1,4 @@
-use crate::docker::{ImageArchitecture, PossibleImage};
+use crate::docker::{ImagePlatform, PossibleImage};
 use crate::shell::{self, MessageInfo};
 use crate::{CrossToml, Result, Target, TargetList};
 
@@ -68,10 +68,10 @@ impl Environment {
 
  fn image(&self, target: &Target) -> Result<Option<PossibleImage>> {
  self.get_target_var(target, "IMAGE")
- .map(|i| i.parse().expect("infallible"))
+ .map(Into::into)
  .map(|mut i: PossibleImage| {
  if let Some(toolchain) = self.get_target_var(target, "IMAGE_TOOLCHAIN") {
- i.toolchain = Some(ImageArchitecture::from_target(toolchain.into()));
+ i.toolchain = Some(ImagePlatform::from_target(toolchain.into())?);
  Ok(i)
  } else {
  Ok(i)

src/cross_toml.rs

@@ -218,12 +218,12 @@ impl CrossToml {
 
  /// Returns the `target.{}.image` part of `Cross.toml`
  pub fn image(&self, target: &Target) -> Option<PossibleImage> {
- self.get_string(target, |_| None, |t| t.image.as_ref())
+ self.get_config(target, |_| None, |t| t.image.as_ref())
  }
 
  /// Returns the `{}.dockerfile` or `{}.dockerfile.file` part of `Cross.toml`
  pub fn dockerfile(&self, target: &Target) -> Option<String> {
- self.get_string(
+ self.get_config(
  target,
  |b| b.dockerfile.as_ref().map(|c| &c.file),
  |t| t.dockerfile.as_ref().map(|c| &c.file),
@@ -232,7 +232,7 @@ impl CrossToml {
 
  /// Returns the `target.{}.dockerfile.context` part of `Cross.toml`
  pub fn dockerfile_context(&self, target: &Target) -> Option<String> {
- self.get_string(
+ self.get_config(
  target,
  |b| b.dockerfile.as_ref().and_then(|c| c.context.as_ref()),
  |t| t.dockerfile.as_ref().and_then(|c| c.context.as_ref()),
@@ -266,7 +266,7 @@ impl CrossToml {
 
  /// Returns the `target.{}.runner` part of `Cross.toml`
  pub fn runner(&self, target: &Target) -> Option<String> {
- self.get_string(target, |_| None, |t| t.runner.as_ref())
+ self.get_config(target, |_| None, |t| t.runner.as_ref())
  }
 
  /// Returns the `build.xargo` or the `target.{}.xargo` part of `Cross.toml`
@@ -310,7 +310,7 @@ impl CrossToml {
  self.targets.get(target)
  }
 
- fn get_string<'a, T: Clone + 'a>(
+ fn get_config<'a, T: Clone + 'a>(
  &'a self,
  target: &Target,
  get_build: impl Fn(&'a CrossBuildConfig) -> Option<&'a T>,
@@ -406,15 +406,15 @@ where
 
 #[cfg(test)]
 mod tests {
- use crate::docker::ImageArchitecture;
+ use crate::docker::ImagePlatform;
 
  use super::*;
  const MSG_INFO: MessageInfo = MessageInfo {
  color_choice: shell::ColorChoice::Never,
  verbosity: shell::Verbosity::Quiet,
  };
 
- macro_rules! s {
+ macro_rules! p {
  ($x:literal) => {
  $x.parse()?
  };
@@ -482,7 +482,7 @@ mod tests {
  },
  xargo: Some(false),
  build_std: Some(true),
- image: Some("test-image".parse().expect("infallible")),
+ image: Some("test-image".into()),
  runner: None,
  dockerfile: None,
  pre_build: Some(vec![]),
@@ -524,9 +524,9 @@ mod tests {
  build_std: None,
  image: Some(PossibleImage {
  name: "test-image".to_string(),
- toolchain: Some(ImageArchitecture::from_target(
+ toolchain: Some(ImagePlatform::from_target(
  "aarch64-unknown-linux-musl".into(),
- )),
+ )?),
  }),
  dockerfile: Some(CrossTargetDockerfileConfig {
  file: "Dockerfile.test".to_string(),
@@ -750,39 +750,39 @@ mod tests {
  let build = &cfg_expected.build;
  assert_eq!(build.build_std, Some(true));
  assert_eq!(build.xargo, Some(false));
- assert_eq!(build.default_target, Some(s!("aarch64-unknown-linux-gnu")));
+ assert_eq!(build.default_target, Some(p!("aarch64-unknown-linux-gnu")));
  assert_eq!(build.pre_build, None);
  assert_eq!(build.dockerfile, None);
- assert_eq!(build.env.passthrough, Some(vec![s!("VAR3"), s!("VAR4")]));
+ assert_eq!(build.env.passthrough, Some(vec![p!("VAR3"), p!("VAR4")]));
  assert_eq!(build.env.volumes, Some(vec![]));
 
  let targets = &cfg_expected.targets;
  let aarch64 = &targets[&Target::new_built_in("aarch64-unknown-linux-gnu")];
  assert_eq!(aarch64.build_std, Some(true));
  assert_eq!(aarch64.xargo, Some(false));
- assert_eq!(aarch64.image, Some(s!("test-image1")));
+ assert_eq!(aarch64.image, Some(p!("test-image1")));
  assert_eq!(aarch64.pre_build, None);
  assert_eq!(aarch64.dockerfile, None);
- assert_eq!(aarch64.env.passthrough, Some(vec![s!("VAR1")]));
- assert_eq!(aarch64.env.volumes, Some(vec![s!("VOL1_ARG")]));
+ assert_eq!(aarch64.env.passthrough, Some(vec![p!("VAR1")]));
+ assert_eq!(aarch64.env.volumes, Some(vec![p!("VOL1_ARG")]));
 
  let target2 = &targets[&Target::new_custom("target2")];
  assert_eq!(target2.build_std, Some(false));
  assert_eq!(target2.xargo, Some(false));
- assert_eq!(target2.image, Some(s!("test-image2-precedence")));
+ assert_eq!(target2.image, Some(p!("test-image2-precedence")));
  assert_eq!(target2.pre_build, None);
  assert_eq!(target2.dockerfile, None);
- assert_eq!(target2.env.passthrough, Some(vec![s!("VAR2_PRECEDENCE")]));
- assert_eq!(target2.env.volumes, Some(vec![s!("VOL2_ARG_PRECEDENCE")]));
+ assert_eq!(target2.env.passthrough, Some(vec![p!("VAR2_PRECEDENCE")]));
+ assert_eq!(target2.env.volumes, Some(vec![p!("VOL2_ARG_PRECEDENCE")]));
 
  let target3 = &targets[&Target::new_custom("target3")];
  assert_eq!(target3.build_std, Some(true));
  assert_eq!(target3.xargo, Some(false));
- assert_eq!(target3.image, Some(s!("test-image3")));
+ assert_eq!(target3.image, Some(p!("test-image3")));
  assert_eq!(target3.pre_build, None);
  assert_eq!(target3.dockerfile, None);
- assert_eq!(target3.env.passthrough, Some(vec![s!("VAR3")]));
- assert_eq!(target3.env.volumes, Some(vec![s!("VOL3_ARG")]));
+ assert_eq!(target3.env.passthrough, Some(vec![p!("VAR3")]));
+ assert_eq!(target3.env.volumes, Some(vec![p!("VOL3_ARG")]));
 
  Ok(())
  }

src/docker/custom.rs

@@ -1,12 +1,12 @@
 use std::io::Write;
 use std::path::{Path, PathBuf};
 
-use crate::docker::{Engine, ImageArchitecture};
+use crate::docker::{Engine, ImagePlatform};
 use crate::shell::MessageInfo;
 use crate::{config::Config, docker, CargoMetadata, Target};
-use crate::{errors::*, file, CommandExt, ToUtf8};
+use crate::{errors::*, file, CommandExt, TargetTriple, ToUtf8};
 
-use super::{get_image, parse_docker_opts, path_hash};
+use super::{get_image_name, parse_docker_opts, path_hash};
 
 pub const CROSS_CUSTOM_DOCKERFILE_IMAGE_PREFIX: &str = "cross-custom-";
 
@@ -16,11 +16,11 @@ pub enum Dockerfile<'a> {
  path: &'a str,
  context: Option<&'a str>,
  name: Option<&'a str>,
- runs_with: &'a ImageArchitecture,
+ runs_with: &'a ImagePlatform,
  },
  Custom {
  content: String,
- runs_with: &'a ImageArchitecture,
+ runs_with: &'a ImagePlatform,
  },
 }
 
@@ -33,12 +33,14 @@ impl<'a> Dockerfile<'a> {
  engine: &Engine,
  host_root: &Path,
  build_args: impl IntoIterator<Item = (impl AsRef<str>, impl AsRef<str>)>,
- target_triple: &Target,
  msg_info: MessageInfo,
  ) -> Result<String> {
+ let target_triple = &self.runs_with().target;
  let mut docker_build = docker::subcommand(engine, "build");
  docker_build.current_dir(host_root);
  docker_build.env("DOCKER_SCAN_SUGGEST", "false");
+ docker_build.args(&["--platform", &self.runs_with().docker_platform()]);
+
  docker_build.args([
  "--label",
  &format!(
@@ -51,7 +53,6 @@ impl<'a> Dockerfile<'a> {
  &format!(
  "{}.runs-with={}",
  crate::CROSS_LABEL_DOMAIN,
- // FIXME(emilgardis): this should maybe be the full image architecture, but that information is already encoded in the image
  self.runs_with().target
  ),
  ]);
@@ -92,7 +93,13 @@ impl<'a> Dockerfile<'a> {
  };
 
  if matches!(self, Dockerfile::File { .. }) {
- if let Ok(cross_base_image) = self::get_image(config, target_triple) {
+ if let Ok(cross_base_image) = self::get_image_name(
+ config,
+ &Target::Custom {
+ // HACK: This shouldn't be needed, but fixing it requires a major rework of targets
+ triple: target_triple.clone(),
+ },
+ ) {
  docker_build.args([
  "--build-arg",
  &format!("CROSS_BASE_IMAGE={cross_base_image}"),
@@ -116,7 +123,11 @@ impl<'a> Dockerfile<'a> {
  Ok(image_name)
  }
 
- pub fn image_name(&self, target_triple: &Target, metadata: &CargoMetadata) -> Result<String> {
+ pub fn image_name(
+ &self,
+ target_triple: &TargetTriple,
+ metadata: &CargoMetadata,
+ ) -> Result<String> {
  match self {
  Dockerfile::File {
  name: Some(name), ..
@@ -148,7 +159,7 @@ impl<'a> Dockerfile<'a> {
  _ => None,
  }
  }
- fn runs_with(&self) -> &ImageArchitecture {
+ fn runs_with(&self) -> &ImagePlatform {
  match self {
  Dockerfile::File { runs_with, .. } => runs_with,
  Dockerfile::Custom { runs_with, .. } => runs_with,