search

rust-lang / cc-rs

Rust library for build scripts to compile C/C++ code into a Rust library
https://docs.rs/cc
Apache License 2.0
1.81k stars 434 forks source link

Fix regression: remove `--target` when targeting Android on non-Windows platforms #991

Closed dpaoliello closed 6 months ago

dpaoliello commented 6 months ago

The --target argument should not be passed to Clang when targeting Android, as the Android NDK contains wrappers for Clang that automatically set the target.

However, https://github.com/rust-lang/cc-rs/commit/53564e00498156c9be00361c4b039952cbf7ff59 introduced a regression where --target was being passed on non-Windows platforms, which broke the compile-ui tests for the Rust compiler when building for arm-android: https://github.com/rust-lang/rust/pull/121854#issuecomment-1973518078

This fix restores the original logic AND respects the new has_internal_target_arg flag (regardless if we're targeting Android or not).

I've verified that the arm-android build succeeds with this change: https://github.com/rust-lang/rust/actions/runs/8146073082/job/22263762750?pr=121874

Fixes #990

dpaoliello commented 6 months ago

FYI, @NobodyXu @ChrisDenton

dpaoliello commented 6 months ago

Is there anyway of preventing such breakage by adding regression tests?

Added.

dpaoliello commented 6 months ago

@NobodyXu can we please get a release with this fix, I believe it's the only blocker left to updating cc-rs in the Rust toolchain

NobodyXu commented 6 months ago

Created release, waiting for publish