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

ndk-build: Invoke clang directly instead of through wrapper scripts #306

Merged
merged 1 commit into from
Jul 4, 2022
Merged
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
1 change: 1 addition & 0 deletions ndk-build/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Unreleased

- Allow NDK r23 `-lgcc` workaround to work for target directories containing spaces. ([#298](https://github.com/rust-windowing/android-ndk-rs/pull/298))
- Invoke `clang` directly instead of through the NDK's wrapper scripts. ([#306](https://github.com/rust-windowing/android-ndk-rs/pull/306))

# 0.6.0 (2022-06-11)

Expand Down
2 changes: 1 addition & 1 deletion ndk-build/src/apk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl<'a> UnalignedApk<'a> {
search_paths: &[&Path],
) -> Result<(), NdkError> {
let abi_dir = path.join(target.android_abi());
for entry in fs::read_dir(&abi_dir).map_err(|e| NdkError::IoPathError(e, abi_dir))? {
for entry in fs::read_dir(&abi_dir).map_err(|e| NdkError::IoPathError(abi_dir, e))? {
let entry = entry?;
let path = entry.path();
if path.extension() == Some(OsStr::new("so")) {
Expand Down
59 changes: 39 additions & 20 deletions ndk-build/src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,35 @@ pub fn cargo_ndk(
target_dir: impl AsRef<Path>,
) -> Result<Command, NdkError> {
let triple = target.rust_triple();
let clang_target = format!("--target={}{}", target.ndk_llvm_triple(), sdk_version);
let mut cargo = Command::new("cargo");

let (clang, clang_pp) = ndk.clang(target, sdk_version)?;
// Read initial RUSTFLAGS
let mut rustflags = match std::env::var("CARGO_ENCODED_RUSTFLAGS") {
Ok(val) => val,
Err(std::env::VarError::NotPresent) => "".to_string(),
Err(std::env::VarError::NotUnicode(_)) => {
panic!("RUSTFLAGS environment variable contains non-unicode characters")
}
};

let (clang, clang_pp) = ndk.clang()?;

// Configure cross-compiler for `cc` crate
// https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables
cargo.env(format!("CC_{}", triple), &clang);
cargo.env(format!("CFLAGS_{}", triple), &clang_target);
cargo.env(format!("CXX_{}", triple), &clang_pp);
cargo.env(format!("CXXFLAGS_{}", triple), &clang_target);

// Configure LINKER for `rustc`
// https://doc.rust-lang.org/beta/cargo/reference/environment-variables.html#configuration-environment-variables
cargo.env(cargo_env_target_cfg("LINKER", triple), &clang);
if !rustflags.is_empty() {
rustflags.push('\x1f');
}
rustflags.push_str("-Clink-arg=");
rustflags.push_str(&clang_target);

let ar = ndk.toolchain_bin("ar", target)?;
cargo.env(format!("AR_{}", triple), &ar);
Expand All @@ -32,32 +55,28 @@ pub fn cargo_ndk(
let cargo_apk_link_dir = target_dir
.as_ref()
.join("cargo-apk-temp-extra-link-libraries");
std::fs::create_dir_all(&cargo_apk_link_dir)?;
std::fs::write(cargo_apk_link_dir.join("libgcc.a"), "INPUT(-lunwind)")
.expect("Failed to write");
std::fs::create_dir_all(&cargo_apk_link_dir)
.map_err(|e| NdkError::IoPathError(cargo_apk_link_dir.clone(), e))?;
let libgcc = cargo_apk_link_dir.join("libgcc.a");
std::fs::write(&libgcc, "INPUT(-lunwind)").map_err(|e| NdkError::IoPathError(libgcc, e))?;

// cdylibs in transitive dependencies still get built and also need this
// workaround linker flag, yet arguments passed to `cargo rustc` are only
// forwarded to the final compiler invocation rendering our workaround ineffective.
// The cargo page documenting this discrepancy (https://doc.rust-lang.org/cargo/commands/cargo-rustc.html)
// suggests to resort to RUSTFLAGS, which are updated below:
let mut rustflags = match std::env::var("CARGO_ENCODED_RUSTFLAGS") {
Ok(val) => val,
Err(std::env::VarError::NotPresent) => "".to_string(),
Err(std::env::VarError::NotUnicode(_)) => {
panic!("RUSTFLAGS environment variable contains non-unicode characters")
}
};
if !rustflags.is_empty() {
rustflags.push('\x1f');
}
rustflags += "-L\x1f";
rustflags += cargo_apk_link_dir
.to_str()
.expect("Target dir must be valid UTF-8");
cargo.env("CARGO_ENCODED_RUSTFLAGS", rustflags);
// suggests to resort to RUSTFLAGS.
// Note that `rustflags` will never be empty because of an unconditional `.push_str` above,
// so we can safely start with appending \x1f here.
rustflags.push_str("\x1f-L\x1f");
rustflags.push_str(
cargo_apk_link_dir
.to_str()
.expect("Target dir must be valid UTF-8"),
);
}

cargo.env("CARGO_ENCODED_RUSTFLAGS", rustflags);

Ok(cargo)
}

Expand Down
4 changes: 2 additions & 2 deletions ndk-build/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ pub enum NdkError {
UnsupportedHost(String),
#[error(transparent)]
Io(#[from] IoError),
#[error("{0:?}: `{1}`")]
IoPathError(#[source] IoError, PathBuf),
#[error("IoError on `{0:?}`: {1}")]
IoPathError(PathBuf, #[source] IoError),
#[error("Invalid semver")]
InvalidSemver,
#[error("Command `{}` had a non-zero exit code.", format!("{:?}", .0).replace('"', ""))]
Expand Down
28 changes: 14 additions & 14 deletions ndk-build/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
macro_rules! bin {
($bin:expr) => {{
#[cfg(not(target_os = "windows"))]
let bin = $bin;
#[cfg(target_os = "windows")]
let bin = concat!($bin, ".exe");
bin
}};
($bin:expr) => {
if cfg!(target_os = "windows") {
concat!($bin, ".exe")
} else {
$bin
}
};
}

macro_rules! bat {
($bat:expr) => {{
#[cfg(not(target_os = "windows"))]
let bat = $bat;
#[cfg(target_os = "windows")]
let bat = concat!($bat, ".bat");
bat
}};
($bat:expr) => {
if cfg!(target_os = "windows") {
concat!($bat, ".bat")
} else {
$bat
}
};
}

pub mod apk;
Expand Down
25 changes: 13 additions & 12 deletions ndk-build/src/ndk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,21 +228,21 @@ impl Ndk {
Ok(toolchain_dir)
}

pub fn clang(&self, target: Target, platform: u32) -> Result<(PathBuf, PathBuf), NdkError> {
#[cfg(target_os = "windows")]
let ext = ".cmd";
#[cfg(not(target_os = "windows"))]
let ext = "";
pub fn clang(&self) -> Result<(PathBuf, PathBuf), NdkError> {
let ext = if cfg!(target_os = "windows") {
"exe"
} else {
""
};

let bin_name = format!("{}{}-clang", target.ndk_llvm_triple(), platform);
let bin_path = self.toolchain_dir()?.join("bin");

let clang = bin_path.join(format!("{}{}", &bin_name, ext));
let clang = bin_path.join("clang").with_extension(ext);
if !clang.exists() {
return Err(NdkError::PathNotFound(clang));
}

let clang_pp = bin_path.join(format!("{}++{}", &bin_name, ext));
let clang_pp = bin_path.join("clang++").with_extension(ext);
if !clang_pp.exists() {
return Err(NdkError::PathNotFound(clang_pp));
}
Expand All @@ -251,10 +251,11 @@ impl Ndk {
}

pub fn toolchain_bin(&self, name: &str, target: Target) -> Result<PathBuf, NdkError> {
#[cfg(target_os = "windows")]
let ext = ".exe";
#[cfg(not(target_os = "windows"))]
let ext = "";
let ext = if cfg!(target_os = "windows") {
".exe"
} else {
""
};

let toolchain_path = self.toolchain_dir()?.join("bin");

Expand Down