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.
bbqsrc · May 16, 2023 · 10a7234 · 10a7234
1 parent 49a2ef3
commit 10a7234
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 98 deletions.
diff --git 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")]
@@ -54,11 +59,73 @@ fn cargo_env_target_cfg(triple: &str, key: &str) -> String {
     format!("CARGO_TARGET_{}_{}", &triple.replace('-', "_"), key).to_uppercase()
 }
 
-#[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<P>(filename: P) -> io::Result<io::Lines<io::BufReader<File>>>
+where
+    P: AsRef<Path>,
+{
+    let file = File::open(filename)?;
+    Ok(io::BufReader::new(file).lines())
+}
+
+fn write_lines<P>(filename: P, lines: &[String]) -> io::Result<()>
+where
+    P: AsRef<Path>,
+{
+    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`
+fn ndk_r25_workaround_patch_cmd_script<P>(filename: P, apply: bool) -> bool
+where
+    P: AsRef<Path> + 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<String> = 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 +138,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 +194,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 +203,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
@@ -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<PathBuf>,
 
@@ -370,6 +377,7 @@ pub(crate) fn run(args: Vec<String>) {
             &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
@@ -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::<Vec<_>>();
-
-    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::<Vec<_>>();
 
     cli::run(args);