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

Add dynamic linking support for faster builds #26

Closed
wants to merge 5 commits into from
Closed

Add dynamic linking support for faster builds #26

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7cb7c9c
Add dynamic linking support for faster builds
DSchroer May 28, 2023
cf0fc6a
Remove extra file
DSchroer May 28, 2023
1900b2b
Remove unneeded 'static
DSchroer May 28, 2023
95bb105
Merge branch 'feat/dynamic-link' of github.com:DSchroer/opencascade-r…
DSchroer May 28, 2023
e594009
Fix missing library
DSchroer May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/opencascade-sys/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ exclude = [
"OCCT/tools/*",
]

[features]
dynamic = []

[dependencies]
cxx = "1"

Expand Down
97 changes: 59 additions & 38 deletions crates/opencascade-sys/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,29 @@
const LIBS: [&str; 18] = [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can avoid having to specify the array length:

Suggested change
const LIBS: [&str; 18] = [
const LIBS: &[&str] = &[

"TKMath",
"TKMath",
"TKernel",
"TKGeomBase",
"TKG2d",
"TKG3d",
"TKTopAlgo",
"TKGeomAlgo",
"TKGeomBase",
"TKBRep",
"TKPrim",
"TKSTL",
"TKMesh",
"TKShHealing",
"TKFillet",
"TKBool",
"TKBO",
"TKOffset",
];

fn main() {
let target = std::env::var("TARGET").expect("No TARGET environment variable defined");
let is_windows = target.to_lowercase().contains("windows");
let is_windows_gnu = target.to_lowercase().contains("windows-gnu");

let dst = cmake::Config::new("OCCT")
.define("BUILD_LIBRARY_TYPE", "Static")
.define("BUILD_MODULE_Draw", "FALSE")
.define("USE_OPENGL", "FALSE")
.define("USE_GLES2", "FALSE")
.define("USE_D3D", "FALSE")
.define("USE_VTK", "FALSE")
.define("USE_TCL", "FALSE")
.define("USE_XLIB", "FALSE")
.define("USE_FREETYPE", "FALSE")
.define("USE_FREEIMAGE", "FALSE")
.define("USE_OPENVR", "FALSE")
.define("USE_FFMPEG", "FALSE")
.define("INSTALL_DIR_LIB", "lib")
.define("INSTALL_DIR_INCLUDE", "include")
.build();

println!("cargo:rustc-link-search=native={}", dst.join("lib").display());
println!("cargo:rustc-link-lib=static=TKMath");
println!("cargo:rustc-link-lib=static=TKernel");
println!("cargo:rustc-link-lib=static=TKFeat");
println!("cargo:rustc-link-lib=static=TKGeomBase");
println!("cargo:rustc-link-lib=static=TKG2d");
println!("cargo:rustc-link-lib=static=TKG3d");
println!("cargo:rustc-link-lib=static=TKTopAlgo");
println!("cargo:rustc-link-lib=static=TKGeomAlgo");
println!("cargo:rustc-link-lib=static=TKGeomBase");
println!("cargo:rustc-link-lib=static=TKBRep");
println!("cargo:rustc-link-lib=static=TKPrim");
println!("cargo:rustc-link-lib=static=TKSTL");
println!("cargo:rustc-link-lib=static=TKMesh");
println!("cargo:rustc-link-lib=static=TKShHealing");
println!("cargo:rustc-link-lib=static=TKFillet");
println!("cargo:rustc-link-lib=static=TKBool");
println!("cargo:rustc-link-lib=static=TKBO");
println!("cargo:rustc-link-lib=static=TKOffset");
let is_dynamic = std::env::var("CARGO_FEATURE_DYNAMIC").is_ok();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use the cfg macro or a cfg attribute.

if cfg!(feature = "dynamic") {
    ...
}

or something like

#[cfg(feature = "dynamic")]
fn is_dynamic() {
    true    
}

#[cfg(not(feature = "dynamic"))]
fn is_dynamic() {
    false
}

In this case the cfg!() macro probably makes more sense.


if is_windows {
println!("cargo:rustc-link-lib=dylib=user32");
Expand All @@ -50,11 +35,24 @@ fn main() {
build.define("OCC_CONVERT_SIGNALS", "TRUE");
}

if !is_dynamic {
let opencascade_include = build_opencascade();
build.include(opencascade_include);

for lib in LIBS {
println!("cargo:rustc-link-lib=static={lib}");
}
} else {
for lib in LIBS {
println!("cargo:rustc-link-lib=dylib={lib}");
}
build.include("/usr/include/opencascade");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is very portable, for example if I install opencascade with homebrew on macos, it'll be in /opt/homebrew/include/opencascade.

pkg-config is pretty commonly used for this sort of task.

I have an example build.rs for another project which has a feature flag for dynamic vs. static linking, as well as usage of pkgconfig.

It'll increase the complexity of build.rs, but it makes the library more likely to build without errors when dynamically linking.

}

build
.cpp(true)
.flag_if_supported("-std=c++11")
.define("_USE_MATH_DEFINES", "TRUE")
.include(format!("{}", dst.join("include").display()))
.include("include")
.compile("wrapper");

Expand All @@ -63,3 +61,26 @@ fn main() {
println!("cargo:rerun-if-changed=src/lib.rs");
println!("cargo:rerun-if-changed=include/wrapper.hxx");
}

fn build_opencascade() -> String {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be PathBuf instead of String (see the comment below)

Suggested change
fn build_opencascade() -> String {
fn build_opencascade() -> PathBuf {

let dst = cmake::Config::new("OCCT")
.define("BUILD_LIBRARY_TYPE", "Static")
.define("BUILD_MODULE_Draw", "FALSE")
.define("USE_OPENGL", "FALSE")
.define("USE_GLES2", "FALSE")
.define("USE_D3D", "FALSE")
.define("USE_VTK", "FALSE")
.define("USE_TCL", "FALSE")
.define("USE_XLIB", "FALSE")
.define("USE_FREETYPE", "FALSE")
.define("USE_FREEIMAGE", "FALSE")
.define("USE_OPENVR", "FALSE")
.define("USE_FFMPEG", "FALSE")
.define("INSTALL_DIR_LIB", "lib")
.define("INSTALL_DIR_INCLUDE", "include")
.build();

println!("cargo:rustc-link-search=native={}", dst.join("lib").display());

format!("{}", dst.join("include").display())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this return value can just be

Suggested change
format!("{}", dst.join("include").display())
dst.join("include")

}