-
Notifications
You must be signed in to change notification settings - Fork 80
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
Multi-Crate project - second try #1178
base: main
Are you sure you want to change the base?
Multi-Crate project - second try #1178
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1178 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 73 73
Lines 12426 12426
=========================================
Hits 12426 12426 ☔ View full report in Codecov by Sentry. |
db794ad
to
14e1d29
Compare
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.
This isn't required for the libraries published to crates.io, so this isn't specified in the workspace Cargo.toml.
No need to change the privacy of these; just need to have the symbols referenced within the staticlib crate.
Previously, downstream QML modules weren't exported. This should now be fixed, as QML modules are always exported.
qml_multi_crates is the grand finale in our quest for CMake&Cargo compatibility. It is a project that can be built as a Cargo lib and binary, as well as imported into CMake with a C++ binary and uses multiple Cargo crates internally. It does rely on the init_crate! and init_qml_module! macros, but otherwise works quite seamless.
14e1d29
to
b8b5603
Compare
// Otherwise linking will fail! | ||
#[test] | ||
fn init_dependencies() { | ||
cxx_qt::init_crate!(qml_multi_crates); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the init crate also init the QML modules from it ? As where does com.kdab.cxx_qt.demo.sub1
get initialised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(assume from one of these as things are reexported?)
// in the staticlib (if you use Rust symbols from these | ||
// crates in this crate, you can skip these `extern crate` statements). | ||
extern crate sub1; | ||
extern crate sub2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if we should have a further sub3
crate that uses types from sub1 and sub2 to check that is possible. Or is that not possible ? 🤔 Can be in a separate later branch though...
@@ -27,6 +27,7 @@ SPDX-License-Identifier: MIT OR Apache-2.0 | |||
- [Shared types](./bridge/shared_types.md) | |||
- [Attributes](./bridge/attributes.md) | |||
- [Traits](./bridge/traits.md) | |||
- [Common Issues](./common-issues.md) | |||
- [For Contributors: CXX-Qt Internals](./internals/index.md) | |||
- [Build System](./internals/build-system.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does anything in the build system notes need updating ?
"examples/qml_multi_crates/rust/main", | ||
"examples/qml_multi_crates/rust/sub1", | ||
"examples/qml_multi_crates/rust/sub2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should you add a changelog entry stating that this is now possible? (multi crate import/qml modules etc)
@@ -18,6 +18,9 @@ members = [ | |||
"examples/qml_features/rust", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you reword the WIP commit in the chain?
A combination of #598 and #1169.
Closes #598
Closes #148
Closes #1135