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.

Sign up for GitHub

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

Jump to bottom

WIP: examples: create a multi project to test multiple crates #598

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

WIP: examples: create a multi project to test multiple crates #598

wants to merge 12 commits into from

Conversation

ahayzen-kdab
Copy link
Collaborator

No description provided.

@Be-ing
Copy link
Contributor

Be-ing commented Jun 28, 2023

Blocked on rust-lang/rust#108081

This was referenced Jun 28, 2023
Open
Merged
Open
@Be-ing
Copy link
Contributor

Be-ing commented Jul 3, 2023

Upstream PR to make this work: rust-lang/rust#113301

@Be-ing Be-ing mentioned this pull request Aug 15, 2023
Merged
@ahayzen-kdab ahayzen-kdab force-pushed the 592-multi-crate-project branch 2 times, most recently from a402176 to 5979009 Compare August 16, 2023 08:11
@Be-ing
Copy link
Contributor

Be-ing commented Sep 29, 2023

The upstream PR has been merged and will be released in stable Rust 1.74, which is scheduled for 16 November 2023. I suggest to leave this as a draft until then.

@ahayzen-kdab
Copy link
Collaborator Author

@Be-ing Awesome news, thanks! :-)

@Be-ing
Copy link
Contributor

Be-ing commented Nov 16, 2023

Rust 1.74 has been released so this now builds with stable Rust! 🥳

@ahayzen-kdab
Copy link
Collaborator Author

@Be-ing sweet! Great work on getting that stabilised into Rust 💪 We look at getting this landed and the reshuffle internally in the next cycle I think as 0.6 is imminent with a deadline we need to reach.

@ahayzen-kdab
Copy link
Collaborator Author

Seems to be getting a linking issue with double resources, need to investigate. Wonder if it's the QML module changes.

@Be-ing Be-ing mentioned this pull request Nov 25, 2023
Merged
@Be-ing
Copy link
Contributor

Be-ing commented Nov 25, 2023

Seems to be getting a linking issue with double resources, need to investigate. Wonder if it's the QML module changes.

Fixed in #755

@ahayzen-kdab
Copy link
Collaborator Author

Locally this builds now, but it fails to run as it can't find the sub QML QObjects, needs some further investigation or maybe a clean build.

@Be-ing
Copy link
Contributor

Be-ing commented Nov 27, 2023

I can reproduce that. nm -C build/examples/meta_project/example_meta_project|grep cxx_qt_demo_sub1 shows that the symbols for registering the QML type do end up in the executable... so why aren't they working? 🤔

@Be-ing
Copy link
Contributor

Be-ing commented Nov 29, 2023

I opened a PR for your fork to get this working: ahayzen-kdab#4 The errors were simply from missing #[qml_element] attributes :)

@ahayzen-kdab
Copy link
Collaborator Author

I opened a PR for your fork to get this working: ahayzen-kdab#4 The errors were simply from missing #[qml_element] attributes :)

Aha! great spot thanks!

@ahayzen-kdab
Copy link
Collaborator Author

Seems macOS works, Windows has had a wobble with file locking / mapped stuff, Linux explodes using ld.gold.

Is this one of those, only lld works situations ?

@Be-ing
Copy link
Contributor

Be-ing commented Nov 29, 2023 •
edited
Loading

Seems we're continuing the CXX-Qt tradition of stumbling over the weirdest edge cases of toolchains 😅

Somehow Cargo does specify linking the same qt-static-initializers library twice with --whole-archive, so it makes sense why gold fails... but why does Cargo do that? And only does that for cargo test?? Perhaps a Cargo bug?

As for the Windows Qt5 build error:

 error: failed to write file '\\?\C:\cxx-qt\build\cargo\build\x86_64-pc-windows-msvc\release\deps\libsub1-ef2617c41816f8ec.rlib': The requested operation cannot be performed on a file with a user-mapped section open. (os error 1224)

I don't know. Another cargo/rustc bug? A Windows bug? I don't see this error code anywhere in any rust-lang GitHub repositories.

A quick hack around both of these problems would be replacing the --all-targets CLI option to cargo test in the root CMakeLists.txt with --workspace --exclude qml-meta-project. The qml-meta-project crate doesn't even have Cargo tests, so there's not much point building it in that context.

@ahayzen-kdab
Copy link
Collaborator Author

Seems we're continuing the CXX-Qt tradition of stumbling over the weirdest edge cases of toolchains 😅

😅

Yeah, a switch around to ensure it builds (and maybe runs) would be useful, I'll probably look into this next month as I'm tight on time.

@Be-ing
Copy link
Contributor

Be-ing commented Nov 29, 2023

I don't think it's worth deep diving into the root causes of these errors right now considering that qml-meta-project doesn't even have Cargo tests. So I think switching to --workspace --exclude qml-meta-project would be sufficient for finally merging this PR.

@Be-ing Be-ing mentioned this pull request Nov 29, 2023
Draft
@Be-ing
Copy link
Contributor

Be-ing commented Dec 1, 2023

Hopefully this will be enough to merge finally: ahayzen-kdab#5

@LeonMatthesKDAB LeonMatthesKDAB mentioned this pull request Dec 15, 2023
Open
ahayzen-kdab and others added 8 commits January 15, 2024 15:00
@ahayzen-kdab
examples: create a multi project to test multiple crates
56b5e37
@ahayzen-kdab
qt-build-utils: only run qmlcachegen when there are QML files
087dc04 
Otherwise we have a build failure as qmlcache_loader.cpp file
does not exist.
@Be-ing @ahayzen-kdab
meta_project: add missing #[qml_element] attributes
4d66511
@Be-ing @ahayzen-kdab
meta_project: update build system to new practices
f94f3ad
@Be-ing @ahayzen-kdab
meta_project: don't specify sub1/sub2 as rlibs in Cargo.toml
235c2ad 
rlibs are the default for library crates; no need to specify
that explicitly.
@Be-ing @ahayzen-kdab
meta_project: don't double invoke increment methods
88328c6 
The `increment` methods of the subobjects are invoked
from QML, so don't also invoke them from Rust.

This also makes the behavior of the application clearer:
Main counts by 1, Sub1 counts by 2, Sub2 counts by 3.
@Be-ing @ahayzen-kdab
meta_project: specify and document Rust 1.74 MSRV
8ca8bb6 
This isn't required for the libraries published to crates.io,
so this isn't specified in the workspace Cargo.toml.
@Be-ing @ahayzen-kdab
meta_project: use extern crate rather than pub use
dead1b1 
No need to change the privacy of these; just need to have the
symbols referenced within the staticlib crate.
@ahayzen-kdab
Copy link
Collaborator Author

Split only running cachegen when there are QML files in #811

@ahayzen-kdab
Copy link
Collaborator Author

Wonder if it's the Rust side that needs to not be parallel rather than the C++/CMake side, but still shouldn't happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@Be-ing Be-ing Be-ing left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ahayzen-kdab @Be-ing @petrochenkov