zzag / kwin-effects-yet-another-magic-lamp

Just Yet Another Magic Lamp effect
GNU General Public License v2.0
162 stars 26 forks source link

Add dbus devel deps #23

Closed marcdeop closed 4 years ago

marcdeop commented 4 years ago

Even though #2 is marked as resolved, I faced the issue while compiling on Fedora.

This will:

marcdeop commented 4 years ago

I assumed Finddbus.cmake is usable as it's distributed under BSD license.

zzag commented 4 years ago

Currently, this plugin uses pkg-config to determine the file path of org.kde.kwin.Effects.xml

https://github.com/zzag/kwin-effects-yet-another-magic-lamp/blob/f411876086099638677d35c4cc023e5d046eff6a/src/kcm/CMakeLists.txt#L10-L12

but it is sort of incorrect because KWin ships a config file.

We need to do something along these lines instead

# src/CMakeLists.txt
find_package(KWinDBusInterface CONFIG REQUIRED)

# src/kcm/CMakeLists.txt
qt5_add_dbus_interface (kcm_SRCS ${KWIN_EFFECTS_INTERFACE} kwineffects_interface) 
marcdeop commented 4 years ago

@zzag but that sounds like a separate issue.

Should I open a bug report and try to fix it there?

zzag commented 4 years ago

This effect needs dbus-devel only because of the interfaces_dir variable, but as I said previously, there is a better way to find the file path of org.kde.kwin.Effects.xml, which doesn't require installing dbus-1-dev or dbus-devel.

marcdeop commented 4 years ago

This effect needs dbus-devel only because of the interfaces_dir variable, but as I said previously, there is a better way to find the file path of org.kde.kwin.Effects.xml, which doesn't require installing dbus-1-dev or dbus-devel.

ok, now I get it :-)

marcdeop commented 4 years ago

@zzag I updated the PR, feel free to re-review again :-)

zzag commented 4 years ago

Thanks.