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

Need an explicit way for cxxbridge to produce header/source file #462

Closed
XiangpengHao opened this issue Nov 12, 2020 · 18 comments
Closed

Need an explicit way for cxxbridge to produce header/source file #462

XiangpengHao opened this issue Nov 12, 2020 · 18 comments
Labels
integration How cxx fits into the big picture of an organization's codebase and builds

Comments

@XiangpengHao
Copy link
Contributor

XiangpengHao commented Nov 12, 2020

The cxxbridge-cmd is not ideal:

  1. The cxxbridge-cmd version might be different from cxx version, which can cause problems
  2. It's hard to track file changes outside rust project, i.e. determine the best time to re-generate the header/source file

I used the side-effect from cxx_build::bridge(...) to generate the necessary files https://github.com/XiangpengHao/cxx-cmake-example/blob/27dfab58ff4888d4ede71fdad45a7d112640d2b0/rust_part/build.rs#L2 and then copy those files to desired locations https://github.com/XiangpengHao/cxx-cmake-example/blob/27dfab58ff4888d4ede71fdad45a7d112640d2b0/rust_part/CMakeLists.txt#L20

It would be great if we have some ways to explictly write the ouput file, maybe something like

// build.rs
fn main() {
    cxx_build::bridge("src/lib.rs").header("/path/lib.rs.h").source("/path/lib.rs.cpp");

    println!("cargo:rerun-if-changed=src/lib.rs");
}
@dtolnay dtolnay changed the title Need an explict way for cxxbridge to produce header/source file Need an explicit way for cxxbridge to produce header/source file Dec 13, 2020
@gui81
Copy link

gui81 commented Jul 14, 2021

I agree this would really be helpful. The build environment always has to be in sync with the version of cxx, which might encourage locking on an outdated version.

@Be-ing
Copy link

Be-ing commented Aug 27, 2022

I have recently overhauled cxx-qt's build process and stumbled over this problem. cxx-qt has its own code generation steps in addition to cxx. Initially, we had cxx-qt-builder generating C++ code from build.rs but not compiling it, instead relying on CMake to compile the generated C++ code. This was an ugly situation. cxx-qt-builder had to run at CMake's configure time so it could pass the paths of the generated files to CMake (involving a convoluted process of serializing C++ source code and file paths to JSON 😵 ).

My first attempt to improve this process was to create a CLI executable for cxx-qt analogous to cxxbridge. I got that to work with CMake, but once I had it working, I realized it wasn't a great idea. Several issues arise with a separate CLI executable written in Rust. The CLI executable has to be distributed somehow. I initially was thinking of doing this through vcpkg (in addition to some C++ code and a CMake module specific to cxx-qt, but those parts wouldn't be needed for cxx by itself), however, vcpkg doesn't (currently) support building Rust. cargo install is an option, but that is problematic because cxx and its dependencies have to be redundantly compiled in the cargo build for the Rust library, which would slow down build times. Furthermore, with the CLI executable separated from the compilation of the Rust code, that introduces the possibility of version mismatches between the CLI executable and the cxx version used in the Rust code. That could be mitigated with version checks, but implementing that would be cumbersome (Which part of the build process would be responsible for doing the version check? The CLI tool? The C++ build system? The Rust macro?).

Instead of using a CLI tool, I refactored cxx-qt-builder to work similar to cxx-builder. Now, the build.rs not only generates the C++ code, it also compiles the C++ using the cc crate. Implementing this turned out to be tricky because Cargo does not have a canonical way for build scripts to output artifacts to a stable location. I read #949 and #229 but didn't see any good solution. I noticed that cxx-build implemented a hack to walk up the directory tree from OUT_DIR in an attempt to find the Cargo target directory and symlink headers from a cxxbridge directory at the root of CARGO_TARGET_DIR. I got something similar working in a proof-of-concept by having CMake set the CARGO_TARGET_DIR environment variable so build.rs could make use of it. However, I decided against this approach because I don't want to rely on unstable implementation details of Cargo.

The solution I came up with is to have CMake set an environment variable called CXXQT_EXPORT_DIR to explicitly tell cxx-qt-builder where to output headers. CMake sets this so cxx-qt-builder outputs the files into CMake's build directory instead of Cargo's build directory. This way, CMake can reliably add that directory to its include paths:

set(CRATE qml-minimal)
# Corrosion creates a CMake target with the same name as the crate.
corrosion_import_crate(MANIFEST_PATH src/rust/Cargo.toml CRATES ${CRATE})
# The Rust library's build script needs to be told where to output the generated headers so CMake can find them
# by setting the CXXQT_EXPORT_DIR environment variable.
set(CXXQT_EXPORT_DIR "${CMAKE_CURRENT_BINARY_DIR}/cxxqt")
corrosion_set_env_vars(${CRATE}
    "CXXQT_EXPORT_DIR=${CXXQT_EXPORT_DIR}"
    "QMAKE=${QMAKE}" # specific to cxx-qt's build process; wouldn't be needed for cxx
)
# Include the headers generated by the Rust library's build script. Each crate gets its own subdirectory under
# CXXQT_EXPORT_DIR. This allows you to include headers generated by multiple crates without risk of one crate
# overwriting another's files.
target_include_directories(${CRATE} INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}")

In #949 (comment) @dtolnay said:

The cxx-build crate is definitely intended only for pure Cargo managed builds, so plumbing its outputs into a CMake managed build is not an endorsed use case.

I would like to change this by implementing the above approach where cxx-build would output its generated headers to a path specified by an environment variable. If the environment variable is not specified, which would be the case for doing the entire build with Cargo, then use OUT_DIR instead. I propose calling the environment variable CXX_EXPORT_DIR. This way, Corrosion could automatically set this environment variable and add it to the C++ include paths. The result would be that integrating a crate which has cxx bindings into a CMake build would be as simple as using Corrosion normally:

set(CRATE crate-name)
corrosion_import_crate(MANIFEST_PATH src/rust/Cargo.toml CRATES ${CRATE})
target_link_libraries(my_application PUBLIC ${CRATE})

If this is accepted upstream by both cxx and Corrosion, that could further simplify the process for downstreams using cxx-qt. I could change cxx-qt-builder to use the same environment variable as cxx-builder. The end result would allow simplifying the CMake code required to integrate a Rust library using cxx-qt into a CMake build:

set(CRATE qml-minimal)
corrosion_import_crate(MANIFEST_PATH src/rust/Cargo.toml CRATES ${CRATE})

# specific to cxx-qt's build process; wouldn't be needed for cxx
corrosion_set_env_vars(${CRATE} "QMAKE=${QMAKE}")
target_link_libraries(${CRATE} INTERFACE
    Qt${QT_VERSION_MAJOR}::Core
    Qt${QT_VERSION_MAJOR}::Gui
    Qt${QT_VERSION_MAJOR}::Qml
    Qt${QT_VERSION_MAJOR}::QuickControls2
)

target_link_libraries(my_application PUBLIC ${CRATE})

I have been talking about integrating cxx & cxx-qt with CMake, but this would also make it easy to integrate them with other C++ build systems. All that would be required from the C++ build system is setting the environment variable when calling cargo build and adding it to the C++ include paths.

@dtolnay what do you think of this proposal? Pinging the Corrosion maintainer @jschwe as well.

@dtolnay
Copy link
Owner

dtolnay commented Aug 27, 2022

The cxx-build crate is definitely intended only for pure Cargo managed builds, so plumbing its outputs into a CMake managed build is not an endorsed use case.

This is my preference still.

At a glance, what you are doing in cxx-qt seems similar to how https://github.com/google/autocxx uses cxx (just in the superficial sense that there is something which generates input for cxx to operate on, runs cxx's C++ code generator, generates some other stuff of its own, and links them all together).

@adetaylor created https://crates.io/crates/cxx-gen specifically for this purpose. @Be-ing could you look into whether that meets your needs? The documentation is not thorough and some aspects are not fleshed out — for example in comparison with cxx-build and the cxxbridge CLI, it produces worse error messages, and it does not expose any cfg attribute support unlike the other two. Those are all things that can be fixed; cxx-build is not an appropriate interface for your use case.

@Be-ing
Copy link

Be-ing commented Aug 27, 2022

Could you elaborate why that is your preference? I don't think a separate crate is a good solution. The beauty of cxx-builder handling this is that downstream users of cxx wouldn't have to do anything to support both building executables with Cargo alone and having a C++ build system invoke Cargo to build a library and linking it into a C++ application.

@Be-ing
Copy link

Be-ing commented Aug 27, 2022

cxx-qt-build is already using cxx-gen in its code generation. That's orthogonal to the issue here about outputting the generated C++ headers somewhere a C++ build system can add them to include paths.

The bidirectionality of cxx's bindings are one of its big strengths, but without a reliable solution to this issue, it can't easily be integrated into existing C++ applications.

@Be-ing
Copy link

Be-ing commented Aug 27, 2022

cxx-build is not an appropriate interface for your use case

You're right, it's not, and I agree it should not be. I'm not trying to use it in cxx-qt-build. We are using cxx-build in cxx-qt-lib's build.rs for bindings to common Qt types. cxx-qt-build is a separate crate that uses cxx-gen plus adds Qt-specific code generation steps. cxx-qt-lib does not depend on cxx-qt-build. cxx-qt-build is used by downstream libraries or applications in their build.rs. cxx-qt-build can be used to build both libraries linked into a C++ application or used to build Rust executables only using Cargo without a C++ build system.

The goal of my proposal is not to use cxx-build in cxx-qt-build. What I am proposing is to make cxx-build work similar to how I've implemented cxx-qt-build by outputting the C++ headers to a location specified by an environment variable (only if the environment variable exists, otherwise cxx-build will continue working as it does now using OUT_DIR). This would allow Corrosion to automatically handle setting the environment variable and C++ include paths. After that, cxx-qt-build and cxx-build could converge on using the same environment variable. In the end, using cxx-build or cxx-qt-build with Corrosion would be the same as using any other Rust library; downstream users wouldn't have to be concerned with any of this.

@Be-ing
Copy link

Be-ing commented Aug 27, 2022

(By the way, cxx-qt's documentation and README are in need of updating. We've changed a lot in the past several weeks but haven't updated the documentation yet. I'll get to work on that next week.)

@Be-ing
Copy link

Be-ing commented Oct 7, 2022

make cxx-build work similar to how I've implemented cxx-qt-build by outputting the C++ headers to a location specified by an environment variable (only if the environment variable exists, otherwise cxx-build will continue working as it does now using OUT_DIR). This would allow Corrosion to automatically handle setting the environment variable and C++ include paths. After that, cxx-qt-build and cxx-build could converge on using the same environment variable. In the end, using cxx-build or cxx-qt-build with Corrosion would be the same as using any other Rust library; downstream users wouldn't have to be concerned with any of this.

@dtolnay what are your thoughts on this? I'd be happy to make a PR implementing this if you're on board with the idea.

Be-ing added a commit to Be-ing/cxx that referenced this issue Oct 28, 2022
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 dtolnay#462
Be-ing added a commit to Be-ing/cxx that referenced this issue Oct 28, 2022
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 dtolnay#462
@dtolnay
Copy link
Owner

dtolnay commented Oct 29, 2022

The cxxbridge-cmd is not ideal:

  1. The cxxbridge-cmd version might be different from cxx version, which can cause problems
  2. It's hard to track file changes outside rust project, i.e. determine the best time to re-generate the header/source file

Neither of these factors is inherent to cxxbridge-cmd, it's only in regard to how a particular build system chooses to integrate it. Done well, neither of those should occur. There's examples in https://github.com/dtolnay/cxx/blob/1.0.80/demo/BUCK and https://github.com/dtolnay/cxx/blob/1.0.80/demo/BUILD of how Buck and Bazel would do it.

  1. Regarding (1), when someone runs bazel build //demo:demo, cxxbridge-cmd is built from source just like cxx and all the rest of the dependencies would be. There is no reliance on a (possibly incongruous version) cxxbridge command on the person's $PATH or accessed through a distro package manager. The build system hermetically builds everything necessary (or downloads from cache). You'd coordinate the cxx and cxxbridge-cmd version exactly the same as how you'd otherwise coordinate a cxx and cxx-build version.

  2. Regarding (2), the build system has perfect knowledge of the inputs and outputs of every build step, because it's the one dispatching the cxxbridge invocation, it's not hidden in a build.rs whose behavior is opaque to the build system. The build system will rerun the cxxbridge invocation if and only if the input .rs file containing #[cxx::bridge] has changed, or the headers included by the bridge have changed, or the cxxbridge binary itself has needed to be rebuilt (because of version update or any other reason).

For build systems that aren't going to be able to accommodate a setup similar to that, I left a comment in #1120 (review) laying out a way for them to reliably integrate cxx-build instead, using an invocation of cxx-build performed by a Cargo build.rs.

@dtolnay dtolnay closed this as completed Oct 29, 2022
@dtolnay dtolnay added the integration How cxx fits into the big picture of an organization's codebase and builds label Oct 29, 2022
@Be-ing
Copy link

Be-ing commented Oct 29, 2022

There's examples in https://github.com/dtolnay/cxx/blob/1.0.80/demo/BUCK and https://github.com/dtolnay/cxx/blob/1.0.80/demo/BUILD of how Buck and Bazel would do it.

That is a special situation because it's within this repository where cxxbridge and the cxx proc_macro are maintained together. That is not the case downstream, where the cxx macro comes from Cargo via crates.io and cxxbridge comes from... well, it's not clear where it's supposed to come from. You could do cargo install --version, but then you'd have to get the version from somewhere. You could manually duplicate the version to whatever part of the build process does cargo install, but that would easily get out of sync with the macro crate whenever cargo update is run. Plus, the build artifacts wouldn't be shared between cargo install and the crate using the cxx macro, which would build cxx-gen and its dependencies twice. That could be alleviated by caching the built cxxbridge binary on CI, but you'd have to ensure that cache gets invalidated whenever the cxx macro is updated.

All of this would be much easier to setup, less error-prone to maintain, and build faster by keeping the versions of cxx and cxx-build together in Cargo.toml, where they would both be updated together with cargo update.

For build systems that aren't going to be able to accommodate a setup similar to that, I left a comment in #1120 (review) laying out a way for them to reliably integrate cxx-build instead, using an invocation of cxx-build performed by a Cargo build.rs.

That suggestion would be even more complex than using cxxbridge and doesn't address any of the concerns raised in this issue.

The cxx-build crate is definitely intended only for pure Cargo managed builds, so plumbing its outputs into a CMake managed build is not an endorsed use case... This is my preference still.

Why? I haven't seen any technical reasons explained why cxx-build shouldn't have some way to output its generated headers somewhere an external C++ build system knows where they are. You raised some concerns with the specific implementation in #1120, but as far as I can tell, those could be easily addressed.

@dtolnay
Copy link
Owner

dtolnay commented Oct 29, 2022

That is a special situation because it's within this repository where cxxbridge and the cxx proc_macro are maintained together.

It isn't really, the Bazel rules look substantially identical to my link regardless of whether they're in this repo or a different one. For example, here is one using rust_cxx_bridge via Bazel in a different repo: anysphere-messaging/client/daemon/BUILD. Similarly, this repo's Bazel rules build clap and plenty of other crates.io libraries which I am not author of and are not maintained in this repo. Controlling versions of them is not any different from usual, they are determined by third-party/Cargo.lock and updated by cargo update.

All of this would be much easier to setup, less error-prone to maintain, and build faster by keeping the versions of cxx and cxx-build together in Cargo.toml, where they would both be updated together with cargo update.

If you like that workflow, a non-Cargo build system that nevertheless involves Cargo can do exactly the same thing for cxxbridge-cmd. You can list it in Cargo.toml. It will appear in Cargo.lock and you can update it using cargo update. That aspect is not different from cxx-build.

You could do cargo install --version, but then you'd have to get the version from somewhere. You could manually duplicate the version to whatever part of the build process does cargo install, but that would easily get out of sync with the macro crate whenever cargo update is run.

It sounds like you're desiring for it to appear in Cargo.lock. You can read it from there using e.g. cargo tree -i cxxbridge-cmd --depth 0.

Plus, the build artifacts wouldn't be shared between cargo install and the crate using the cxx macro, which would build cxx-gen and its dependencies twice.

Neither cxx nor cxxbridge-cmd depends on cxx-gen (other than as a dev-dependency for use in the example code in docs, which isn't relevant here). For sharing the rest of the dependencies like syn, that's a matter of using the same target directory for both builds, either via CARGO_TARGET_DIR in the environment, or Cargo's --target-dir command line flag.

To be clear, in a non-Cargo build system that involves performing Cargo invocations as part of the build, this is not what you should use. The command-line code generator is primarily intended for build systems that wouldn't involve Cargo at build time (Buck and Bazel are examples of this). For setups that already use Cargo and run cxx-build from a build.rs, #1120 (review) is the recommended approach and you don't need to worry about how a cxxbridge-cmd gets installed.

I haven't seen any technical reasons explained why cxx-build shouldn't have some way to output its generated headers somewhere an external C++ build system knows where they are.

The technical reason is that I get to not support CMake which continues to give me the impression of being a disaster of a build system from everything that I have seen, at least as far as the integration of Cargo packages has been pursued to date. By following the approach in #1120 (review), whenever there is a CMake-specific way that CMake is unable to deliver the cxx-build functionality explained in https://cxx.rs/build/cargo.html, it's clear it needs to be fixed in corrosion/CMake. In the other way, whenever there is a CMake-specific way that something does not work, I become responsible for code changes in cxx-build to accommodating CMake, which is not appealing to me.

@Be-ing
Copy link

Be-ing commented Oct 30, 2022

The command-line code generator is primarily intended for build systems that wouldn't involve Cargo at build time (Buck and Bazel are examples of this).

Ah, thank you for explaining this. The documentation does not communicate this. I think this has been a big point of confusion.

CMake does not have any facilities for invoking rustc on its own. That is not necessary to incorporate Rust into a CMake build, nor am I aware of anyone attempting or asking for that. I searched the CMake issue tracker and the web with DuckDuckGo and don't see anybody asking for that. Judging from this comment from a CMake developer, I highly doubt CMake will ever take up the maintenance burden of calling rustc itself. Instead, I see on the web various attempts at having CMake call Cargo. Of these, Corrosion seems to be the most mature, and in my experience it works quite well. What Corrosion does is create CMake targets that call Cargo to build a library and link it into the library/executable that CMake builds.

For the case of build systems that bypass Cargo and call rustc themselves so build.rs isn't available, yes, the cxxbridge CLI tool makes sense. Now I understand why you thought my suggestion to deprecate cxxbridge was ridiculous. However, I still think using cxxbridge is far from an ideal solution for C++ build systems that don't have any features to build Rust on their own without Cargo.

If you like that workflow, a non-Cargo build system that nevertheless involves Cargo can do exactly the same thing for cxxbridge-cmd. You can list it in Cargo.toml. It will appear in Cargo.lock and you can update it using cargo update. That aspect is not different from cxx-build.

I tested this, and yes you can keep track of the cxxbridge version that way... but that still doesn't solve the problem because Cargo won't build the cxxbridge binary target when it's listed as a dependency of another crate.

It sounds like you're desiring for it to appear in Cargo.lock. You can read it from there using e.g. cargo tree -i cxxbridge-cmd --depth 0.

That's neat, thanks for sharing that.

For setups that already use Cargo and run cxx-build from a build.rs, #1120 (review) is the recommended approach and you don't need to worry about how a cxxbridge-cmd gets installed.

That addresses a nonexistent problem. There is no need for the C++ build system to get all of the include paths that cxx-build uses. If the crate using cxx needs include paths for some C++ library, the C++ build system already needs to find that library because the C++ build system does the final link. This would be the same for cxxbridge.

The only thing that is missing for cxx-build to work well with any C++ build system to provide a means for the C++ build system to tell cxx-build where to put the generated headers so the C++ build system can add them to its include paths.

The technical reason is that I get to not support CMake which continues to give me the impression of being a disaster of a build system from everything that I have seen, at least as far as the integration of Cargo packages has been pursued to date.

I have Opinions about build systems as well and I've intentionally left that out of this discussion. I have plenty of gripes about CMake myself, but like it or not, CMake and lots of other build systems that aren't Bazel are widely used in lots of contexts. We could bikeshed forever about the pros and cons of various C++ build systems. The issues raised here apply to any C++ build system that invokes Cargo to build a library as part of a C++ application or library.

If the first step to incorporating Rust into an existing C++ project is replacing the whole build system... well, that's just not going to happen in most cases. More likely, C++ developers considering adding Rust into their application would give up on the idea of using Rust at all. In order for it to be practical to incrementally introduce Rust into a C++ codebase, it must be practical to integrate with the C++ build systems people are already using.

whenever there is a CMake-specific way that something does not work, I become responsible for code changes in cxx-build to accommodating CMake, which is not appealing to me.

I understand your concern with this. Feel free to ping me whenever such issues are reported. That said, I anticipate people will bug you less about using CXX with CMake when there's an easy to use solution that is documented well. As #1120 demonstrates, all that is needed is adding about 10 lines of code to cxx-build which can be written such that it has no effect on how cxx-build works unless a C++ build system explicitly opts into using it. That's all I'm asking to change about cxx-build. I don't anticipate that being a large maintenance burden.

@jschwe
Copy link

jschwe commented Oct 30, 2022

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.

This is something that Corrosion definitely does not want to support. Corrosion supports two scenarios:

  • Let linking be handled by CMake
  • Let linking be handled by Rust

If someone wants to let Rust invoke the linker, then the build.rs cxx approach already works today, since it is all handled by cargo. If you want linking to be handled by CMake, then in my opinion the command-linecxx-bridge tool would be the preferred approach, since CMake is then aware of what happens.
@Be-ing's suggestion is to support using the build.rs approach, but still handle the linking on the CMake side. I think @dtolnay has given good reasons why this is problematic (mainly that CMake is completely oblivious of all the println!() commands given from the build.rs to cargo).

@Be-ing
Copy link

Be-ing commented Oct 30, 2022

If you want linking to be handled by CMake, then in my opinion the command-linecxx-bridge tool would be the preferred approach, since CMake is then aware of what happens.
@Be-ing's suggestion is to support using the build.rs approach, but still handle the linking on the CMake side. I think @dtolnay has given good reasons why this is problematic (mainly that CMake is completely oblivious of all the println!() commands given from the build.rs to cargo).

You're correct that compiling the generated C++ code in build.rs and having a C++ build system do the final link requires both build.rs and the C++ build system to find external C++ libraries used by the crate which uses cxx. However, in my experience, this is required regardless, otherwise cargo test won't work for the crate that uses cxx. Also, for the crate using cxx to be usable in both cases (building with only Cargo or invoking Cargo from a C++ build system), build.rs needs to locate the external C++ library.

@dtolnay
Copy link
Owner

dtolnay commented Oct 30, 2022

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.

This is something that Corrosion definitely does not want to support.

I guess I don't mean Corrosion here, but "person using Corrosion". This thread exists because if you have C++ calling Rust, and the Rust library built using Cargo exposes a C++ API using cxx, your C++ code needs to have cxx's generated header on its include path. That's what I mean by a person sticking a non-Cargo build step (the C++ code, which calls Rust) downstream of a Cargo-based build.

I think this falls under "Let linking be handled by CMake" in your characterization, but linking is really orthogonal to all of this about headers and include paths, which @Be-ing's last comment also indicates. You could just as easily have C++ call Rust but still have the final link handled by Rust.

If Corrosion is the one building the Rust code (which is depended on by (obviously non-Corrosion) C++ code), then I think the author would want Corrosion to be able to expose an include path through which their C++ code becomes able to access the appropriate headers that their Rust code implements, which is what #1120 (review) gets at.

@Be-ing
Copy link

Be-ing commented Oct 30, 2022

Corrosion supports two scenarios:

  • Let linking be handled by CMake
  • Let linking be handled by Rust

What do you mean by Corrosion supporting linking being handled by Rust? If Rust does the final link, why use Corrosion at all?

@Be-ing
Copy link

Be-ing commented Oct 30, 2022

linking is really orthogonal to all of this about headers and include paths

Kinda, but it is related. When I tried the CLI tool approach, MSVC's linker failed to link Cargo's tests with a bunch of errors like this:

          qml_minimal-fed87d1c9e36c7ca.1ac972c8m5b6xrub.rcgu.o : error LNK2019: unresolved external symbol cxx_qt$my_object$cxxbridge1$MyObject$set_number referenced in function _ZN11qml_minimal9my_object8MyObject10set_number17h0180ea11ac71a14aE
          qml_minimal-fed87d1c9e36c7ca.1ac972c8m5b6xrub.rcgu.o : error LNK2019: unresolved external symbol cxx_qt$my_object$cxxbridge1$MyObject$set_string referenced in function _ZN11qml_minimal9my_object8MyObject10set_string17hb5108fc7fb9215adE
          libcxx_qt_lib-c5c11005f6c39e76.rlib(cxx_qt_lib-c5c11005f6c39e76.38ljfw97mgteislm.rcgu.o) : error LNK2019: unresolved external symbol rust$cxxqtlib1$cxxbridge1$qstring_init_from_rust_string referenced in function _ZN10cxx_qt_lib5types7qstring3ffi29qstring_init_from_rust_string17h8f75fa133531d4bdE
          libcxx_qt_lib-c5c11005f6c39e76.rlib(cxx_qt_lib-c5c11005f6c39e76.38ljfw97mgteislm.rcgu.o) : error LNK2019: unresolved external symbol rust$cxxqtlib1$cxxbridge1$qstring_to_rust_string referenced in function _ZN10cxx_qt_lib5types7qstring3ffi22qstring_to_rust_string17h12bc99a963e8e90bE
          libcxx_qt_lib-c5c11005f6c39e76.rlib(cxx_qt_lib-c5c11005f6c39e76.38ljfw97mgteislm.rcgu.o) : error LNK2019: unresolved external symbol cxxbridge1$unique_ptr$QString$null referenced in function _ZN93_$LT$cxx_qt_lib..types..qstring..ffi..QString$u20$as$u20$cxx..unique_ptr..UniquePtrTarget$GT$6__null17hd9ed30e5994f00b1E

I didn't investigate exactly what caused these errors, but they went away when switching to compiling the generated C++ in build.rs.

@decathorpe
Copy link

@Be-ing asked me to comment here.

From my experience (packaging 1000-2000 Rust crates for Fedora Linux), the way bindgen supports this has proven to be very flexible and robust. While it provides a bindgen executable that can be run "manually", its library interface is often preferred, as it can be called in build.rs scripts, and is hence automatically integrated into the cargo build process. Additionally, bindgen provides an API for specifying the output directory for any generated files. I know it's not an exact comparison (cxx generates C++ files, bindgen generates Rust files), but both are code generators, and being able to tell them where to actually put their output is kind of an important feature / setting.

Repository owner locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
integration How cxx fits into the big picture of an organization's codebase and builds
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants