cxx-build: symlink headers to GENERATED_HEADER_DIR

[Be-ing]

Symlink (or copy on Windows) generated headers to a directory specified by the GENERATED_HEADER_DIR environment variable, if this variable is set. This makes cxx-build usable by C++ build systems if they set GENERATED_HEADER_DIR before calling cargo and add it to their include paths. This is a better solution than the CLI tool for several reasons:

• Don't need to build/distribute/install a separate program
• Saves build time because cxx crates are not built redundantly for the CLI tool and the macro
• No risk of version mismatches between CLI tool and cxx macro crate
• One library crate that uses cxx can be usable in applications built either with Cargo or a C++ build system

Fixes #462

[dtolnay]

This use of an environment variable is not compatible with Cargo's expectations around build script behavior. As written, this definitely isn't something we can land because it breaks any build script that does not already contain its own accurate "cargo:rerun-if-…" lines. If cxx-build prints this line, cargo will not rerun the build script for any other relevant reason, for example even source file changes in the file holding the #[cxx::bridge]. Meanwhile not printing this line is also not correct, because build scripts would end up not seeing changes to this environment variable.

Would you be able to propose some other ways to handle what you need in cxx-qt?

[Be-ing]

breaks any build script that does not already contain its own accurate "cargo:rerun-if-…" lines

Wouldn't such a build script kinda be broken already? The example in cxx-build's documentation clearly shows the build script should have cargo:rerun-if-changed lines for the bridge files and Cargo's documentation recommends this:

it is recommended that every build script emit at least one of the rerun-if instructions

On the other hand, since it is already the build script's responsibility to print rerun-if-changed for the bridge files, it would be consistent to leave rerun-if-env-changed=GENERATED_HEADER_DIR to the build script...

[dtolnay]

It would generally not already be kinda broken. rerun-if-changed is optional and Cargo has the sane default behavior of rerunning if any file in the project has changed. For example just from the first page of https://crates.io/crates/cxx-build/reverse_dependencies, here are several build scripts that are not broken today but would be broken by adding a rerun-if-env-changed into cxx-build:

• https://github.com/MaterializeInc/rust-protobuf-native/blob/protobuf-native-v0.2.1+3.19.1/protobuf-native/build.rs
• https://github.com/tsurucapital/quadprogpp-rs/blob/v0.1.0/quadprogpp-sys/build.rs
• https://github.com/risc0/risc0/blob/v0.11.1/risc0/core/build.rs (and 4 other build.rs in that repo)

[Be-ing]

Thinking about this further, either this must be the build script's responsibility, or cxx-build would need to only print this for bridges with extern Rust sections. Crates that only have extern C++ sections should not have this.

[Be-ing]

Even among crates with extern Rust sections, they might not be designed to be used with build systems besides Cargo. For example, the extern Rust could only be used to interact with C++ files within the crate that are compiled with cc by the Cargo build script. In that case, rerun-if-changed-env=GENERATED_HEADER_DIR would be wrong. So the decision whether to print rerun-if-changed-env must be left to the downstream build script.

The primary use case for this is for Rust libraries written to be linked into a specific C++ application. I doubt people will want to use this to create C++ bindings for general purpose Rust libraries, though that would technically be possible.

[Be-ing]

Note that if the cxx library has hand written C++ headers, its build script is responsible for copying those to GENERATED_HEADER_DIR (alternatively, the C++ build system could add the path to the crate's source code to its include paths, but I wouldn't recommend that).

[Be-ing]

PR to make use of this with CMake using Corrosion: corrosion-rs/corrosion#247

[dtolnay]

I got a chance to skim how corrosion and CMake handle integration of Cargo-based code, and some of the repos and issues that have been linked by all the discussions on this topic, and it reinforced my past impression that the whole thing is overwhelmingly a disaster (in contrast to buck+reindeer or bazel+raze). I feel strongly that this PR would not be a net improvement to corrosion's situation, and I don't think it is sufficient or necessary for handling cxx headers, so I'll close the PR.

Some examples of things that go wrong with the setup you have envisioned:

• If CMake depends on crate A which depends on crate B which depends on crate C, where A and C involve cxx-build, then both A's and C's generated headers would end up in the GENERATED_HEADER_DIR, which is not correct. Downstream code must only be allowed to import from A's headers.

• If either crate uses CFG.exported_header_dirs, CFG.exported_header_prefixes, or CFG.exported_header_links, the content of those directories would not be reflected in GENERATED_HEADER_DIR.

The behavior and experience implemented by cxx-build are specific to Cargo builds. If corrosion would like to stick non-Cargo build steps downstream of a Cargo-based invocation of cxx-build, the only correct way to implement that is by sufficiently accurately pretending to be Cargo in the rest of the build as well. For example: create a crate containing:

// lib.rs
#[cxx::bridge]
pub mod ffi {}

// build.rs
fn main() {
    cxx_build::bridge("lib.rs").compile("...");
}

In this crate, cxx-build guarantees that exactly the right include-directories will get set up as documented in https://cxx.rs/build/cargo.html, and handed to the underlying C++ compiler invocation. On top of that, the cc crate guarantees that a CXX environment variable will be respected in choosing what C++ compiler to invoke. So corrosion can build this crate with a CXX environment variable that points to a program that parses and extracts the include-directories from the compiler command line arguments.

[Be-ing]

Downstream code must only be allowed to import from A's headers.

If crate C can use crate A's headers, why shouldn't external C++ code?

If either crate uses CFG.exported_header_dirs, CFG.exported_header_prefixes, or CFG.exported_header_links, the content of those directories would not be reflected in GENERATED_HEADER_DIR.

Right, that would be up to build scripts to handle if they need that. Or alternatively I could put more work into this to copy/symlink headers from those paths into GENERATED_HEADER_DIR.

Corrosion can build this crate with a CXX environment variable that points to a program that parses and extracts the include-directories from the compiler command line arguments.

That would be a very convoluted workaround.

Overall I think you may have identified some legitimate challenges, but I think they could be overcome and this shouldn't be rushed to a premature conclusion.

[Be-ing]

If CMake depends on crate A which depends on crate B which depends on crate C, where A and C involve cxx-build

Oh, are you talking about a case where crate B uses crate A for bindings to some C++ library, but crate B doesn't re-export anything from crate A? In that case, you're right, a single environment variable might have unintended consequences. This could be overcome by including the crate name in the environment variable name, for example GENERATED_HEADER_DIR_my_cxx_library. Corrosion could still handle that easily.

[Be-ing]

If either crate uses CFG.exported_header_dirs, CFG.exported_header_prefixes, or CFG.exported_header_links, the content of those directories would not be reflected in GENERATED_HEADER_DIR.

This applies equally to the cxxbridge CLI tool.

[Be-ing]

I don't think it is sufficient or necessary for handling cxx headers, so I'll close the PR.

Please don't dismiss use cases outside of what you have personally encountered. The existing solution with cxxbridge is cumbersome, confusing (consider that all but one of the CMake examples listed in the documentation have a build.rs using cxx_build in addition to cxxbridge), and brittle. Quite a few users have communicated this (#462, #949, #229). I'm not saying it can't work, but that getting it working requires a lot more work than it needs to and there's lots of room for the build process to go wrong.

my work codebase using Buck has several hundred binding crates successfully built on it.

(from corrosion-rs/corrosion#244 (comment))
Clearly you have more experience with complex dependency graphs involving cxx than I do. I'm glad you've pointed out pitfalls in my code, but I don't think that means the whole approach should be dismissed. I think we can come up with a mutually agreeable solution together if the discussion isn't shut down prematurely. If #1120 (comment) isn't sufficient, perhaps some way could be added for symlinking/copying the generated headers to be opt-in for build scripts using cxx-build.