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

Allow users to ignore config files in the package. #936

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/936.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "added",
"description": "allow users to ignore config files in the package.",
"issues": [621]
}
1 change: 1 addition & 0 deletions docs/cross_toml.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ For example:

```toml
[build.env]
ignore-cargo-config = false
Alexhuszagh marked this conversation as resolved.
Show resolved Hide resolved
volumes = ["VOL1_ARG", "VOL2_ARG"]
passthrough = ["IMPORTANT_ENV_VARIABLES"]
```
Expand Down
12 changes: 12 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ impl Environment {
self.get_target_var(target, "RUNNER")
}

fn ignore_cargo_config(&self, target: &Target) -> (Option<bool>, Option<bool>) {
self.get_values_for("ENV_IGNORE_CARGO_CONFIG", target, bool_from_envvar)
}

fn passthrough(&self, target: &Target) -> (Option<Vec<String>>, Option<Vec<String>>) {
self.get_values_for("ENV_PASSTHROUGH", target, split_to_cloned_by_ws)
}
Expand Down Expand Up @@ -275,6 +279,14 @@ impl Config {
self.bool_from_config(target, Environment::build_std, CrossToml::build_std)
}

pub fn ignore_cargo_config(&self, target: &Target) -> Option<bool> {
self.bool_from_config(
target,
Environment::ignore_cargo_config,
CrossToml::ignore_cargo_config,
)
}

pub fn image(&self, target: &Target) -> Result<Option<String>> {
self.string_from_config(target, Environment::image, CrossToml::image)
}
Expand Down
18 changes: 18 additions & 0 deletions src/cross_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use std::str::FromStr;

/// Environment configuration
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Default)]
#[serde(rename_all = "kebab-case")]
pub struct CrossEnvConfig {
ignore_cargo_config: Option<bool>,
volumes: Option<Vec<String>>,
passthrough: Option<Vec<String>>,
}
Expand Down Expand Up @@ -273,6 +275,15 @@ impl CrossToml {
self.get_value(target, |b| b.build_std, |t| t.build_std)
}

/// Returns the whether to ignore cargo config files.
pub fn ignore_cargo_config(&self, target: &Target) -> (Option<bool>, Option<bool>) {
self.get_value(
target,
|b| b.env.ignore_cargo_config,
|t| t.env.ignore_cargo_config,
)
}

/// Returns the list of environment variables to pass through for `build` and `target`
pub fn env_passthrough(&self, target: &Target) -> (Option<&[String]>, Option<&[String]>) {
self.get_ref(
Expand Down Expand Up @@ -489,6 +500,7 @@ mod tests {
targets: HashMap::new(),
build: CrossBuildConfig {
env: CrossEnvConfig {
ignore_cargo_config: Some(false),
volumes: Some(vec![s!("VOL1_ARG"), s!("VOL2_ARG")]),
passthrough: Some(vec![s!("VAR1"), s!("VAR2")]),
},
Expand All @@ -506,6 +518,7 @@ mod tests {
pre-build = ["echo 'Hello World!'"]

[build.env]
ignore-cargo-config = false
volumes = ["VOL1_ARG", "VOL2_ARG"]
passthrough = ["VAR1", "VAR2"]
"#;
Expand All @@ -526,6 +539,7 @@ mod tests {
},
CrossTargetConfig {
env: CrossEnvConfig {
ignore_cargo_config: None,
passthrough: Some(vec![s!("VAR1"), s!("VAR2")]),
volumes: Some(vec![s!("VOL1_ARG"), s!("VOL2_ARG")]),
},
Expand Down Expand Up @@ -580,6 +594,7 @@ mod tests {
pre_build: Some(PreBuild::Lines(vec![s!("echo 'Hello'")])),
runner: None,
env: CrossEnvConfig {
ignore_cargo_config: None,
passthrough: None,
volumes: Some(vec![s!("VOL")]),
},
Expand All @@ -590,6 +605,7 @@ mod tests {
targets: target_map,
build: CrossBuildConfig {
env: CrossEnvConfig {
ignore_cargo_config: Some(true),
volumes: None,
passthrough: Some(vec![]),
},
Expand All @@ -607,6 +623,7 @@ mod tests {
pre-build = []

[build.env]
ignore-cargo-config = true
passthrough = []

[target.aarch64-unknown-linux-gnu]
Expand Down Expand Up @@ -648,6 +665,7 @@ mod tests {
targets: HashMap::new(),
build: CrossBuildConfig {
env: CrossEnvConfig {
ignore_cargo_config: None,
passthrough: None,
volumes: None,
},
Expand Down
2 changes: 1 addition & 1 deletion src/docker/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub(crate) fn run(
docker
.args(&["-v", &format!("{}:/rust:Z,ro", dirs.sysroot.to_utf8()?)])
.args(&["-v", &format!("{}:/target:Z", dirs.target.to_utf8()?)]);
docker_cwd(&mut docker, &paths)?;
docker_cwd(&mut docker, &paths, options.ignore_cargo_config)?;

// When running inside NixOS or using Nix packaging we need to add the Nix
// Store to the running container so it can load the needed binaries.
Expand Down
2 changes: 1 addition & 1 deletion src/docker/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,7 @@ symlink_recurse \"${{prefix}}\"
let mut docker = subcommand(engine, "exec");
docker_user_id(&mut docker, engine.kind);
docker_envvars(&mut docker, &options.config, target, msg_info)?;
docker_cwd(&mut docker, &paths)?;
docker_cwd(&mut docker, &paths, options.ignore_cargo_config)?;
docker.arg(&container);
docker.args(&["sh", "-c", &format!("PATH=$PATH:/rust/bin {:?}", cmd)]);
bail_container_exited!();
Expand Down
64 changes: 55 additions & 9 deletions src/docker/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,12 @@ use crate::cargo::{cargo_metadata_with_args, CargoMetadata};
use crate::config::{bool_from_envvar, Config};
use crate::errors::*;
use crate::extensions::{CommandExt, SafeCommand};
use crate::file::{self, write_file, ToUtf8};
use crate::file::{self, write_file, PathExt, ToUtf8};
use crate::id;
use crate::rustc::{self, VersionMetaExt};
use crate::shell::{MessageInfo, Verbosity};
use crate::Target;

#[cfg(target_os = "windows")]
use crate::file::PathExt;

pub use super::custom::CROSS_CUSTOM_DOCKERFILE_IMAGE_PREFIX;

pub const CROSS_IMAGE: &str = "ghcr.io/cross-rs";
Expand All @@ -37,15 +34,23 @@ pub struct DockerOptions {
pub target: Target,
pub config: Config,
pub uses_xargo: bool,
pub ignore_cargo_config: bool,
}

impl DockerOptions {
pub fn new(engine: Engine, target: Target, config: Config, uses_xargo: bool) -> DockerOptions {
pub fn new(
engine: Engine,
target: Target,
config: Config,
uses_xargo: bool,
ignore_cargo_config: bool,
) -> DockerOptions {
DockerOptions {
engine,
target,
config,
uses_xargo,
ignore_cargo_config,
}
}

Expand Down Expand Up @@ -220,10 +225,18 @@ impl DockerPaths {
self.workspace_from_cwd().is_ok()
}

pub fn cargo_home(&self) -> &Path {
&self.directories.cargo
}

pub fn mount_cwd(&self) -> &str {
&self.directories.mount_cwd
}

pub fn mount_root(&self) -> &str {
&self.directories.mount_root
}

pub fn host_root(&self) -> &Path {
&self.directories.host_root
}
Expand Down Expand Up @@ -499,8 +512,44 @@ pub(crate) fn docker_envvars(
Ok(())
}

pub(crate) fn docker_cwd(docker: &mut Command, paths: &DockerPaths) -> Result<()> {
fn mount_to_ignore_cargo_config(
docker: &mut Command,
paths: &DockerPaths,
ignore_cargo_config: bool,
) -> Result<()> {
let check_mount =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks if the path exists on the host (canonicalized), and only if it does, mounts it. This avoids any permission issues or creating undesired directories.

|cmd: &mut Command, host: &Path, mount: &Path, relpath: &Path| -> Result<()> {
let cargo_dir = relpath.join(".cargo");
if host.join(&cargo_dir).exists() {
// this is fine, since it has to be a POSIX path on the mount.
cmd.args(&["-v", &mount.join(&cargo_dir).as_posix()?]);
}

Ok(())
};
if ignore_cargo_config {
let mount_root = Path::new(paths.mount_root());
let mount_cwd = Path::new(paths.mount_cwd());
check_mount(docker, &paths.cwd, mount_cwd, Path::new(""))?;
// CWD isn't guaranteed to be a subdirectory of the mount root.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we've removed /project, this changes the logic here, so we don't necessarily have a relpath to the workspace root.

if let Ok(mut relpath) = mount_cwd.strip_prefix(mount_root) {
while let Some(parent) = relpath.parent() {
check_mount(docker, paths.host_root(), mount_root, parent)?;
relpath = parent;
}
}
}

Ok(())
}

pub(crate) fn docker_cwd(
docker: &mut Command,
paths: &DockerPaths,
ignore_cargo_config: bool,
) -> Result<()> {
docker.args(&["-w", paths.mount_cwd()]);
mount_to_ignore_cargo_config(docker, paths, ignore_cargo_config)?;

Ok(())
}
Expand Down Expand Up @@ -788,9 +837,6 @@ mod tests {
use super::*;
use crate::id;

#[cfg(not(target_os = "windows"))]
use crate::file::PathExt;

#[test]
fn test_docker_user_id() {
let var = "CROSS_ROOTLESS_CONTAINER_ENGINE";
Expand Down
10 changes: 8 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,14 @@ pub fn run() -> Result<ExitStatus> {
}

let paths = docker::DockerPaths::create(&engine, metadata, cwd, sysroot)?;
let options =
docker::DockerOptions::new(engine, target.clone(), config, uses_xargo);
let ignore_cargo_config = config.ignore_cargo_config(&target).unwrap_or_default();
let options = docker::DockerOptions::new(
engine,
target.clone(),
config,
uses_xargo,
ignore_cargo_config,
);
let status = docker::run(options, paths, &filtered_args, &mut msg_info)
.wrap_err("could not run container")?;
let needs_host = args.subcommand.map_or(false, |sc| sc.needs_host(is_remote));
Expand Down