add dlopen Cargo feature using dlib to load fontconfig at runtime #12

Be-ing · 2022-02-14T22:30:58Z

Fixes #11

I modernized the code a bit along the way.

fontconfig/src/lib.rs

instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. The feature can be enabled by setting the RUST_FONTCONFIG_DLOPEN environment variable to avoid needing to propagate the Cargo feature all the way through downstream Cargo.toml's. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

Be-ing · 2022-02-16T20:18:35Z

I added build.rs's so setting the RUST_FONTCONFIG_DLOPEN environment variable enables the dlopen feature. This allows people building downstream applications to control whether it is linked or dlopen'ed without needing to pass the Cargo feature all the way down the stack of Cargo.toml's. Refer to discussion here.

wezm

Overall seems ok, a few things to fix up. I'd probably want sign off from the Slint folks that they will accept slint-ui/slint#956 so that we have a concrete use case before merging.

fontconfig/src/lib.rs

fontconfig-sys/Cargo.toml

wezm · 2022-02-16T23:40:50Z

.travis.yml

@@ -13,7 +13,7 @@ rust:
  - nightly
  - beta
  - stable
-  - 1.38.0
+  - 1.43.0


We're going to need additional CI builds to check that it works with and without the dlopen feature enabled.

Can we remove the beta and nightly CI jobs? If it works with stable, why wouldn't it work with beta and nightly?

The idea is that sometimes incompatibilities are introduced in newer versions and this might catch that. However, that requires a CI cron job, which hasn't been set up so may as well remove them and speed up the build.

wezm · 2022-02-16T23:42:52Z

fontconfig-sys/Cargo.toml

@@ -27,6 +27,11 @@ name = "fontconfig_sys"

 [dependencies]
 const-cstr = "0.3"
+dlib = "0.5.0"
+lazy_static = "1.4.0"


We probably want to allow using any lazy_static version in the >1.4 series so this version requirement can be 1.4. Having said that there's ongoing work (See rust-lang/rfcs#2788 and rust-lang/rust#74465) to integrate once_cell into Rust so I would prefer to use that instead.

https://users.rust-lang.org/t/psa-please-specify-precise-dependency-versions-in-cargo-toml/71277

fontconfig-sys/src/fontconfig.rs

Be-ing · 2022-02-17T20:53:19Z

I find it confusing how fontconfig-sys has a fontconfig module within it. It is easy to confuse with the fontconfig safe wrapper crate. Currently the imports in the fontconfig safe wrapper's lib.rs are:

use crate::fontconfig_sys::fontconfig as sys;
use crate::fontconfig_sys::ffi_dispatch;

#[cfg(feature = "dlopen")]
use sys::{LIB, LIB_RESULT};
#[cfg(not(feature = "dlopen"))]
use sys::*;

I'd like to change this to:

use crate::fontconfig_sys as sys;
use crate::fontconfig_sys::ffi_dispatch;

#[cfg(feature = "dlopen")]
use sys::{LIB, LIB_RESULT};
#[cfg(not(feature = "dlopen"))]
use sys::*;

by moving the contents of fontconfig-sys/src/fontconfig.rs into fontconfig-sys/src/lib.rs. What do you think about this?

Fixes yeslogic#11

It was confusing to have a fontconfig crate which depends on the fontconfig-sys crate which in turn has a module called fontconfig. This simplifies the paths so fontconfig-sys does not have any submodules; everything is in its lib.rs now. So import paths change from use fontconfig_sys::fontconfig::* to use fontconfig_sys::*

wezm · 2022-02-17T23:51:53Z

by moving the contents of fontconfig-sys/src/fontconfig.rs into fontconfig-sys/src/lib.rs. What do you think about this?

Seems fine, I think that structure was just inherited from the original project it was forked from.

Be-ing · 2022-02-18T00:22:05Z

I'd probably want sign off from the Slint folks that they will accept slint-ui/slint#956 so that we have a concrete use case before merging.

I have been discussing this with them and got the impression that they intend to merge slint-ui/slint#956 after this PR is merged. @tronical, @ogoffart, can one of you confirm this?

instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. The feature can be enabled by setting the RUST_FONTCONFIG_DLOPEN environment variable to avoid needing to propagate the Cargo feature all the way through downstream Cargo.toml's. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

tronical · 2022-02-18T09:04:57Z

I think switching Slint over to use yeslogic's fontconfig crate would be an improvement. I love you how folks are actively maintaining it and responding to feedback.

Be-ing · 2022-02-21T23:19:52Z

Thanks for reviewing! Could you publish a new release to crates.io?

wezm · 2022-02-22T02:09:12Z

Thanks for reviewing! Could you publish a new release to crates.io?

Yes, once I have #13 passing on CI

wezm · 2022-02-22T04:19:16Z

New crates have been published.

instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. The feature can be enabled by setting the RUST_FONTCONFIG_DLOPEN environment variable to avoid needing to propagate the Cargo feature all the way through downstream Cargo.toml's. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

switch to yeslogic-fontconfig-sys instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: ``` $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm ``` Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. The feature can be enabled by setting the RUST_FONTCONFIG_DLOPEN environment variable to avoid needing to propagate the Cargo feature all the way through downstream Cargo.toml's. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

switch to yeslogic-fontconfig-sys instead of servo-fontconfig servo-fontconfig statically links a vendored fontconfig library if fontconfig is not found by pkgconfig. Using a vendored fontconfig library instead of the system fontconfig library doesn't actually work well though: slint-ui/slint#88 Building a vendored copy of fontconfig is also problematic because fontconfig has a lot of C dependencies, which makes it difficult to cross compile the vendored copy of fontconfig: ``` $ pkg-config fontconfig --static --libs -lfontconfig -lfreetype -lz -lbz2 -lpng16 -lm -lm -lz -lharfbuzz -lm -lglib-2.0 -lm -lpcre -lsysprof-capture-4 -pthread -lgraphite2 -lbrotlidec -lbrotlicommon -lxml2 -lz -llzma -lm ``` Instead of using a vendored copy, with yeslogic/fontconfig-rs#12 yeslogic-fontconfig-sys will have a Cargo feature to dlopen fontconfig at runtime instead of linking it at build time. This is exposed in font-kit with the new source-fontconfig-dlopen feature, which is disabled by default. This feature makes it considerably easier to cross compile by avoiding the need to cross compile fontconfig and all its dependencies.

Be-ing commented Feb 14, 2022

View reviewed changes

fontconfig/src/lib.rs Show resolved Hide resolved

Be-ing force-pushed the dlopen branch 8 times, most recently from bada199 to 0b9008c Compare February 15, 2022 20:51

Be-ing mentioned this pull request Feb 15, 2022

switch to yeslogic-fontconfig-sys instead of servo-fontconfig servo/font-kit#192

Merged

Be-ing mentioned this pull request Feb 16, 2022

switch to yeslogic-fontconfig-sys from servo-fontconfig slint-ui/slint#956

Merged

3 tasks

Be-ing force-pushed the dlopen branch from 0b9008c to fa46fa3 Compare February 16, 2022 09:35

wezm reviewed Feb 17, 2022

View reviewed changes

Be-ing force-pushed the dlopen branch 8 times, most recently from 5029ed8 to 5a966c9 Compare February 17, 2022 20:39

add dlopen Cargo feature using dlib to load fontconfig at runtime

a4e26f6

Fixes yeslogic#11

Be-ing force-pushed the dlopen branch from 5a966c9 to a4e26f6 Compare February 17, 2022 21:02

Be-ing force-pushed the dlopen branch from 55b6ff8 to 9b7bc5f Compare February 17, 2022 21:58

wezm merged commit 0f7c93f into yeslogic:master Feb 21, 2022

Be-ing mentioned this pull request Feb 22, 2022

Add docs and test coverage for dlopen feature #13

Merged

Be-ing mentioned this pull request Feb 22, 2022

use dlib instead of static linking vendored copy servo/libfontconfig#65

Closed

Be-ing deleted the dlopen branch April 5, 2022 01:51

@@ @@ -13,7 +13,7 @@ rust: @@
                 - nightly
                 - beta
                 - stable
-                - 1.38.0
+                - 1.43.0

add dlopen Cargo feature using dlib to load fontconfig at runtime #12

add dlopen Cargo feature using dlib to load fontconfig at runtime #12

Uh oh!

Conversation

Be-ing commented Feb 14, 2022

Uh oh!

Uh oh!

Be-ing commented Feb 16, 2022

Uh oh!

wezm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wezm Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

Be-ing Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

wezm Feb 18, 2022

Choose a reason for hiding this comment

Uh oh!

wezm Feb 16, 2022

Choose a reason for hiding this comment

Uh oh!

Be-ing Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Be-ing commented Feb 17, 2022

Uh oh!

wezm commented Feb 17, 2022

Uh oh!

Be-ing commented Feb 18, 2022

Uh oh!

tronical commented Feb 18, 2022

Uh oh!

Be-ing commented Feb 21, 2022

Uh oh!

wezm commented Feb 22, 2022

Uh oh!

wezm commented Feb 22, 2022

Uh oh!

Uh oh!