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

Ability to use libdbus for client's dbus messaging #147

Merged
merged 1 commit into from
May 29, 2019

Conversation

gicmo
Copy link
Contributor

@gicmo gicmo commented May 24, 2019

In flatpaks systemd is not available, and thus would need to be build as an extra dependency (see com.valvesoftware.Steam#180). For the GameMode Tester App, I re-implemented the client bits and extracted those into a separate project: gamemode-libdbus. The code from this PR is basically taking from there and adds support for using libdbus.
I am a bit unsure what is the best way to go:

  • Add libdbus support to GameMode (this PR):
    • pro: central place where code is located
    • con: having #ifdef/if ! systemd_dep.found() sprinkled all over the place and two basically different client libs in one code-base
  • Use gamemode-libdbus in flatpaks
    • pro: small dependency for flatpaks
    • con: having to maintain a separate client implementation (maybe not that bad?)
  • Always use libdbus in GameMode instead of systemd
    • pro: no ifdef/if-hell
    • pro: central location of all client code
    • con: tow different dbus APIs present in one project (maybe not so bad?)

What do you guys think?

@mdiluz
Copy link
Contributor

mdiluz commented May 24, 2019

So I suppose I should explain that I used systemd because I felt it's api was far more elegant and much easier to use. I also could actually get the damn thing to work, where libdbus seemed to constantly give pain.

Given libdbus-1 and systemd are effectively interchangeable in this context (correct?) I think swapping the client lib to libdbus seems fine. We'll have two dbus implementations at once but oh well.

If you wanted to PR a swap the daemon to dbus as well wouldn't see that as a bad change either as long as it was cleanly written and thoroughly tested. Just maybe wait a bit - I have one hell of a project cleanup/reorganisation PR incoming...

@gicmo
Copy link
Contributor Author

gicmo commented May 24, 2019

@mdiluz very cool. I will change this PR to switch the client and then sit tight until your cleanup work is landed!

@mdiluz
Copy link
Contributor

mdiluz commented May 24, 2019

Also side note: GameMode Tester is very cool. Was wondering if I needed to make the testing code a bit less of a hack because currently it only allows testing if there isn't a portal between the two. This at least helps a ton, but I'll maybe do a PR sometime to refactor testing into something more broadly usable.

@cwilling
Copy link

Does this change help other more general non-systemd use cases (not necessarily just flatpack usage)? I see #15 and #51 have already asked about this - perhaps this change is a possible solution. If so, very useful.

@mdiluz
Copy link
Contributor

mdiluz commented May 26, 2019

@cwilling if the daemon is swapped over too, then yes. Though I think there'll be a little nuance in making sure it's a positive solution for both user and developer experience.

@gicmo gicmo force-pushed the libdbus branch 2 times, most recently from 265bb1b to 325f02a Compare May 27, 2019 11:43
Switch the dbus implementation for the client from systemd to
libdbus. The main reason is that, in flatpaks systemd is not easily
available. No phenomenological change for users of the library,
hopefully.
@aejsmith aejsmith merged commit df73880 into FeralInteractive:master May 29, 2019
@gicmo gicmo deleted the libdbus branch May 29, 2019 14:58
@aejsmith
Copy link
Contributor

I'm happy with doing this for the client library now, looks good to me and I can't see any regressions so I've merged it. Noticed just after merging that you'd marked it as draft though... did you have more planned changes?

@gicmo
Copy link
Contributor Author

gicmo commented May 31, 2019

@aejsmith Sorry for the late answer. Nope, was good to go. Just forgot to remove the draft status. Thanks for merging! I will convert the server to libdbus too, once #148 is merged.

@cwilling
Copy link

cwilling commented Jun 1, 2019

Great @aejsmith, looking forward to it. I was going to try it myself although without any previous experience of dbus API. It's definitely better to be done by someone who knows what they're doing :)

mdiluz added a commit to mdiluz/gamemode that referenced this pull request Jun 1, 2019
afayaz-feral pushed a commit that referenced this pull request May 26, 2020
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.

4 participants