Skip to content

Call clang directly when cross-compiling from Windows to Android #572

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

Merged
merged 11 commits into from
Dec 2, 2020
31 changes: 31 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,37 @@ impl Build {
tool
};

// New "standalone" C/C++ cross-compiler executables from recent Android NDK
// are just shell scripts that call main clang binary (from Android NDK) with
// proper `--target` argument.
//
// For example, armv7a-linux-androideabi16-clang passes
// `--target=armv7a-linux-androideabi16` to clang.
//
// As the shell script calls the main clang binary, the command line limit length
// on Windows is restricted to around 8k characters instead of around 32k characters.
// To remove this limit, we call the main clang binary directly and contruct the
// `--target=` ourselves.
if host.contains("windows") && android_clang_compiler_uses_target_arg_internally(&tool.path)
{
if let Some(path) = tool.path.file_name() {
let file_name = path.to_str().unwrap().to_owned();
let (target, clang) = file_name.split_at(file_name.rfind("-").unwrap());

tool.path.set_file_name(clang.trim_start_matches("-"));
tool.path.set_extension("exe");
tool.args.push(format!("--target={}", target).into());

// Additionally, shell scripts for target i686-linux-android versions 16 to 24
// pass the `mstackrealign` option so we do that here as well.
for version in 16..=23 {
if target.contains(&format!("i686-linux-android{}", version)) {
tool.args.push("-mstackrealign".into());
Copy link
Member

Choose a reason for hiding this comment

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

Is this what the current scripts do? Is there a reason to perhaps not pass this unconditionally? (I have no idea what this flag does)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea that's what the scripts do. Only the i686-linux-android16-23 have the -mstackrealign option added in the script, which force realigns the stack at entry of every function. Don't think we want to do that unconditionally as I am not sure what the flag does as well.

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, can you add a comment for why this arg is added? Also, is 23 the latest version? Or do newer versions not require this argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Newer versions do not require the arguments

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, in that case could this perhaps be tweaked a bit differently to instead of looping over versions to instead take a look at the target and try to parse a version at the end? If this no longer happens then this is basically just handling older historical compilers then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

}
}
};
}

// If we found `cl.exe` in our environment, the tool we're returning is
// an MSVC-like tool, *and* no env vars were set then set env vars for
// the tool that we're returning.
Expand Down