KDAB / cxx-qt

Safe interop between Rust and Qt
https://kdab.github.io/cxx-qt/book/
964 stars 66 forks source link

MSVC CI doesn't compile with Rust 1.78.0 #958

Closed LeonMatthesKDAB closed 1 week ago

LeonMatthesKDAB commented 1 month ago

With Rust 1.78.0, we seem to run into an issue with duplicate symbol definitions of __NULL_IMPORT_DESCRIPTOR.

See: https://github.com/KDAB/cxx-qt/actions/runs/9167401332/job/25204499375

This can be worked around by reverting to 1.77 for our CI (see #957 ), but we still need to fix this so that we remain compatible with upstream Rust.

Billyzou0741326 commented 2 weeks ago

Just want to mention that I also ran into this issue with rust 1.78 (now 1.79) + msvc.

My understanding is that after #964 goes live, the main changes to be made are:

  1. Simplifying target_link_libraries from "$<LINK_LIBRARY:WHOLE_ARCHIVE,${CRATE}-static>" down to ${CRATE}, and
  2. Explicitly invoke cxx_qt::init(); from c++ main entrypoint

Did I miss anything?

I guess in the meantime I can make do with using the mingw gnu toolchain, but would be nice to have msvc builds unblocked

LeonMatthesKDAB commented 2 weeks ago

Hi, we're very actively working to solve this issue at the moment.

We basically have 3 ways we tried solve this currently:

  1. 964 - generates a init method that needs to be called

  2. 963 - Limits whole-archive to only the plugins and exports those to CMake

  3. 978 - Uses .o files that are linked directly instead of passing everything through a staticlib.

Currently it looks like #978 is the way to go. We will need to find the best way to fetch our custom CMake code, but then we are also able to simplify the CMake code we require from our uses.

Basically you replace corrosion_import_crate with cxxqt_import_crate, and add cxxqt_import_qml_module for any QML modules, then link to either the crate directly or the qml module, if any. And then you can get rid of most of the current CMake WHOLE_ARCHIVE stuff and everything :partying_face:

We hope to land this initial fix within the coming days. There will be a few more changes coming down the line, which we hope will simplify the build process even further for our end users.

Billyzou0741326 commented 2 weeks ago

978 looks promising. Having all the cmake heavylifting configuration managed by will be a significant improvement on the build config.

I'll experiment with the changes over the weekends if it's stable enough for testing.

LeonMatthesKDAB commented 1 week ago

This should be solved as of #978