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

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

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

Conversation

alatiera
Copy link

No description provided.

@@ -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

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

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.

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