Support dynamic linking for prebuilt binaries by eerii · Pull Request #148 · gdesmott/system-deps

eerii · 2026-09-04T11:39:43Z

Prebuilt binaries can come with static and shared libraries (gstreamer bundles come with both). The initial support for them forced static linking, but dynamic linking may be beneficial for some applications, especially when dealing with licenses. This is motivated by an experiment to support dynamic linking of gstreamer from the downloaded bundles.

This patch makes it so the SYSTEM_DEPS_LINK environment variable works for prebuilt binaries too. In this case, the default remains static to avoid breaking the previous behaviour. It doesn't affect the probing of system-installed libraries at all.

Note that this mode only changes how libraries are linked. Deployment is out of scope, and left entirely to the applications using system-deps, meaning they are responsible for placing the libraries in the correct place and setting the relevant paths in the binary. We already provide Config::query_path, which gives the path of the extracted bundle so applications can distribute it as they see fit.

codecov · 2026-09-04T11:41:34Z

Codecov Report

❌ Patch coverage is 70.80745% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.77%. Comparing base (4aa1528) to head (2981765).

Files with missing lines	Patch %	Lines
src/lib.rs	44.04%	47 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #148      +/-   ##
==========================================
- Coverage   92.64%   91.77%   -0.88%     
==========================================
  Files           8        8              
  Lines        3564     3708     +144     
==========================================
+ Hits         3302     3403     +101     
- Misses        262      305      +43

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

eerii · 2026-09-04T11:51:58Z

The clippy warning is unrelated to this patch, it comes from updating the rust version. I submitted a fix in #149.

gdesmott · 2026-09-21T11:21:08Z

Your clippy fix has been merged so you can rebase this branch on top of main to check the CI.

I'll let @thiblahute review it as he's the one knowing about prebuilt binaries.

thiblahute · 2026-09-21T13:35:44Z

+        self.wildcards
+            .iter()
+            .find(|(prefix, provider)| {
+                key.starts_with(prefix.as_str()) && self.paths.contains_key(*provider)


Pre-existing, but now more visible: wildcards is a HashMap, so with overlapping wildcards the provider picked is nondeterministic, and it now also decides the link mode.

thiblahute · 2026-09-21T13:35:44Z

    /// The `cfg()` expression used in `Cargo.toml` is currently not supported
    UnsupportedCfg(String),
+    /// Two packages provided by the same prebuilt binary resolved to different link modes
+    LinkModeMismatch(String, String),


Error isn't #[non_exhaustive], so adding a variant is a semver break. Maybe fine if the next release is a major one anyway, is that the plan @gdesmott ?

thiblahute · 2026-09-21T13:35:44Z

                "Internally built {s1} {s2} but minimum required version is {s3}"
            ),
            Self::UnsupportedCfg(s) => write!(f, "Unsupported cfg() expression: {s}"),
+            Self::LinkModeMismatch(name, package) => write!(


In this match arm the pattern variables are (name, package), but check_link_modes builds the variant as LinkModeMismatch(provider, name), so name here is really the provider -- (provider, package) would read better.

thiblahute · 2026-09-21T13:35:44Z

+    }
+
+    /// Check if the dependency should be statically linked.
+    fn link_mode(&self, pkg: &str) -> bool {


It returns a bool meaning "static", so is_static() (or returning an enum) would read better than link_mode().

thiblahute · 2026-09-21T13:35:44Z

+
+        match vars.iter().flatten().find_map(|var| self.env.get(var)) {
+            Some(mode) => mode == "static",
+            None => provider.is_some(),


The default comes from provider(), which ignores SYSTEM_DEPS_$NAME_NO_PREBUILT / SYSTEM_DEPS_NO_PREBUILT, while query_path() does respect them. So when a user opts a package out of the bundle, it is probed from the system but still defaults to static linking.

I reproduced it by adding this test to src/test_binary.rs on this branch, reusing the config_with() helper from the PR (the test bundle provides dep):

#[test] fn no_prebuilt_link_mode() { let c = config_with(HashMap::from([ ("SYSTEM_DEPS_DEP_NO_PREBUILT", "1".into()), ])); // dep is no longer served by the bundle... assert!(c.query_path("dep").is_none()); // ...but still defaults to static: this assert fails assert!(!c.link_mode("dep")); }

The second assert fails with cargo test --features binary.

I think the default (and the provider lookup) should be based on whether the package is actually served by the bundle, i.e. query_path(pkg).is_some(), and a test like the one above should be added.

thiblahute · 2026-09-21T13:35:44Z

+                .skip(1)
+                .any(|parent| lib_dirs.iter().any(|other| other == &parent));
+            if !nested {
+                println!("cargo:rustc-link-search=native={}", dir.display());


Are these rustc-link-search lines needed? The -sys crate already emits the link paths from pkg-config, and those propagate to the final link.

thiblahute · 2026-09-21T13:35:44Z

+
+        if rpath_origin == "$ORIGIN" {
+            println!(
+                "cargo:rustc-link-arg=-Wl,--disable-new-dtags,-rpath,{rpath_origin}/{runtime_subdir}"


--disable-new-dtags (forcing DT_RPATH so it also applies to transitive deps) is a deliberate choice, please add a comment explaining it.

thiblahute · 2026-09-21T13:35:44Z

                    .cargo_metadata(false)
                    .range_version(metadata::parse_version(version))
-                    .statik(statik);
+                    .statik(statik || prebuilt);


Why query pkg-config with --static for prebuilt packages even in dynamic mode? That pulls in Libs.private, so a dynamic build links every private dependency of the bundle (overlinking, and on macOS a lot more dylibs loaded at startup).

If it works around something missing in the bundle's .pc files, please add a comment; otherwise I'd expect plain .statik(statik) here.

thiblahute · 2026-09-21T13:35:44Z

        Ok(libraries)
    }

+    fn check_link_modes(&self, libraries: &Dependencies) -> Result<(), Error> {


Same root cause as the link_mode() default: grouping by provider() includes packages that are no longer served by the bundle. SYSTEM_DEPS_DEP_NO_PREBUILT=1 + SYSTEM_DEPS_DEP_LINK=dynamic fails with LinkModeMismatch although dep comes from the system and cannot mix anything with the bundle.

Could you add a test for the NO_PREBUILT case as well?

thiblahute · 2026-09-21T13:35:44Z


    let testdata = libraries.get_by_name("testdata").unwrap();
-    assert!(testdata.statik == cfg!(feature = "binary"));
+    assert!(!testdata.statik);


This is a behaviour change: before, enabling the binary feature made every library static, system ones included; now system libs default to dynamic. I think it's the right fix, but the PR description says it "doesn't affect the probing of system-installed libraries at all", which isn't quite true. Worth mentioning in the changelog.

eerii marked this pull request as draft September 7, 2026 02:35

eerii force-pushed the dynamic-binary branch from 433004a to 39740f1 Compare September 7, 2026 08:49

eerii marked this pull request as ready for review September 7, 2026 10:59

Support dynamic linking for prebuilt binaries

2981765

eerii force-pushed the dynamic-binary branch from 39740f1 to 2981765 Compare September 8, 2026 21:44

eerii mentioned this pull request Sep 8, 2026

Document LGPL requirements incurred by enabling the media feature (linking gstreamer) servo/servo#47770

Open

gdesmott requested a review from thiblahute September 21, 2026 11:21

thiblahute requested changes Sep 21, 2026

View reviewed changes

Navigation Menu

Repository navigation

Support dynamic linking for prebuilt binaries - #148

Support dynamic linking for prebuilt binaries#148
eerii wants to merge 1 commit into
gdesmott:mainfrom
eerii:dynamic-binary

eerii commented Sep 4, 2026

Uh oh!

codecov Bot commented Sep 4, 2026 •

edited

Loading

Uh oh!

eerii commented Sep 4, 2026

Uh oh!

gdesmott commented Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026 •

edited

Loading

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

thiblahute Sep 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

eerii commented Sep 4, 2026

Uh oh!

codecov Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

eerii commented Sep 4, 2026

Uh oh!

gdesmott commented Sep 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thiblahute Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Sep 4, 2026 •

edited

Loading

thiblahute Sep 21, 2026 •

edited

Loading