From 31c8e31588b6e60ccce8d93b7bfb1a35490c883f Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Tue, 16 May 2023 21:01:18 +0100 Subject: [PATCH] Support patching r25 NDK with quoting workaround on Windows This is an alternative fix for #92 that avoids hitting command line length limitations and avoid needing to use `cargo-ndk` as a spring board for running `clang`, which avoids needing temporary hard links or copying the `cargo-ndk` binary to the target/ directory. By default cargo-ndk will just emit an error message if it recognises the current NDK needs a quoting workaround, and will also emit a warning suggesting to re-run with `--apply-ndk-quote-workaround` so that `cargo-ndk` can automatically apply the required workaround. --- .github/workflows/ci.yml | 2 +- src/cargo.rs | 131 ++++++++++++++++++++++++--------------- src/cli.rs | 8 +++ src/main.rs | 47 -------------- 4 files changed, 90 insertions(+), 98 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93a0c48..cdf76e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: run: cargo install --path . - name: Run test working-directory: example - run: cargo ndk build + run: cargo ndk --apply-ndk-quote-workaround build - name: Check code formatting continue-on-error: true run: cargo fmt --all -- --check diff --git a/src/cargo.rs b/src/cargo.rs index 31a87c5..b887af5 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -1,12 +1,17 @@ use std::{ env, ffi::OsString, + fs::File, + io::{self, BufRead, Write}, path::{Path, PathBuf}, process::Command, }; use cargo_metadata::{camino::Utf8PathBuf, semver::Version}; +#[cfg(target_os = "windows")] +const NDK_CMD_QUOTE_ISSUE: &str = "https://github.com/android/ndk/issues/1856"; + #[cfg(target_os = "macos")] const ARCH: &str = "darwin-x86_64"; #[cfg(target_os = "linux")] @@ -55,10 +60,75 @@ fn cargo_env_target_cfg(triple: &str, key: &str) -> String { } #[cfg(windows)] -fn cargo_target_dir(out_dir: &Utf8PathBuf) -> PathBuf { - std::env::var("CARGO_TARGET_DIR") - .map(PathBuf::from) - .unwrap_or_else(|_| out_dir.clone().into_std_path_buf()) +fn read_lines

(filename: P) -> io::Result>> +where + P: AsRef, +{ + let file = File::open(filename)?; + Ok(io::BufReader::new(file).lines()) +} + +#[cfg(windows)] +fn write_lines

(filename: P, lines: &[String]) -> io::Result<()> +where + P: AsRef, +{ + let file = File::create(filename)?; + for line in lines { + writeln!(&file, "{line}")?; + } + Ok(()) +} + +/// This fixes a quoting bug in the r25 NDK .cmd wrapper scripts for +/// Windows, ref: https://github.com/android/ndk/issues/1856 +/// +/// Returns `true` if the workaround is required, else `false` +#[cfg(windows)] +fn ndk_r25_workaround_patch_cmd_script

(filename: P, apply: bool) -> bool +where + P: AsRef + std::fmt::Debug, +{ + let mut patched = false; + let lines = read_lines(&filename).unwrap_or_else(|err| { + log::error!("Failed to read {filename:?} to check if quoting workaround required: {err:?}"); + std::process::exit(1); + }); + + let lines: Vec = lines + .filter_map(|line| { + if let Ok(line) = line { + if line == r#"if "%1" == "-cc1" goto :L"# { + patched = true; + Some(r#"if "%~1" == "-cc1" goto :L"#.to_string()) + } else { + Some(line) + } + } else { + None + } + }) + .collect(); + + if patched { + if !apply { + log::error!( + "NDK .cmd wrapper needs quoting workaround ({NDK_CMD_QUOTE_ISSUE}): {filename:?}" + ); + patched + } else { + if let Err(err) = write_lines(&filename, &lines) { + log::error!("Failed to patch {filename:?} to workaround quoting bug ({NDK_CMD_QUOTE_ISSUE}): {err:?}"); + std::process::exit(1); + } + log::info!( + "Applied NDK .cmd wrapper quoting workaround ({NDK_CMD_QUOTE_ISSUE}): {filename:?}" + ); + false + } + } else { + false + } } #[allow(clippy::too_many_arguments)] @@ -71,6 +141,7 @@ pub(crate) fn run( cargo_args: &[String], cargo_manifest: &Path, bindgen: bool, + apply_ndk_quote_workaround: bool, #[allow(unused_variables)] out_dir: &Utf8PathBuf, ) -> std::process::ExitStatus { log::debug!("Detected NDK version: {:?}", &version); @@ -126,7 +197,6 @@ pub(crate) fn run( let mut cargo_cmd = Command::new(cargo_bin); - #[cfg(not(windows))] cargo_cmd .current_dir(dir) .env(&ar_key, &target_ar) @@ -136,55 +206,16 @@ pub(crate) fn run( .env(cargo_env_target_cfg(triple, "ar"), &target_ar) .env(cargo_env_target_cfg(triple, "linker"), &target_linker); - #[cfg(windows)] - let cargo_ndk_target_dir = - cargo_target_dir(out_dir).join(format!(".cargo-ndk-{}", env!("CARGO_PKG_VERSION"))); - #[cfg(windows)] { - let main = std::env::args().next().unwrap(); - if !cargo_ndk_target_dir.exists() { - std::fs::create_dir_all(&cargo_ndk_target_dir).unwrap(); - } - - for f in ["ar", "cc", "cxx", "ranlib", "triple-ar", "triple-linker"] { - let executable = cargo_ndk_target_dir.join(f).with_extension("exe"); - if executable.exists() { - continue; - } - - match std::fs::hard_link(&main, &executable) - .or_else(|_| std::fs::copy(&main, executable).map(|_| ())) - { - Ok(_) => {} - Err(e) => { - log::error!("Failed to create hardlink or copy for '{f}'."); - log::error!("{}", e); - std::process::exit(1); - } + if version.major == 25 { + let needs_workaround = + ndk_r25_workaround_patch_cmd_script(target_linker, apply_ndk_quote_workaround) + | ndk_r25_workaround_patch_cmd_script(target_cxx, true); + if needs_workaround { + log::warn!("Re-run with --apply-ndk-quote-workaround to patch NDK with quoting workaround ({NDK_CMD_QUOTE_ISSUE})"); } } - - cargo_cmd - .current_dir(dir) - .env(&ar_key, cargo_ndk_target_dir.join("ar.exe")) - .env(&cc_key, cargo_ndk_target_dir.join("cc.exe")) - .env(&cxx_key, cargo_ndk_target_dir.join("cxx.exe")) - .env(&ranlib_key, cargo_ndk_target_dir.join("ranlib.exe")) - .env( - cargo_env_target_cfg(triple, "ar"), - cargo_ndk_target_dir.join("triple-ar.exe"), - ) - .env( - cargo_env_target_cfg(triple, "linker"), - cargo_ndk_target_dir.join("triple-linker.exe"), - ) - .env("CARGO_NDK_AR", &target_ar) - .env("CARGO_NDK_CC", &target_linker) - .env("CARGO_NDK_CXX", &target_cxx) - .env("CARGO_NDK_RANLIB", &target_ranlib) - .env("CARGO_NDK_TRIPLE_AR", &target_ar) - .env("CARGO_NDK_TRIPLE_LINKER", &target_linker); } let extra_include = format!("{}/usr/include/{}", &target_sysroot.display(), triple); diff --git a/src/cli.rs b/src/cli.rs index c350dbf..eaf8529 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -35,6 +35,13 @@ struct Args { #[options(no_short, help = "disable stripping debug symbols", default = "false")] no_strip: bool, + #[options( + no_short, + help = "apply a quoting workaround to NDK .cmd wrappers on Windows (https://github.com/android/ndk/issues/1856)", + default = "false" + )] + apply_ndk_quote_workaround: bool, + #[options(no_short, meta = "PATH", help = "path to Cargo.toml")] manifest_path: Option, @@ -370,6 +377,7 @@ pub(crate) fn run(args: Vec) { &args.cargo_args, &cargo_manifest, args.bindgen, + args.apply_ndk_quote_workaround, &out_dir, ); let code = status.code().unwrap_or(-1); diff --git a/src/main.rs b/src/main.rs index 77a63bd..e41200d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,30 +5,6 @@ mod cargo; mod cli; mod meta; -#[cfg(windows)] -fn args_hack(cmd: &str) -> anyhow::Result<()> { - use std::os::windows::process::CommandExt; - - let args = wargs::command_line_to_argv(None) - .skip(1) - .collect::>(); - - let mut process = std::process::Command::new(cmd) - .raw_arg(args.join(" ")) - .spawn()?; - - let status = process.wait()?; - - if status.success() { - Ok(()) - } else { - Err(anyhow::anyhow!( - "Errored with code {}", - status.code().unwrap() - )) - } -} - fn main() -> anyhow::Result<()> { env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info")).init(); @@ -37,29 +13,6 @@ fn main() -> anyhow::Result<()> { exit(1); } - #[cfg(windows)] - { - let main_arg = std::env::args().next().unwrap(); - let main_arg = std::path::Path::new(&main_arg) - .file_stem() - .unwrap() - .to_str() - .unwrap(); - - if main_arg != "cargo-ndk" { - let maybe = main_arg.to_uppercase().replace('-', "_"); - let app = match std::env::var(format!("CARGO_NDK_{maybe}")) { - Ok(cmd) => cmd, - Err(err) => { - log::error!("{}", err); - panic!("{}", err); - } - }; - log::debug!("Running command: {app}"); - return args_hack(&app); - } - } - let args = std::env::args().skip(2).collect::>(); cli::run(args);