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

MSVC CI doesn't compile with Rust 1.78.0 #958

Closed
LeonMatthesKDAB opened this issue May 21, 2024 · 4 comments
Closed

MSVC CI doesn't compile with Rust 1.78.0 #958

LeonMatthesKDAB opened this issue May 21, 2024 · 4 comments

Comments

@LeonMatthesKDAB
Copy link
Collaborator

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.

LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue May 21, 2024
This works around our CI issues on Windows, as they seem to have appeared
with the 1.78 update

See: KDAB#958
LeonMatthesKDAB added a commit that referenced this issue May 21, 2024
This works around our CI issues on Windows, as they seem to have appeared
with the 1.78 update

See: #958
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue May 23, 2024
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue May 23, 2024
LeonMatthesKDAB added a commit to LeonMatthesKDAB/cxx-qt that referenced this issue May 23, 2024
Previously, we linked the entire staticlib generated by the Rust
compiler into our CMake target.
This was part of what caused the recent issues in CI (see KDAB#958).
Cargo already only linked the plugin&resources library as whole-archive,
which is where where it is needed.
Now we simply export the initializers library into the CXXQT_EXPORT_DIR,
so we can only link the initializers with whole-archive.
This is a lot cleaner and should lead to fewer linker failures.

Closes KDAB#958
@Billyzou0741326
Copy link

Billyzou0741326 commented Jun 20, 2024

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
Copy link
Collaborator Author

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

We basically have 3 ways we tried solve this currently:

  1. WIP: cxx-qt-build: use init method rather than WHOLE_ARCHIVE for CMake #964 - generates a init method that needs to be called
  2. Only use whole-archive on the -initalizers library that contains QML plugins/resources #963 - Limits whole-archive to only the plugins and exports those to CMake
  3. CMake: Add custom CMake code to simplify build #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 🥳

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
Copy link

Billyzou0741326 commented Jun 21, 2024

#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
Copy link
Collaborator Author

This should be solved as of #978

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants