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

cargo: Use CARGO_ENCODED_RUSTFLAGS to support spaces in environment paths #186

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
23 changes: 13 additions & 10 deletions xbuild/src/cargo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use self::config::LocalizedConfig;
use self::manifest::Manifest;
use crate::{CompileTarget, Opt};

const SEP: &str = "\x1f";

pub struct Cargo {
package: String,
features: Vec<String>,
Expand Down Expand Up @@ -236,7 +238,7 @@ pub struct CargoBuild {
triple: Option<&'static str>,
c_flags: String,
cxx_flags: String,
rust_flags: String,
rust_flags: Vec<String>,
}

impl CargoBuild {
Expand Down Expand Up @@ -401,31 +403,29 @@ impl CargoBuild {
}

pub fn add_lib_dir(&mut self, path: &Path) {
self.rust_flags
.push_str(&format!("-Lnative={} ", path.display()));
self.rust_flags.push(format!("-Lnative={}", path.display()));
}

pub fn add_framework_dir(&mut self, path: &Path) {
self.rust_flags
.push_str(&format!("-Lframework={} ", path.display()));
.push(format!("-Lframework={}", path.display()));
}

pub fn link_lib(&mut self, name: &str) {
self.rust_flags.push_str(&format!("-l{} ", name));
self.rust_flags.push(format!("-l{}", name));
}

pub fn link_framework(&mut self, name: &str) {
self.rust_flags.push_str(&format!("-lframework={} ", name));
self.rust_flags.push(format!("-lframework={}", name));
}

pub fn add_target_feature(&mut self, target_feature: &str) {
self.rust_flags
.push_str(&format!("-Ctarget-feature={} ", target_feature));
.push(format!("-Ctarget-feature={}", target_feature));
}

pub fn add_link_arg(&mut self, link_arg: &str) {
self.rust_flags
.push_str(&format!("-Clink-arg={} ", link_arg));
self.rust_flags.push(format!("-Clink-arg={}", link_arg));
}

pub fn add_define(&mut self, name: &str, value: &str) {
Expand Down Expand Up @@ -465,7 +465,10 @@ impl CargoBuild {
}

pub fn exec(mut self) -> Result<()> {
self.cargo_target_env("RUSTFLAGS", &self.rust_flags.clone());
// TODO: There's no target-triple specific ENCODED_RUSTFLAGS: https://github.com/rust-lang/cargo/issues/14398

Choose a reason for hiding this comment

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

It's not clear to me why we need a target triple specific version of this flag, can't we use the regular version?

Copy link
Member Author

@MarijnS95 MarijnS95 Aug 21, 2024

Choose a reason for hiding this comment

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

To make sure that none of these RUSTFLAGS (and C(XX)FLAGS) apply to binaries built for the host, i.e. build.rs. There used to be some additional complexity with the existence of target-applies-to-host, but I can't remember the details for it.

// self.cargo_target_env("ENCODED_RUSTFLAGS", &self.rust_flags.join(SEP));
self.cmd
.env("CARGO_ENCODED_RUSTFLAGS", self.rust_flags.join(SEP));
self.cc_triple_env("CFLAGS", &self.c_flags.clone());
// These strings already end with a space if they're non-empty:
self.cc_triple_env("CXXFLAGS", &format!("{}{}", self.c_flags, self.cxx_flags));
Expand Down
Loading