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

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

Merged
merged 11 commits into from
Dec 2, 2020
Merged

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

merged 11 commits into from
Dec 2, 2020

Conversation

maxded
Copy link
Contributor

@maxded maxded commented Nov 23, 2020

On Windows, the Android clang compiler is provided as a .cmd file instead of a .exe file. The .cmd file is spawned as a process and thus has a command-line limit of around 32000 characters, which you will most likely not reach. However, the .cmd launches the clang.exe, which results in a command-line limit of around 8200 characters, which larger projects with lots of include directories will easily reach.

As a fix, I've reintroduced response files when compiling from Windows to Android.

@alexcrichton
Copy link
Member

Thanks for the PR!

I was hoping it wouldn't come to this though since response files were just removed in #564. Is there any way to reasonably shorten the command line for your project? Or perhaps consider this a bug for Android that we can spawn the process but it can't spawn a subprocess?

@maxded maxded changed the title Generate response file when cross compiling from Windows to Android Call clang directly when cross-compiling from Windows to Android Nov 25, 2020
@maxded
Copy link
Contributor Author

maxded commented Nov 25, 2020

Good point! Was unsure myself whos responsibility this was to "fix". As shortening the command line for my project is not possible I've come up with a second, better, solution.

All the .cmd files do on Windows is call the main clang binary with the proper --target argument. As all that data is known to us we can construct this ourselves and call the main clang binary directly. :)

@alexcrichton
Copy link
Member

Seems like a nice strategy if we can get it to work! I think though that one of the purposes of #495 is that the wrapper scripts pass a different --target than rustc, and that's intentional for the compilation of the C code?

@maxded
Copy link
Contributor Author

maxded commented Nov 26, 2020

Yes they do pass a different --target, which instead of having the shell script pass it, I construct the proper --target myself and call clang directly.

A call to the aarch64-linux-android21-clang.cmd shell script internally calls clang.exe with --target=aarch64-linux-android21. Basically, the proper target and clang "version" (clang or clang++) is in the .cmd file name. So by extracting those ourselves we can call clang.exe or clang++.exe directly.

On Windows, I've checked all .cmd files and being able to extract the the proper --target is true for all of them! The i686-linux-android16 to 24 .cmd files also add the -mstackrealign argument, which I've done as well.

So unless I'm misunderstanding or missing something I think we're good to go. :)

src/lib.rs Outdated

Some(true)
};
f();
Copy link
Member

Choose a reason for hiding this comment

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

I think the inner closure should be ok here to remove, it's fine to just have one if let for the file_name binding and avoid the closure business.

src/lib.rs Outdated

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!

@alexcrichton alexcrichton merged commit f935d6b into rust-lang:master Dec 2, 2020
@alexcrichton
Copy link
Member

👍

Thanks again!

@maxded maxded mentioned this pull request Dec 3, 2020
@suqiernb
Copy link

// build.rs
extern crate cc;

fn main() {
    let mut build = cc::Build::new();
    let tool = build.get_compiler();
    println!("{:#?}", tool);
    build.file("src/core.c")
        .compile("core");
}
D:\XXX\demo>cargo build --release --target i686-linux-android
   Compiling demo v0.1.0 (D:\Development\WorkSpace\clion\rust\study\demo)
error: failed to run custom build command for `demo v0.1.0 (D:XXX\demo)`

Caused by:
  process didn't exit successfully: `D:\XXX\demo\target\release\build\demo-eaac7e9af5279e16\build-s
cript-build` (exit code: 1)
  --- stdout
  OPT_LEVEL = Some("3")
  TARGET = Some("i686-linux-android")
  HOST = Some("x86_64-pc-windows-msvc")
  CC_i686-linux-android = None
  CC_i686_linux_android = None
  TARGET_CC = None
  CC = None
  CFLAGS_i686-linux-android = None
  CFLAGS_i686_linux_android = None
  TARGET_CFLAGS = None
  CFLAGS = None
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("false")
  Tool {
      path: "clang.exe",
      cc_wrapper_path: None,
      cc_wrapper_args: [],
      args: [
          "--target=i686-linux-android",
          "-O3",
          "-DANDROID",
          "-ffunction-sections",
          "-fdata-sections",
          "-fPIC",
          "--target=i686-linux-android",
          "-Wall",
          "-Wextra",
      ],
      env: [],
      family: Clang,
      cuda: false,
      removed_args: [],
  }
  running: "clang.exe" "--target=i686-linux-android" "-O3" "-DANDROID" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=i686-linux-android" "-Wall" "-Wextra" "-o" "D:XXX\\demo\\target\\i686-linux-android\\release\\build\\demo-0d1113bc106cefdd\\out\\src/ffi/core.o" "-c" "src/ffi/core.c"

  --- stderr


  error occurred: Failed to find tool. Is `clang.exe` installed? (see https://github.com/alexcrichton/cc-rs#compile-time-requirements for help)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants