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

target: Use the correct libdir when run in flatpak #386

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

target: Use the correct libdir when run in flatpak #386

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion src/target.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,12 @@ impl Target {
self.os.eq_ignore_ascii_case("windows")
}

fn is_flatpak(&self) -> bool {
PathBuf::from("/.flatpak-info").exists()
Copy link
Author

@alatiera alatiera Jun 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

an even more correct solution would be parsing this file and using the LD_LIBRARY_PATH= key from it, but as is should be fine as well for building apps.

(There is an edge case for building runtimes with flatpak-builder where they would want /usr/lib64 in that case, but we could do that if the need arises imo)

Copy link
Owner

Choose a reason for hiding this comment

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

Is meson also doing this? (I'm all for consistency)

Copy link
Owner

Choose a reason for hiding this comment

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

Also, I wonder if when building flatpak you wouldn't have already a standardized way to pass the libdir you'd expect?

Copy link
Author

@alatiera alatiera Jun 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is meson also doing this? (I'm all for consistency)

Not yet, with meson flatpak passes the --libdir explicitly (though I was thinking of adding a similar codepath in meson just to make sure), this popped up in an app issue which was calling cargo-c by hand.

Also, I wonder if when building flatpak you wouldn't have already a standardized way to pass the libdir you'd expect?

For buildsystems that flatpak-builder knows about, meson, autotools etc, it does call them with --prefix/--libdir/... explicitly, however when you need to do something custom like random bash commands or scripts, which is how applications usually install the rust gtk gstreamer plugin, it has to be done by hand and it broke the other day.

        {
            "name": "gst-plugin-gtk4",
            "buildsystem": "simple",
            "sources": [
                {
                "type": "archive",
                "url": "https://crates.io/api/v1/crates/gst-plugin-gtk4/0.12.5/download",
                "dest-filename": "gst-plugin-gtk4-0.12.5.tar.gz",
                "sha256": "56e483cb1452f056ae94ccd5f63bdec697e04c87b30d89eb30c3f934042e1022"
                },
                "gst-plugin-gtk4-sources.json"
            ],
            "build-commands": [
                "cargo cinstall --offline --release --features=wayland,x11glx,x11egl,gtk_v4_14 --library-type=cdylib --prefix=/app --libdir=/app/lib"
            ],

Nothing earth shuttering, but we could avoid having to specify the libdir.

Additionally LD_LIBRARY_PATH is exported and set in the build sandbox, but contains the same value as the flatpak-info file. Now the info file is guaranteed to always be there, but I am not so sure about the env var. If the variable is also guaranteed it's probably better to check for that when in flatpak, as someone might try to overwrite it (even though they should avoid doing that). Let me get back to you on that.

Copy link
Owner

Choose a reason for hiding this comment

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

If you could add support for cargo-c so that --libdir lib or --libdir lib64 is passed we'd have less special case in the codebase.

}

pub fn default_libdir(&self) -> PathBuf {
if self.is_target_overridden || self.is_freebsd() {
if self.is_target_overridden || self.is_freebsd() || self.is_flatpak() {
return "lib".into();
}

Expand Down