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

fix not linking to dbus #5

Merged
merged 1 commit into from
Oct 10, 2022
Merged

Conversation

Deminder
Copy link
Contributor

@Deminder Deminder commented Oct 10, 2022

I have been wondering why this did not cause problems before but dbus is not linked in the shared library.
#4 is also happening now for me (new mpv version).

[mpv_inhibit_gnome] C plugin error: '/app/etc/mpv/scripts/mpv_inhibit_gnome.so: undefined symbol: dbus_bus_get'
[mpv_inhibit_gnome] Could not load SO plugin /app/etc/mpv/scripts/mpv_inhibit_gnome.so

@Erick555
Copy link

It would be nice if makefile honored CFLAGS/LDFLAGS env variables. Currently it unconditionally adds -g and doesn't use any compiler/linker optimization

@Deminder
Copy link
Contributor Author

Deminder commented Oct 10, 2022

@Erick555 what do you think of building with meson, instead?
https://github.com/Deminder/mpv_inhibit_gnome/blob/meson-build/meson.build

@Erick555
Copy link

Well, it depends on what is upstream preference. BTW: why prefix=/etc?

@Deminder
Copy link
Contributor Author

why prefix=/etc?

For a default (non-flatpak) system-wide install.
./install.sh flatpak installs to flatpak config dir https://github.com/Deminder/mpv_inhibit_gnome/blob/meson-build/install.sh

@Guldoman
Copy link
Owner

Nice work, LGTM. I'll tag a new release with this fix in a few hours.

It would be nice if makefile honored CFLAGS/LDFLAGS env variables. Currently it unconditionally adds -g and doesn't use any compiler/linker optimization

What should it look like?
Would simply doing

CFLAGS += $(shell pkg-config --libs --cflags dbus-1)

be enough?
Or should we also change the compilation commands to also use LDFLAGS?

@Guldoman Guldoman merged commit 10b87ce into Guldoman:master Oct 10, 2022
@Erick555
Copy link

Erick555 commented Oct 10, 2022

For a default (non-flatpak) system-wide install.

The project README says system-wide install path is /usr/share/mpv/scripts.

@Guldoman

What should it look like? Would simply doing

CFLAGS += $(shell pkg-config --libs --cflags dbus-1)

Yeah, it should work like that

be enough? Or should we also change the compilation commands to also use LDFLAGS?

Maybe like below:

CFLAGS += $(shell pkg-config --cflags dbus-1)
LDFLAGS += $(shell pkg-config --libs dbus-1)

then:
gcc $(CFLAGS) $(LDFLAGS) ...

@Guldoman
Copy link
Owner

Would you mind opening a PR with your suggestions? Otherwise I'll do that myself in a few days.

@Erick555
Copy link

I'm pretty busy right now so It's possible that I wouldn't find the time before you do 😄

@Guldoman
Copy link
Owner

No worries!

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.

3 participants