Skip to content

Commit

Permalink
Use method calls to initialize dependencies
Browse files Browse the repository at this point in the history
This speeds up rebuilds, as we only need to link to upstream
dependencies initializers, not build their initializers every time.

It should also allow us to manually initialize certain libraries down
the line with something similar to Q_INIT_RESOURCE, etc.

Closes #1166
  • Loading branch information
LeonMatthesKDAB committed Feb 5, 2025
1 parent 90039d7 commit d526f5e
Show file tree
Hide file tree
Showing 12 changed files with 348 additions and 247 deletions.
27 changes: 24 additions & 3 deletions book/src/internals/build-system.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,34 @@ Qt code often contains initialization code that is called by a static variable t

However, when linking into a static library, and then linking into the main executable, the linker will discard everything from the library that isn't used by the main executable, including these static initializers, as they're never actually used and just exist to run their constructor code.

There are two ways to solve this:
There are multiple ways to solve this:

- Export an object file and link that to the main binary. Object files are always included completely
- Use the whole-archive linker flag which forces inclusion of every object within the static library.
- If we include the entire static lib generated by cargo, then we'll likely get duplicate symbols, as this really includes **everything** that your Rust code **may** need, even if you don't use it.
- This has caused some recent regressions with Rust 1.78+, where MSVC could no longer link CXX-Qt due to duplicate symbols
- The way to solve this is to only export the static initializers as a library and link that into CMake.
- Manually calling the static initializer code
- This is basically what Q_INIT_RESOURCE and Q_IMPORT_PLUGIN do
- They call the registration method directly, which circumvents the static initializers and forces the static initializers to be linked if they would otherwise be discarded.

At the moment we employ a mix of all methods.

First and foremost, we wrap all our initializers into functions with well-defined names (starting with `cxx_qt_init`) and C-compatible signatures.
This allows us to manually call the initializers from any point in the linker chain, which forces their inclusion.
These initializer functions call the initializer functions from their upstream dependencies so that the entire dependency tree is initialized.

However, we don't want to have to call the initializers manually in every resulting binary.
To solve this, we use static initializers that simply call the initializer function of the crate/Qml module, thereby initializing all dependencies.
As noted earlier, these static initializers are routinely optimized out by the linker.

For Cargo builds we prevent this by linking all initializers with +whole-archive which forces all of them to be included.
Experience has shown that this gives us the best compatibility overall, as linking object files to Cargo builds turned out to be quite finicky.
As the initializers contain very few symbols themselves, this should also rarely lead to issues with duplicate symbols.

In CMake we mirror Qts behavior, which is to build the static initializer as an `OBJECT` library.
The initializer functions themselves are still built into the Rust static library and the `OBJECT` library must therefore link to it.
This is taken care of by the `cxx_qt_import_crate`/`_import_qml_module` functions.

### Header files

Expand Down Expand Up @@ -87,7 +108,7 @@ However, we also want to provide some custom functions that wrap corrosion and s

Currently we provide two functions:

- cxxqt_import_crate
- cxx_qt_import_crate
- A wrapper over corrosion_import_crate that defines the `CXXQT_EXPORT_DIR`, imports the initializers object files, etc.
- cxxqt_import_qml_module
- cxx_qt_import_qml_module
- Import a given QML module by URI from the given SOURCE_CRATE and provide it as a target.
4 changes: 2 additions & 2 deletions crates/cxx-qt-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ cxx-gen.workspace = true
cxx-qt-gen.workspace = true
proc-macro2.workspace = true
quote.workspace = true
qt-build-utils.workspace = true
qt-build-utils = { workspace = true, features = ["serde"] }
codespan-reporting = "0.11"
version_check = "0.9"
serde = { version = "1.0", features = ["default", "derive"] }
serde.workspace = true
serde_json = "1.0"

[features]
Expand Down
29 changes: 2 additions & 27 deletions crates/cxx-qt-build/src/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use std::path::{Path, PathBuf};
/// When generating a library with cxx-qt-build, the library may need to export certain flags or headers.
/// These are all specified by this Interface struct, which should be passed to the [crate::CxxQtBuilder::library] function.
pub struct Interface {
pub(crate) initializers: Vec<PathBuf>,
// The name of the links keys, whose CXX-Qt dependencies to reexport
pub(crate) reexport_links: HashSet<String>,
pub(crate) exported_include_prefixes: Vec<String>,
Expand All @@ -28,7 +27,6 @@ pub struct Interface {
impl Default for Interface {
fn default() -> Self {
Self {
initializers: Vec::new(),
reexport_links: HashSet::new(),
exported_include_prefixes: vec![super::crate_name()],
exported_include_directories: Vec::new(),
Expand All @@ -37,21 +35,6 @@ impl Default for Interface {
}

impl Interface {
/// Add a C++ file path that will be exported as an initializer to downstream dependencies.
///
/// Initializer files will be built into object files, instead of linked into the static
/// library.
/// This way, the static variables and their constructors in this code will not be optimized
/// out by the linker.
pub fn initializer(mut self, path: impl AsRef<Path>) -> Self {
let path = PathBuf::from(path.as_ref());
let path = path
.canonicalize()
.expect("Failed to canonicalize path to initializer! Does the path exist?");
self.initializers.push(path);
self
}

/// Export all headers with the given prefix to downstream dependencies
///
/// Note: This will overwrite any previously specified header_prefixes, including the default
Expand Down Expand Up @@ -124,7 +107,7 @@ pub(crate) struct Manifest {
pub(crate) name: String,
pub(crate) link_name: String,
pub(crate) qt_modules: Vec<String>,
pub(crate) initializers: Vec<PathBuf>,
pub(crate) initializers: Vec<qt_build_utils::Initializer>,
pub(crate) exported_include_prefixes: Vec<String>,
}

Expand Down Expand Up @@ -173,18 +156,10 @@ impl Dependency {
}
}

pub(crate) fn initializer_paths(
interface: Option<&Interface>,
dependencies: &[Dependency],
) -> HashSet<PathBuf> {
pub(crate) fn initializers(dependencies: &[Dependency]) -> Vec<qt_build_utils::Initializer> {
dependencies
.iter()
.flat_map(|dep| dep.manifest.initializers.iter().cloned())
.chain(
interface
.iter()
.flat_map(|interface| interface.initializers.iter().cloned()),
)
.collect()
}

Expand Down
6 changes: 6 additions & 0 deletions crates/cxx-qt-build/src/dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ pub(crate) fn is_exporting() -> bool {
export().is_some()
}

pub(crate) fn initializers(key: &str) -> PathBuf {
let path = out().join("cxx-qt-build").join("initializers").join(key);
std::fs::create_dir_all(&path).expect("Failed to create initializers path!");
path
}

#[cfg(unix)]
pub(crate) fn symlink_or_copy_directory(
source: impl AsRef<Path>,
Expand Down
Loading

0 comments on commit d526f5e

Please sign in to comment.