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

Bidirectional cxxbridge Issues #280

Open
Xaldew opened this issue Dec 20, 2022 · 20 comments
Open

Bidirectional cxxbridge Issues #280

Xaldew opened this issue Dec 20, 2022 · 20 comments

Comments

@Xaldew
Copy link

Xaldew commented Dec 20, 2022

Apologies in advance if this is the wrong place to ask this, but anyway:

I have tried for a bit to use Corrosion to build and share code between a C++
project and a Rust project, starting with a simple example such as reading
images on the C++ side and sending the results back.

The CMake file is pretty straightforward:

# Rust <-> C++ integration.
find_package(corrosion)

if (corrosion_FOUND)
  corrosion_import_crate(MANIFEST_PATH ${PROJECT_SOURCE_DIR}/ext/gfx-rs/Cargo.toml)
  corrosion_add_cxxbridge(imageio-cxx
    CRATE gfx
    MANIFEST_PATH ${PROJECT_SOURCE_DIR}/ext/gfx-rs
    FILES imageio_cxx.rs)
  corrosion_link_libraries(gfx ccl_common)
endif()

The C++ bridge looks something like this:

#include "gfx/src/imageio_cxx.rs.h"
#include "rust/cxx.h"

struct Rgba;
struct RsImage;

RsImage read_image(const rust::Str path);
void write_image(const rust::Str path, const RsImage &image);

and the Rust side of the same looks like this:

#[cxx::bridge]
pub mod ffi
{
    #[derive(Debug)]
    struct Rgba
    {
        r: f32,
        g: f32,
        b: f32,
        a: f32,
    }

    #[derive(Debug)]
    struct RsImage
    {
        width: usize,
        height: usize,
        raster: Vec<Rgba>,
    }
    unsafe extern "C++"
    {
        include!("gfx/external/imageio-bridge.h");
        fn read_image(path: &str) -> Result<RsImage>;
        fn write_image(path: &str, image: &RsImage) -> Result<()>;
    }
}

The build process works just fine, but any rust binary that attempts to use the
bridge triggers a linker error:

  = note: Undefined symbols for architecture arm64:
            "_cxxbridge1$read_image", referenced from:
                gfx::imageio_cxx::ffi::read_image::h5122f98c3963e4d5 in libgfx.rlib(gfx.gfx.58b6ab36-cgu.5.rcgu.o)
          ld: symbol(s) not found for architecture arm64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

Some of the examples I found in the repository suggested that I should construct
the bridge on the CMake side (e.g., the test/cxxbridge), so I tried to add the
following lines to the CMakeLists.txt:

  add_library(imageio-bridge ${PROJECT_SOURCE_DIR}/ext/gfx-rs/external/imageio-bridge.cpp)
  target_link_libraries(imageio-bridge PUBLIC imageio-cxx)
  corrosion_link_libraries(gfx imageio-cxx)

This however seems to trigger dependency graph cycles in CMake:

CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
  "_cargo-build_gfx" of type UTILITY
    depends on "imageio-cxx" (strong)
  "cargo-build_gfx" of type UTILITY
    depends on "_cargo-build_gfx" (strong)
  "imageio-cxx" of type STATIC_LIBRARY
    depends on "cargo-build_gfx" (strong)
    depends on "_cargo-build_gfx" (strong)

So, I'm currently at a bit of loss, what is the recommended way for constructing
these kinds of bridges?

If nothing else, perhaps a slightly larger example with a bidirectional
cxxbridge is warranted?

@jschwe
Copy link
Collaborator

jschwe commented Dec 20, 2022

I'm not too familiar with cxx, so take everything I say with a grain of salt.

This however seems to trigger dependency graph cycles in CMake:

Looking at your example, it seems this is because you do have a cycle in your headers. The rust bridge depends on the C++ header and the C++ header depends on the rust bridge.
Could you split your C++ header into two files, so that you have a clean split between what functions / types C++ exports and the generated header from the Rust side?

@Xaldew
Copy link
Author

Xaldew commented Dec 20, 2022

Looking at your example, it seems this is because you do have a cycle in your headers. The rust bridge depends on the > C++ header and the C++ header depends on the rust bridge.
Could you split your C++ header into two files, so that you have a clean split between what functions / types C++ > exports and the generated header from the Rust side?

Hmm. It is possible to remove the bridge-include in the header, e.g.:

#ifndef EXTERNAL_IMAGEIO_BRIDGE_H
#define EXTERNAL_IMAGEIO_BRIDGE_H

#include "rust/cxx.h"

struct Rgba;
struct RsImage;

RsImage read_image(const rust::Str path);
void write_image(const rust::Str path, const RsImage &image);

#endif /* EXTERNAL_IMAGEIO_BRIDGE_H */

But this didn't change anything though. Or am I misunderstanding your suggestion?

@jschwe
Copy link
Collaborator

jschwe commented Dec 21, 2022

But this didn't change anything though. Or am I misunderstanding your suggestion?

Well, once you removed the cyclic depenency in the headers you would also have to adapt the CMake dependencies

corrosion_link_libraries(gfx imageio-cxx)

You probably want to remove this line - I would expect you would want to link the rust bindings into a C++ library and not into the rust crate.

@Xaldew
Copy link
Author

Xaldew commented Jan 5, 2023

Appologies for the slow replies, ended up being rather busy during the holidays, forgetting this thread. Anyways:

You probably want to remove this line - I would expect you would want to link the rust bindings into a C++ library and > not into the rust crate.

Removing that line does seem to work to fix the CMake dependency cycle, but it unfortunately comes back to the other problem of missing the read_image() symbols:

  = note: Undefined symbols for architecture arm64:
            "_cxxbridge1$read_image", referenced from:
                gfx::imageio_cxx::ffi::read_image::h5122f98c3963e4d5 in libgfx.rlib(gfx.gfx.58b6ab36-cgu.5.rcgu.o)
          ld: symbol(s) not found for architecture arm64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

I guess there's something missing on the Rust side then to make these symbols (that are compiled on the CMake side) available?

@jschwe
Copy link
Collaborator

jschwe commented Jan 5, 2023

In which language was the final executable defined again? Rust or C++?

@Xaldew
Copy link
Author

Xaldew commented Jan 5, 2023

The final executable in this case is a Rust binary called "texture_test", in case it's helpful, here's the full commandline as it is output by cmake .. && make VERBOSE=1:

[  1%] Built target cargo-prebuild_texture_test
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/_cargo-build_texture_test.dir/build.make CMakeFiles/_cargo-build_texture_test.dir/depend
cd /Users/gustaf/Documents/git/phd/ocl-spikes/build && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E cmake_depends "Unix Makefiles" /Users/gustaf/Documents/git/phd/ocl-spikes /Users/gustaf/Documents/git/phd/ocl-spikes /Users/gustaf/Documents/git/phd/ocl-spikes/build /Users/gustaf/Documents/git/phd/ocl-spikes/build /Users/gustaf/Documents/git/phd/ocl-spikes/build/CMakeFiles/_cargo-build_texture_test.dir/DependInfo.cmake --color=
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/_cargo-build_texture_test.dir/build.make CMakeFiles/_cargo-build_texture_test.dir/build
cd /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E env CARGO_TARGET_AARCH64_APPLE_DARWIN_LINKER=/Library/Developer/CommandLineTools/usr/bin/c++ CC_aarch64-apple-darwin=/Library/Developer/CommandLineTools/usr/bin/cc CXX_aarch64-apple-darwin=/Library/Developer/CommandLineTools/usr/bin/c++ SDKROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk LIBRARY_PATH=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib CORROSION_BUILD_DIR=/Users/gustaf/Documents/git/phd/ocl-spikes/build CARGO_BUILD_RUSTC=/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/rustc /Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/cargo rustc --bin=texture_test --target=aarch64-apple-darwin --package gfx --manifest-path /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs/Cargo.toml --target-dir /Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build --release -- -Cdefault-linker-libraries=yes
   Compiling gfx v0.1.0 (/Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs)
error: linking with `/Library/Developer/CommandLineTools/usr/bin/c++` failed: exit status: 1
  |
  = note: "/Library/Developer/CommandLineTools/usr/bin/c++" "-arch" "arm64" "/var/folders/ns/5p2ryw0x1p153x3tcqwffks40000gn/T/rustcm9SRKg/symbols.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.0.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.1.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.10.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.11.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.12.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.13.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.14.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.15.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.2.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.3.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.4.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.5.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.6.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.7.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.8.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.texture_test.285bcc05-cgu.9.rcgu.o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d.2r6ljxq388bfe1fs.rcgu.o" "-L" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps" "-L" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/release/deps" "-L" "/opt/homebrew/Cellar/libpng/1.6.39/lib" "-L" "/usr/lib" "-L" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/build/cxx-47de44de887f8106/out" "-L" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/build/link-cplusplus-ba9513de36dbbdf2/out" "-L" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/build/glfw-sys-d5f6fc13adedfd7a/out/lib" "-L" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/cargo/build/aarch64-apple-darwin/release/deps/libgfx.rlib" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/cargo/build/aarch64-apple-darwin/release/deps/libcxx-dbe2129b19e7a478.rlib" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/cargo/build/aarch64-apple-darwin/release/deps/liblink_cplusplus-20a395290580a1d3.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libstd-08303b9d67b1c21b.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libpanic_unwind-cb2524b0f4cc7267.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libobject-5be164d1477b18d7.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libmemchr-e76ae1a2b71c35f2.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libaddr2line-9e2eeaad94fb258f.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libgimli-3b7dc96191cf7ba5.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/librustc_demangle-194008f6ba023a3e.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libstd_detect-edbcf7cf2c5b3fee.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libhashbrown-46a307aa7f715ac6.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libminiz_oxide-28a12297d98f4e18.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libadler-bc8ccbf9e2ac7439.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/librustc_std_workspace_alloc-1500b38c00a00cc0.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libunwind-57741de0baeb7c8c.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libcfg_if-2354a5a675debdbb.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/liblibc-1432fbf85665684e.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/liballoc-a43ae4ad09488939.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/librustc_std_workspace_core-a8a859a864856684.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libcore-908209eee60fb642.rlib" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib/libcompiler_builtins-1d19a8c26aaad45f.rlib" "-lz" "-lc++" "-lSystem" "-lc" "-lm" "-L" "/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/lib" "-o" "/Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build/aarch64-apple-darwin/release/deps/texture_test-09a0e65109fff95d" "-Wl,-dead_strip"
  = note: Undefined symbols for architecture arm64:
            "_cxxbridge1$read_image", referenced from:
                gfx::imageio_cxx::ffi::read_image::h5122f98c3963e4d5 in libgfx.rlib(gfx.gfx.58b6ab36-cgu.5.rcgu.o)
          ld: symbol(s) not found for architecture arm64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)


error: could not compile `gfx` due to previous error
make[2]: *** [CMakeFiles/_cargo-build_texture_test] Error 101
make[1]: *** [CMakeFiles/_cargo-build_texture_test.dir/all] Error 2
make: *** [all] Error 2

Although, looking at this it looks a little weird, I don't really see why the C++ compiler is invoked here

@jschwe
Copy link
Collaborator

jschwe commented Jan 5, 2023

In this case you might be missing a corrosion_link_libraries(texture_test <cxx_lib_containing_read_image_definition>) to tell the linker to include the C++ library containing read_image into the executable.

Although, looking at this it looks a little weird, I don't really see why the C++ compiler is invoked here

It is quite common that the linker is not invoked directly, but via the compiler. Corrosion by default sets the linker to be the C++ compiler (if C++ is compiled) to ensure that the default C++ libraries are linked. On the most recent master there is a new option NO_LINKER_OVERRIDE to corrosion_import_crate() which prevents this, but then you would have to manually specify all default C++ libraries (via corrosion_link_libraries().

@jschwe
Copy link
Collaborator

jschwe commented Jan 5, 2023

Ah, I just realized that this is probably where the circular dependency issue comes up. Could you checkout the bridge branch (PR #293) and see if the circular dependency issue still exists (when using corrosion_link_libraries?
I'm not super familiar with cxx, so it could be that I'm also overlooking something.

@Xaldew
Copy link
Author

Xaldew commented Jan 5, 2023

Actually, it seems like limiting the dependencies to just the targets that need it seems to work, although one could argue that it is a bit of a workaround. That said, the build went a lot further this time, but fails when it tries to find the cxxbridge generated headers:

[  1%] Building CXX object CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o
/Library/Developer/CommandLineTools/usr/bin/c++ -DHAVE_LIBPNG=1 -DHAVE_OPENEXR=315 -DHAVE_ZLIB=1 -I/opt/homebrew/Cellar/openexr/3.1.5/include -I/opt/homebrew/Cellar/openexr/3.1.5/include/OpenEXR -I/opt/homebrew/Cellar/imath/3.1.6/include -I/opt/homebrew/Cellar/imath/3.1.6/include/Imath -I/opt/homebrew/Cellar/libpng/1.6.39/include/libpng16 -I/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/include -Wall -Wpedantic -Wno-deprecated -Werror -pthread -Wno-unknown-pragmas -Wno-unused-private-field -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk -std=gnu++17 -MD -MT CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o -MF CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o.d -o CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o -c /Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:1:10: fatal error: 'gfx/external/imageio-bridge.h' file not found
#include "gfx/external/imageio-bridge.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o] Error 1
make[1]: *** [CMakeFiles/imageio-cxx.dir/all] Error 2
make: *** [all] Error 2

Which is interesting, and probably easy to fix: Does corrosion place these headers somewhere specific?

Ah, I just realized that this is probably where the circular dependency issue comes up. Could you checkout the bridge branch (PR #293) and see if the circular dependency issue still exists (when using corrosion_link_libraries? I'm not super familiar with cxx, so it could be that I'm also overlooking something.

Sure, I'll give that a branch a spin tomorrow when I got some more time!

@jschwe
Copy link
Collaborator

jschwe commented Jan 6, 2023

Yes, the header is placed under ${CMAKE_CURRENT_BINARY_DIR}/corrosion_generated/cxxbridge/${cxx_target}/include and this path is added to the include directories of cxx_target (imageio-cxx in your case).

@Xaldew
Copy link
Author

Xaldew commented Jan 6, 2023

Seems like using the line corrosion_link_libraries(gfx imageio-cxx) still gives a circular dependency even on the bridge branch, example output below:

CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
  "_cargo-build_gfx" of type UTILITY
    depends on "imageio-cxx" (strong)
  "cargo-build_gfx" of type UTILITY
    depends on "_cargo-build_gfx" (strong)
  "imageio-cxx" of type STATIC_LIBRARY
    depends on "cargo-build_gfx" (strong)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies are allowed only among static libraries.
CMake Generate step failed.  Build files cannot be regenerated correctly.

Yes, the header is placed under ${CMAKE_CURRENT_BINARY_DIR}/corrosion_generated/cxxbridge/${cxx_target}/include and this path is added to the include directories of cxx_target (imageio-cxx in your case).

Hmm, odd. My paths are slightly different:

gustaf@ip81-1 build % find corrosion_generated/cxxbridge/imageio-cxx/include
corrosion_generated/cxxbridge/imageio-cxx/include
corrosion_generated/cxxbridge/imageio-cxx/include/imageio-cxx
corrosion_generated/cxxbridge/imageio-cxx/include/imageio-cxx/imageio_cxx.h

There seems to be a second ${cxx_target} after the include. Is that intentional?

After hacking at it awhile, I the Rust side of the bridge to this:

#[cxx::bridge]
pub mod ffi
{
    #[derive(Debug)]
    struct Rgba
    {
        r: f32,
        g: f32,
        b: f32,
        a: f32,
    }

    #[derive(Debug)]
    struct RsImage
    {
        width: usize,
        height: usize,
        raster: Vec<Rgba>,
    }

    unsafe extern "C++"
    {
        include!("imageio-bridge.h");
        fn read_image(path: &str) -> Result<RsImage>;
        fn write_image(path: &str, image: &RsImage) -> Result<()>;
    }
}

And forced the include directory corrosion_generated/cxxbridge/imageio-cxx to be available in the cmake-file:

  corrosion_link_libraries(texture_test imageio-cxx)
  target_include_directories(imageio-cxx PUBLIC ${PROJECT_SOURCE_DIR}/ext/gfx-rs/external/)

This got me a tiny bit further. Does corrosion keep the cxxbridge header utilities somewhere as well? Since now I'm missing these headers:

[  2%] Building CXX object CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o
/Library/Developer/CommandLineTools/usr/bin/c++ -DHAVE_LIBPNG=1 -DHAVE_OPENEXR=315 -DHAVE_ZLIB=1 -I/opt/homebrew/Cellar/openexr/3.1.5/include -I/opt/homebrew/Cellar/openexr/3.1.5/include/OpenEXR -I/opt/homebrew/Cellar/imath/3.1.6/include -I/opt/homebrew/Cellar/imath/3.1.6/include/Imath -I/opt/homebrew/Cellar/libpng/1.6.39/include/libpng16 -I/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/include -I/Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs/external -Wall -Wpedantic -Wno-deprecated -Werror -pthread -Wno-unknown-pragmas -Wno-unused-private-field -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk -std=gnu++17 -MD -MT CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o -MF CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o.d -o CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o -c /Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp
In file included from /Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:1:
/Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs/external/imageio-bridge.h:4:10: fatal error: 'rust/cxx.h' file not found
#include "rust/cxx.h"
         ^~~~~~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o] Error 1
make[1]: *** [CMakeFiles/imageio-cxx.dir/all] Error 2
make: *** [all] Error 2

Which I belive are the headers to the helper structs that cxxbridge uses

@jschwe
Copy link
Collaborator

jschwe commented Jan 6, 2023

Seems like using the line corrosion_link_libraries(gfx imageio-cxx) still gives a circular dependency even on the bridge branch, example output below:

In this case you might be missing a corrosion_link_libraries(texture_test <cxx_lib_containing_read_image_definition>)

Did this one not work? I think you would want to link the imageio-cxx crate into your final executable, not an intermediate library.

There seems to be a second ${cxx_target} after the include. Is that intentional?

Yes, that seems to be intentional. I don't remember why though - I'd have to look into that. Probably simply to add a namespace to your includes, so you do `#include "cxx_target/blah.h" to make conflicts less likely.

This got me a tiny bit further. Does corrosion keep the cxxbridge header utilities somewhere as well? Since now I'm missing these headers:

AFAIK this extra utliity header does not exist when using the commandline cxxbridge tool. Instead the definitions will be inside your header in the cxx_target directory.

@jschwe
Copy link
Collaborator

jschwe commented Jan 7, 2023

I've added a simple test based on what you wrote, and it seems to be compiling now.
You can have a look here: test/cxxbridge/cxxbridge_rust2cpp/CMakeLists.txt

One thing that I haven't figured out yet is how to get a non-absolute path working with the include!() macro in the cxx bridge, I'll investigate that if I have some more time.

@Xaldew
Copy link
Author

Xaldew commented Jan 9, 2023

Did this one not work? I think you would want to link the imageio-cxx crate into your final executable, not an intermediate library.

Oh, I think I misunderstood your previous comment: I figured you wanted me to try to use the line corrosion_link_libraries(gfx imageio-cxx) on the bridges branch to see if that (still) caused the same circular dependency. Using corrosion_link_libraries(test_textures imageio-cxx) works on both the bridges and the master/main branch.

That said however, I must say that this feels like a bit of a band-aid rather than a proper fix. At least in my case, the imageio-cxx is technically a dependency on the library and not of the individual (test) binaries. Although, I have no idea if that is feasible to fix. Perhaps it's possible to decouple the gfx (Rust) library from the build process? (The _cargo-build_gfx target I guess?)

Yes, that seems to be intentional. I don't remember why though - I'd have to look into that. Probably simply to add a namespace to your includes, so you do `#include "cxx_target/blah.h" to make conflicts less likely.

Makes perfect sense to me, just wanted to be sure :)

AFAIK this extra utliity header does not exist when using the commandline cxxbridge tool. Instead the definitions will be inside your header in the cxx_target directory.

Ah, I see, I must have missed that note in the cxxbridge documentation, and indeed changing that takes me a bit further still, but now I get a bunch of pretty concerning looking errors:

[  1%] Building CXX object CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:710:23: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
const char *cxxbridge1$exception(const char *, ::std::size_t);
                      ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:740:32: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
::rust::repr::PtrLen cxxbridge1$read_image(::rust::Str path, ::RsImage *return$) noexcept {
                               ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:740:79: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
::rust::repr::PtrLen cxxbridge1$read_image(::rust::Str path, ::RsImage *return$) noexcept {
                                                                              ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:741:25: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
  ::RsImage (*read_image$)(::rust::Str) = ::read_image;
                        ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:742:29: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
  ::rust::repr::PtrLen throw$;
                            ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:745:20: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        new (return$) ::RsImage(read_image$(path));
                   ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:745:43: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        new (return$) ::RsImage(read_image$(path));
                                          ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:746:14: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.ptr = nullptr;
             ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:748:28: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
      [&](const char *catch$) noexcept {
                           ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:749:14: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.len = ::std::strlen(catch$);
             ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:749:41: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.len = ::std::strlen(catch$);
                                        ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:750:14: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.ptr = const_cast<char *>(::cxxbridge1$exception(catch$, throw$.len));
             ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:750:53: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.ptr = const_cast<char *>(::cxxbridge1$exception(catch$, throw$.len));
                                                    ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:750:69: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.ptr = const_cast<char *>(::cxxbridge1$exception(catch$, throw$.len));
                                                                    ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:750:77: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
        throw$.ptr = const_cast<char *>(::cxxbridge1$exception(catch$, throw$.len));
                                                                            ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:752:15: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
  return throw$;
              ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:755:32: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
::rust::repr::PtrLen cxxbridge1$write_image(::rust::Str path, const ::RsImage &image) noexcept {
                               ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:756:21: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
  void (*write_image$)(::rust::Str, const ::RsImage &) = ::write_image;
                    ^
/Users/gustaf/Documents/git/phd/ocl-spikes/build/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp:757:29: error: '$' in identifier [-Werror,-Wdollar-in-identifier-extension]
  ::rust::repr::PtrLen throw$;
                            ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make[2]: *** [CMakeFiles/imageio-cxx.dir/corrosion_generated/cxxbridge/imageio-cxx/src/imageio_cxx.cpp.o] Error 1
make[1]: *** [CMakeFiles/imageio-cxx.dir/all] Error 2
make: *** [all] Error 2

Have you seen any of these before?

One thing that I haven't figured out yet is how to get a non-absolute path working with the include!() macro in the cxx bridge, I'll investigate that if I have some more time.

That's interesting, I think I got that to work using the line:

  target_include_directories(imageio-cxx
    PUBLIC
    ${PROJECT_SOURCE_DIR}/ext/gfx-rs/external/
    ${PROJECT_BINARY_DIR}/corrosion_generated/cxxbridge/imageio-cxx/include)

(The include!() header file is located in ${PROJECT_SOURCE_DIR}/ext/gfx-rs/external/).

I've added a simple test based on what you wrote, and it seems to be compiling now. You can have a look here: test/cxxbridge/cxxbridge_rust2cpp/CMakeLists.txt

This looks good to me at least, perhaps the include statement I mentioned above should be included? Or perhaps that should be done as a part of the corrosion_add_cxxbridge() cmake function?

On a separate note, I also noticed that the imageio-cxx target is not properly regenerated when imageio_cxx.rs is edited. Accordingly, I need to remove the old config (rm -rf *) and regenerate it anew for changes to come into effect. Is there perhaps some dependency missing here that prevents a proper update?

@Xaldew
Copy link
Author

Xaldew commented Jan 12, 2023

As I was unable to progress with the previous approach, I tried a different tack
that I had been thinking of for a while: Using features and environment
variables to propagate any necessary information. This way I was actually able
to get something running.

First, on the CMake side, I only import the crate and set the environment
variables as necessary:

# Rust <-> C++ integration.
find_package(corrosion)
if (corrosion_FOUND)
  set(CORROSION_VERBOSE_OUTPUT ON)
  corrosion_import_crate(
    MANIFEST_PATH ${PROJECT_SOURCE_DIR}/ext/gfx-rs/Cargo.toml
    FEATURES corrosion)

  # corrosion_link_libraries(gfx ccl_common)

  # Convert semicolons to colons to prevent CMake from converting it into lists.
  string(REPLACE ";" ":" link_libs "ccl_common;${OpenEXR_LIBRARIES}")
  string(REPLACE ";" ":" link_search "${PROJECT_BINARY_DIR}/src;${OpenEXR_LIBRARY_DIRS}")
  corrosion_set_env_vars(gfx
    CARGO_FEATURE_CORROSION_INCLUDES=${PROJECT_SOURCE_DIR}/src
    CARGO_FEATURE_CORROSION_LINK_LIBS=${link_libs}
    CARGO_FEATURE_CORROSION_LINK_SEARCH=${link_search})
endif()

Annoyingly, I also need to remove all semicolons or translate them to something
else (such as colons in this case) to be able to set multiple libraries in these
variables. Otherwise, it seems like CMake expands the "lists" to strings using
spaces, which causes all manner of havoc later on. Maybe there's a better way to do this?

Further, in the Rust build script I then extract this information from
the same environment variables:

    if let Ok(_s) = std::env::var("CARGO_FEATURE_CORROSION")
    {
        let includes = std::env::var("CARGO_FEATURE_CORROSION_INCLUDES").unwrap_or("".to_string());
        let link_libs = std::env::var("CARGO_FEATURE_CORROSION_LINK_LIBS").unwrap_or("".to_string());
        let link_search = std::env::var("CARGO_FEATURE_CORROSION_LINK_SEARCH").unwrap_or("".to_string());

        let pat = |sym|{sym == ';' || sym == ':' || sym == ' '};
        for lib in link_libs.split_terminator(pat)
        {
            println!("cargo:rustc-link-lib={}", lib);
        }
        for path in link_search.split_terminator(pat)
        {
            println!("cargo:rustc-link-search={}", path);
        }
        let ccl_includes: Vec<_> = includes.split_terminator(pat).collect();

        // Attempt to build the C++-bridge.
        cxx_build::bridge("src/imageio_cxx.rs")
            .file("external/imageio-bridge.cpp")
            .includes(&ccl_includes)
            .flag_if_supported("-std=c++17")
            .flag("-Wno-deprecated")
            .compile("libimageio-bridge");
        println!("cargo:rerun-if-changed=external/imageio-bridge.h");
        println!("cargo:rerun-if-changed=external/imageio-bridge.cpp");
    }

However, while this works, I'm a bit concerned with the scalability of this
solution since I have to manually specify all dependencies. I was hoping that I
would only need to specify ccl_common as a requirement using
corrosion_link_libraries(gfx ccl_common), and that works for symbols exclusive
to the ccl_common library, but not for any of the dependencies. I.e., in the
above example ccl_common depends on OpenEXR, but the libraries and search
paths are not included for it. Is that intentional or would that be considered a
bug?

@jschwe
Copy link
Collaborator

jschwe commented Jan 12, 2023

Otherwise, it seems like CMake expands the "lists" to strings using spaces, which causes all manner of havoc later on.

Well, in CMake a list is just a semicolon delimited string, so you have to pay attention and quote the list variable basically each time you use it to prevent expansion. Possibly quoting the arguments to corrosion_set_env_vars would already be sufficient, but it's also possible that the COMMAND_EXPAND_LISTS argument of the custom command which calls cargo build would anyway expand the list without a way to prevent it. I don't quite remember the interaction here.

While working on #293 I did realize that the add_cxxbridge function currently only works properly when linking Rust into C/C++, but not (yet) the other way around in all cases. I think for your particular case it should not be that hard to fix though. Could you have a look at #293, especially at the newly added testcase and improve the C++ / rust code to something that corresponds somewhat to your failing project? I could then take another look with a bit more realistic of a test project.

@Xaldew
Copy link
Author

Xaldew commented Jan 13, 2023

Well, in CMake a list is just a semicolon delimited string, so you have to pay attention and quote the list variable basically each time you use it to prevent expansion. Possibly quoting the arguments to corrosion_set_env_vars would already be sufficient, but it's also possible that the COMMAND_EXPAND_LISTS argument of the custom command which calls cargo build would anyway expand the list without a way to prevent it. I don't quite remember the interaction here.

Hmm, that's probably another bug then, here's the behavior between quoting the variables vs not:

[ 22%] Built target cargo-prebuild_gfx
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/_cargo-build_gfx.dir/build.make CMakeFiles/_cargo-build_gfx.dir/depend
cd /Users/gustaf/Documents/git/phd/ocl-spikes/build && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E cmake_depends "Unix Makefiles" /Users/gustaf/Documents/git/phd/ocl-spikes /Users/gustaf/Documents/git/phd/ocl-spikes /Users/gustaf/Documents/git/phd/ocl-spikes/build /Users/gustaf/Documents/git/phd/ocl-spikes/build /Users/gustaf/Documents/git/phd/ocl-spikes/build/CMakeFiles/_cargo-build_gfx.dir/DependInfo.cmake --color=
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/_cargo-build_gfx.dir/build.make CMakeFiles/_cargo-build_gfx.dir/build
cd /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E env CARGO_FEATURE_CORROSION_INCLUDES=/Users/gustaf/Documents/git/phd/ocl-spikes/src CARGO_FEATURE_CORROSION_LINK_LIBS=ccl_common OpenEXR-3_1 OpenEXRUtil-3_1 OpenEXRCore-3_1 Iex-3_1 IlmThread-3_1 Imath-3_1 CARGO_FEATURE_CORROSION_LINK_SEARCH=/Users/gustaf/Documents/git/phd/ocl-spikes/build/src /opt/homebrew/Cellar/openexr/3.1.5/lib /opt/homebrew/Cellar/imath/3.1.6/lib CARGO_TARGET_AARCH64_APPLE_DARWIN_LINKER=/Library/Developer/CommandLineTools/usr/bin/c++ CC_aarch64-apple-darwin=/Library/Developer/CommandLineTools/usr/bin/cc CXX_aarch64-apple-darwin=/Library/Developer/CommandLineTools/usr/bin/c++ SDKROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk LIBRARY_PATH=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib CORROSION_BUILD_DIR=/Users/gustaf/Documents/git/phd/ocl-spikes/build CARGO_BUILD_RUSTC=/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/rustc /Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/cargo rustc --lib --target=aarch64-apple-darwin --features corrosion --package gfx --manifest-path /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs/Cargo.toml --target-dir /Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build -- -Cdefault-linker-libraries=yes -L/Users/gustaf/Documents/git/phd/ocl-spikes/build/src -lccl_common
No such file or directory
make[2]: *** [CMakeFiles/_cargo-build_gfx] Error 1
make[1]: *** [CMakeFiles/_cargo-build_gfx.dir/all] Error 2
make: *** [all] Error 2

Versus:

[ 22%] Built target cargo-prebuild_gfx
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/_cargo-build_gfx.dir/build.make CMakeFiles/_cargo-build_gfx.dir/depend
cd /Users/gustaf/Documents/git/phd/ocl-spikes/build && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E cmake_depends "Unix Makefiles" /Users/gustaf/Documents/git/phd/ocl-spikes /Users/gustaf/Documents/git/phd/ocl-spikes /Users/gustaf/Documents/git/phd/ocl-spikes/build /Users/gustaf/Documents/git/phd/ocl-spikes/build /Users/gustaf/Documents/git/phd/ocl-spikes/build/CMakeFiles/_cargo-build_gfx.dir/DependInfo.cmake --color=
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/_cargo-build_gfx.dir/build.make CMakeFiles/_cargo-build_gfx.dir/build
cd /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E env CARGO_FEATURE_CORROSION_INCLUDES=\"/Users/gustaf/Documents/git/phd/ocl-spikes/src\" CARGO_FEATURE_CORROSION_LINK_LIBS=\"ccl_common OpenEXR-3_1 OpenEXRUtil-3_1 OpenEXRCore-3_1 Iex-3_1 IlmThread-3_1 Imath-3_1\" CARGO_FEATURE_CORROSION_LINK_SEARCH=\"/Users/gustaf/Documents/git/phd/ocl-spikes/build/src /opt/homebrew/Cellar/openexr/3.1.5/lib /opt/homebrew/Cellar/imath/3.1.6/lib\" CARGO_TARGET_AARCH64_APPLE_DARWIN_LINKER=/Library/Developer/CommandLineTools/usr/bin/c++ CC_aarch64-apple-darwin=/Library/Developer/CommandLineTools/usr/bin/cc CXX_aarch64-apple-darwin=/Library/Developer/CommandLineTools/usr/bin/c++ SDKROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk LIBRARY_PATH=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib CORROSION_BUILD_DIR=/Users/gustaf/Documents/git/phd/ocl-spikes/build CARGO_BUILD_RUSTC=/Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/rustc /Users/gustaf/.rustup/toolchains/nightly-aarch64-apple-darwin/bin/cargo rustc --lib --target=aarch64-apple-darwin --features corrosion --package gfx --manifest-path /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs/Cargo.toml --target-dir /Users/gustaf/Documents/git/phd/ocl-spikes/build/./cargo/build -- -Cdefault-linker-libraries=yes -L/Users/gustaf/Documents/git/phd/ocl-spikes/build/src -lccl_common
No such file or directory
make[2]: *** [CMakeFiles/_cargo-build_gfx] Error 1
make[1]: *** [CMakeFiles/_cargo-build_gfx.dir/all] Error 2
make: *** [all] Error 2

It seems like the quotes become "part" of the first and last element somehow, not sure how that works.

Here's the CMake changes from above, just in case:

  # Convert semicolons to colons to prevent CMake from converting it into lists.
  string(REPLACE ";" ";" link_libs "ccl_common;${OpenEXR_LIBRARIES}")
  string(REPLACE ";" ";" link_search "${PROJECT_BINARY_DIR}/src;${OpenEXR_LIBRARY_DIRS}")
  corrosion_set_env_vars(gfx
    CARGO_FEATURE_CORROSION_INCLUDES="${PROJECT_SOURCE_DIR}/src"
    CARGO_FEATURE_CORROSION_LINK_LIBS="${link_libs}"
    CARGO_FEATURE_CORROSION_LINK_SEARCH="${link_search}")

While working on #293 I did realize that the add_cxxbridge function currently only works properly when linking Rust into C/C++, but not (yet) the other way around in all cases. I think for your particular case it should not be that hard to fix though. Could you have a look at #293, especially at the newly added testcase and improve the C++ / rust code to something that corresponds somewhat to your failing project? I could then take another look with a bit more realistic of a test project.

Sure, I'll try to squeeze that in soon-ish

@jschwe
Copy link
Collaborator

jschwe commented Jan 13, 2023

It seems like the quotes become "part" of the first and last element somehow, not sure how that works.

Yeah, you need to quote the whole thing, otherwise CMake will keep the quotes.

corrosion_set_env_vars(gfx
"CARGO_FEATURE_CORROSION_INCLUDES=${PROJECT_SOURCE_DIR}/src"
"CARGO_FEATURE_CORROSION_LINK_LIBS=${link_libs}"
"CARGO_FEATURE_CORROSION_LINK_SEARCH=${link_search}")

@Xaldew
Copy link
Author

Xaldew commented Jan 16, 2023

Hmm, strange, that didn't actually change anything:

cd /Users/gustaf/Documents/git/phd/ocl-spikes/ext/gfx-rs && /opt/homebrew/Cellar/cmake/3.25.1/bin/cmake -E env \
CARGO_FEATURE_CORROSION_INCLUDES=/Users/gustaf/Documents/git/phd/ocl-spikes/src \
CARGO_FEATURE_CORROSION_LINK_LIBS=ccl_common OpenEXR-3_1 OpenEXRUtil-3_1 OpenEXRCore-3_1 Iex-3_1 IlmThread-3_1 Imath-3_1 \
CARGO_FEATURE_CORROSION_LINK_SEARCH=/Users/gustaf/Documents/git/phd/ocl-spikes/build/src /opt/homebrew/Cellar/openexr/3.1.5/lib /opt/homebrew/Cellar/imath/3.1.6/lib

Just for completeness I also tried to escape-quote the variable values:

corrosion_set_env_vars(gfx
  "CARGO_FEATURE_CORROSION_INCLUDES=\"${PROJECT_SOURCE_DIR}/src\""
  "CARGO_FEATURE_CORROSION_LINK_LIBS=\"${link_libs}\""
  "CARGO_FEATURE_CORROSION_LINK_SEARCH=\"${link_search}\"")

and to use single quote the internal ones:

corrosion_set_env_vars(gfx
  "CARGO_FEATURE_CORROSION_INCLUDES='${PROJECT_SOURCE_DIR}/src'"
  "CARGO_FEATURE_CORROSION_LINK_LIBS='${link_libs}'"
  "CARGO_FEATURE_CORROSION_LINK_SEARCH='${link_search}'")

But this just gave the same result as before (inserting quotes in the first/last
element).

Either way, this is clearly a separate issue. Perhaps this should be lifted out
to a new ticket? Personally, my opinion on this is that list variables should
probably be passed in verbatim (somehow), and that the documentation for this
call should include a descriptive example, as this could very easily be a source
of confusion.

@jschwe
Copy link
Collaborator

jschwe commented Jan 20, 2023

Hmm, strange, that didn't actually change anything:

In that case this is because of the COMMAND_EXPANDS_LIST setting also expanding this list. I'm afraid there is nothing that can really be done here though, since corrosion needs COMMAND_EXPANDS_LIST .

Perhaps this should be lifted out to a new ticket?

This can't be fixed - The user replacing the semicolon with another character is really the only solution then, AFAIU.

Anyway, I did get something close to your example working, but since there are circular dependencies, this doesn't work with every linker. You may want to consider if you really need a circular dependency between the rust and the C++ libraries. Otherwise, linking with lld does seem to work.

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

No branches or pull requests

2 participants