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.83k stars 442 forks source link

Add `Build::from_cargo` or similar to make it very explicit when in a build script #1219

Open madsmtm opened 1 week ago

madsmtm commented 1 week ago

cc's code is littered with brittle checks centered on rustc target triple. A lot of this is unnecessary though, since Cargo sets cfg values for the target as environment variables, see the docs for more examples. cc can't actually use those in most cases though, since it doesn't know whether it's being run inside a Cargo build script, or outside of it.

A prominent example is that CARGO_CFG_TARGET_ABI would be very useful for detecting Apple's simulator targets (doing it based on the target name is misery, and I think we're currently doing it wrong some places).

So I got to thinking, would it make sense to "split" cc's entry point into two, and deprecated cc::Build::new? Something like:

// build.rs
fn main() {
    // Emits all the rerun-if-changed flags, `cargo_metadata` defaults to true, etc.
    // Reads from `CARGO_CFG_TARGET_*` to initialize target data.
    cc::Build::from_cargo()
        .file("foo.c")
        .file("bar.c")
        .compile("foo");
}

// main.rs
fn main() {
    // Does not print needless Cargo metadata.
    // Using cc from outside build.rs, so has to specify target.
    // The target is parsed eagerly into some internal structure that represents it,
    // maybe using `target-lexicon`, and behind a feature flag? In any case, we
    // can say that this use-case is less supported, and the support burden will
    // perhaps be lessened?
    cc::Build::from_target("aarch64-unknown-linux-gnu")
        .file("foo.c")
        .file("bar.c")
        .compile("foo");
}
madsmtm commented 1 week ago

Actually, I think my biggest issue is the ad-hoc parsing of the target triple; ideally, that would be done first thing inside Build::try_compile as a completely separate step, and the rest of the code would never touch the unparsed target triple.

Would you accept a PR changing the target parsing to be up-front? And would you accept a private dependency on target-lexicon (also used by the Cranelift and GCC rustc backends)? Or would it need to be vendored (possibly with a script) ~to maintain the illusion of low compile times just because you don't have any dependencies~?

NobodyXu commented 1 week ago

Would you accept a PR changing the target parsing to be up-front?

Yeah I think having a new method calls from_cargo_build_script_env makes sense

And would you accept a private dependency on target-lexicon (also used by the Cranelift and GCC rustc backends)?

I think target-lexicon is small enough, as long as the serde feature is not enabled it should be ok to include it.

madsmtm commented 1 week ago

Hmm, I realized that target-lexicon has a build script to determine the host architecture, which would probably be a bit too expensive :/ (mostly in terms of "sequential" delay, Cargo would need to compile and link target-lexicon's build script, then compile target-lexicon, then compile cc, then compile and link the user's build script).

I have opened https://github.com/bytecodealliance/target-lexicon/issues/112 to track that, not sure whether I will start on this in cc before or after that has been resolved.