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.

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

krun-sys: find libkrun using pkg-config #147

Merged
merged 2 commits into from
Feb 26, 2025
Merged

Conversation

slp
Copy link
Collaborator

@slp slp commented Feb 20, 2025

This enables the use of non-system-wide installations of libkrun by exporting the right PKG_CONFIG_PATH.

@teohhanhui
Copy link
Collaborator

teohhanhui commented Feb 20, 2025

We should investigate whether we can just use https://docs.rs/system-deps/latest/system_deps/

(as seem from rust-lang/rust-project-goals#108 (comment))

@slp
Copy link
Collaborator Author

slp commented Feb 20, 2025

Wonderful, our CI uses Ubuntu, and libkrun-dev is not a package there. :-(

We should investigate whether we can just use https://docs.rs/system-deps/latest/system_deps/

(as seem from rust-lang/rust-project-goals#108 (comment))

We're going to start using krun-sys on other projects, but we can move it to some other repo if muvm no longer needs it.

println!("cargo::rerun-if-changed=wrapper.h");
println!("cargo::rustc-link-lib=krun");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm most likely asking out of ignorance here, but https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-link-lib says this emits the -l flag to the compiler.

With this gone, where / how does that happen then? Below we only emit -I flags, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bindgen doesn't really need to link against the library, it just needs the headers.

Copy link
Collaborator

@teohhanhui teohhanhui Feb 20, 2025

Choose a reason for hiding this comment

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

Uhh... But since this crate is krun-sys (and specifically https://doc.rust-lang.org/cargo/reference/build-scripts.html#-sys-packages and https://doc.rust-lang.org/cargo/reference/manifest.html#the-links-field), I think it's expected that it'll actually link to libkrun...

Copy link
Collaborator

Choose a reason for hiding this comment

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

But also, I'm confused where the linking to libkrun happens if not here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was also getting very confused by this, because I was seeing the library getting injected at build time. Turns out pkg_config::probe_library("libkrun")?; also injects (by printing to stdout) parameters to the linker. The documentation of pkg-config is sparse, at best.

We need this to be able to switch krun-sys to pkg-config.

Signed-off-by: Sergio Lopez <slp@redhat.com>
@slp slp force-pushed the krun-use-pkgconfig branch from 6f2c672 to 471b12c Compare February 20, 2025 16:45
@slp slp force-pushed the krun-use-pkgconfig branch from 471b12c to 95aaa18 Compare February 20, 2025 19:42
@@ -10,6 +10,7 @@ links = "krun"

[build-dependencies]
bindgen = { version = "0.69.4", default-features = false }
pkg-config = { version = "0.3" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be explicit here and not use default features.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thanks.

This enables the use of non-system-wide installations of libkrun by
exporting the right PKG_CONFIG_PATH.

Signed-off-by: Sergio Lopez <slp@redhat.com>
@slp slp force-pushed the krun-use-pkgconfig branch from 95aaa18 to fc82dcd Compare February 20, 2025 21:44
@slp slp merged commit ca010ae into AsahiLinux:main Feb 26, 2025
2 checks passed
@slp slp deleted the krun-use-pkgconfig branch February 26, 2025 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants