mpd: Allow mpd connection via UNIX socket #349

FloatingGhost · 2017-07-14T14:24:10Z

Previously, the mpd widget couldn't communicate via a UNIX socket, leading to it being basically useless in that case.

We can use exactly the same command sent through socat to fix this, which this implements.

Instead of using a string instance's sub() function we can instead use the global one to clean things up a bit

lcpz · 2017-07-14T15:36:41Z

Thanks for your time, but this has the same points and problems of #316, since socat is a dependency.

This is my desideratum. In short, I want to remove curl as dependency and use sockets for network-based widgets, but I don't have the time at the moment. I'll keep this open too until it's done.

FloatingGhost · 2017-07-17T09:53:30Z

ok, I've added a basic implementation of a send to unix socket w/ lua-lgi (because I had no internet and had nothing else to do ;0)

The main issue is going to be maintaining that async format - I'm really rather new to lua so idk if there's an easy way to do it

Both network methods appear to work in isolation, just gotta see if they work when integrated properly. Saves anyone else diving into LGI I guess

lcpz · 2017-07-17T13:43:22Z

helpers.lua

@@ -15,6 +15,8 @@ local io         = { lines = io.lines,
 local rawget     = rawget
 local table      = { sort  = table.sort }

+local gio = require("lgi").Gio


Please, rename the variable to Gio, for consistency.

lcpz · 2017-07-17T13:59:23Z

I appreciate your efforts, but don't reinvent the wheel.

My aim is to generalise this as a lain.helper (indeed, you had the right idea).

But, using Gio.UnixInputStream instead of Gio.DataInputStream.

FloatingGhost · 2017-07-17T14:02:55Z

Literally just as I get it to work properly over Gio streams as well >__<

Eh I'll see if I can get that to work instead, SocketClients make me want to cry

psychon · 2017-08-29T08:55:18Z

helpers.lua

+    end
+
+    return recv_buffer
+end


return string.rep(" ", buffer_length + 1) (the +1 is there to match the behaviour of your code, but I don't know if your code really should do that)

psychon · 2017-08-29T08:58:31Z

helpers.lua

+    output_stream:read(recv_buffer)
+    output_stream:read(recv_buffer)
+
+    callback(recv_buffer)


Hm... this will actually "break Lua". Lua assumes that strings are immutable. So, e.g., if any other piece of code constructed the same number of blanks before, the received data will also end up in there.

Also, you should conn:close() when done.

(Note that I just looked over this in a drive-by fashion, @copycat-killer gets to decide things and I have no say in anything here)

@psychon I appreciate any review of yours, for me you are free to come by as you like.

FloatingGhost · 2017-08-29T09:00:31Z

heh, it must be pretty obvious that I'd never used lua before writing this I guess

T'is a silly language
I'm using the PR version on my system and it doesn't seem to... break, so I guess there's that

razamatan · 2019-09-13T08:49:17Z

what more needs to be done here? just a rebase w/ current head to get rid of the merge conflicts?

lcpz · 2019-09-16T14:24:26Z

@razamatan See #349 (comment)

psychon · 2019-09-16T16:20:41Z

@lcpz Do you know about https://awesomewm.org/recipes/mpc.lua already? It connects to mpd over either TCP or a unix socket and provides an asynchronous API for talking to the daemon.

lcpz · 2019-09-16T16:50:59Z

@psychon The idea was indeed to readapt that. No need to reinvent the wheel.

razamatan · 2020-04-17T11:53:22Z

ok.. i'm taking this up. do you want a stateful socket that tries to reuse the connection as long as possible, or do you want to go the direction that @FloatingGhost is headed with reconnecting every time for a request/response?

lcpz · 2020-04-17T19:24:47Z

@razamatan The idea is still this. Hence, sockets. Try to work with the above-mentioned code, it should be easy.

razamatan · 2020-04-17T21:01:29Z

stateful sockets, got it.. i started this and i'm working it out now.

razamatan · 2020-04-20T10:39:26Z

hmm... while making the socket helper/class generalizable, the socket protocol is too ambiguous to make the helper/class useful beyond some initial connection boilerplate. the central read loop needs to be implemented per protocol/usage: http things would go one way, imap another, mpd has it's own client-initiated conversational api, etc. etc. once i gutted the main read loop to be a user-defined callback, it really isn't much. i could make it a base class for each client/protocol to implement/inherit, but i'm not sure that's what you really wanted here.

also, why the preference for UnixInputStream over DataInputStream?

lcpz · 2020-04-21T17:25:09Z

i could make it a base class for each client/protocol to implement/inherit, but i'm not sure that's what you really wanted here

You could create a protocol submodule which, for the time being, provides MPD connections only.

In the future, we can also add HTTP(S) and IMAP (and finally remove Curl as a dependency).

Feel free to push WIP commits, if you want my commentary as you work.

We can squash them when you are done.

also, why the preference for UnixInputStream over DataInputStream?

Because the former is pollable, while the latter is not. But it is no religion: feel free to show me that DataInputStreams suit better.

razamatan · 2020-04-23T08:14:04Z

qq.. what's more important, not taking on any dependency at all, or not adding a non-lua approach? the more and more i'm dealing w/ this lgi wrapped GIO stuff, the more i am inclined to say that luasocket may be a better fit here.

lcpz · 2020-04-23T13:08:52Z

@razamatan There is already a non-Lua approach: it's Curl, which has also been preferred over LuaSocket for these reasons.

EDIT:

what's more important, not taking on any dependency at all, or not adding a non-lua approach?

Actually both. Once we base on GIO, we won't have any dependency at all, Lua or non-Lua.

psychon · 2020-04-23T14:05:03Z

@razamatan Anything I can help you with in LGI? Does perhaps already lgi-devs/lgi#243 help you?

Another helpful pointer could be awful.spawn.read_lines: https://github.com/awesomeWM/awesome/blob/87928befe2dc981bfdbca59c0474c0ff5d39544f/lib/awful/spawn.lua#L504
This gets an input stream and read lines from it. Each line is passed to a callback function. Another callback is called on EOF.

razamatan · 2020-04-23T18:04:06Z

i have things working fine for mpd using lgi and UnixInputStream as requested, but for http, it's a bit more complicated. in order to be robust against all the different possible protocol versions, ssl, redirects, and all the features of the http spec, it's redundant to roll yet another http client. being open source, i could just grab their http.lua and just refactor it to be only client focused and gio based. still, more time taken.

the PITA for lgi is that it doesn't have any method level documentation (showing input and return values) since it does things to make the lua binding more lua like. the overview and guide explain things, but it's much, much easier if there would be just something that shows all the available methods and their signatures. i've been relying on pgi docs online and testing things to verify behaviors.

psychon · 2020-04-23T18:44:38Z

I think "the way to go" with Gio and http is to just open an URL as a file, but that requires a new package whose name I forgot (something like gio-networking). With that, https://developer.gnome.org/gio/stable/GFile.html#g-file-new-for-uri should just work.

razamatan · 2020-05-23T18:49:07Z

the file abstraction may be fine for terminal rpcs like http requests/responses, but for persistent client/server connections like mpd which prefers keeping the connection alive using idle commands, the file abstraction might not be the best thing.

razamatan · 2023-01-13T10:57:25Z

so work got a bit lighter, so i'm picking this up again.

Gio.File seems to do what we want for most cases (as mentioned by @psychon) that just wants to download a full file, but it requires making sure folks have gvfs installed so gobject-introspection picks up uri support in gio. the alternative of going w/ the UnixInputStream approach is that all protocols need to be implemented at the socket layer, which is a tax on ongoing maintenance (i was doing this route, but shelved it after psychon's suggestion). However! i found that lgi supports libsoup, which is the level of support we care for anyway, so i got that working.

@lcpz are you ok with taking a dep on lgi w/ libsoup? people who aren't gnome heavy won't have it installed like me (i run a pretty lean X in gentoo).

#549

lcpz · 2023-01-15T18:18:55Z

@razamatan Thank you for your work, and for sticking around for so long! I appreciate it.

I understand that having no dependencies requires utilities that need to be maintained, which is not reasonable.

Thus, I think (lib)curl is a better choice than libsoup, as it is more mature, lighter, and more adopted.

What do you guys think?

razamatan · 2023-01-15T21:40:55Z

wouldn't using libcurl end up with another dependency on top of it to handle the c <=> lua handling? i understood #349 (comment) to limit the number of things that users would need to install on top of awesomewm. so, lgi => gi => gio|soup was what i thought was wanted.

all this said, unless gnome is thinking of removing libsoup, i don't think it's a bad dependency to take.

lcpz · 2023-01-15T21:59:03Z

I mean to say that there is already curl as a dependency, which seems better than libsoup, so it's not worth to switch.

But I am open to thoughts.

razamatan · 2023-01-15T22:06:10Z

but the usage of curl as a spawn shell command is the root of the issue, right? if we want to move to an all lua interface for these rpcs, we would take on luacurl or some other lua wrapper for libcurl as another dependency. so either some lua->curl wrapper, or making sure folks have libsoup installed so lgi supports it. my feeling is that people who are already in the world of gnome/gobjects likely will have libsoup installed, but most folks won't have a lua wrapper for libcurl installed.

…

________________________________ From: Luca CPZ ***@***.***> Sent: Sunday, January 15, 2023 1:59 PM To: lcpz/lain ***@***.***> Cc: razamatan ***@***.***>; Mention ***@***.***> Subject: Re: [lcpz/lain] mpd: Allow mpd connection via UNIX socket (#349) I mean to say that there is already curl as a dependency, which seems better than libsoup, so it's not worth to switch. But I am open to thoughts. — Reply to this email directly, view it on GitHub<#349 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAD53T4HVCMSF4K27YQOD2TWSRXLDANCNFSM4DTBIVHQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

razamatan · 2023-01-17T22:53:08Z

you know what.. i'm going to see if i can just use basic lua -> c behaviors to avoid using a canned lua->libcurl wrapper as a dependency. i don't think we need a whole lot of functionality. this way, no new deps, and it'll be easier to migrate from curl to libcurl that it provides.

razamatan · 2023-01-18T03:15:18Z

(to clarify the previous comment)

i came to this conclusion b/c https://github.com/Lua-cURL/Lua-cURLv3 (lua-curl; note the hyphen) is the best that's out there for lua and libcurl afaict, and it hasn't been touched in over a year and a half. luacurl's (non-hyphenated) public repo seems to be no more.

if i don't get something going along the lines suggested in the previous comment that seems easy to maintain in a few more hours, i'll give up and settle on libsoup b/c: 1. gnome supports it 2. gi supports it 3. lgi supports it 4. it works in #549. this feels much more stabler than relying on some lua-?curl impl...

razamatan · 2023-01-19T05:09:24Z

ok.. i learned how to call c from lua, and i don't want to maintain another lua*curl wrapper. we don't need another in the world as lua-curl(v3) should be sufficient. however, i don't want to go off and help maintain that library either.

the lightest maintenance cost is going to be whatever we can get cheaply from lgi, so libsoup as i implemented it is the simplest. let's go with it.

FloatingGhost added 2 commits July 14, 2017 15:19

mpd: Allow mpd connection via UNIX socket

69a3eb9

mpd: Import the string function directly

9d6ad74

Instead of using a string instance's sub() function we can instead use the global one to clean things up a bit

lcpz added this to the release 1.0 milestone Jul 14, 2017

helpers: Add UNIX socket connection method to helpers

c20eae8

FloatingGhost added 3 commits July 17, 2017 11:16

helpers: Add Basic IPv4 socket send

d1332d9

Both network methods appear to work in isolation, just gotta see if they work when integrated properly. Saves anyone else diving into LGI I guess

helpers: Fix variable names in network methods

f416604

helpers: Make sure we close the socket when we're done

c4903a9

lcpz reviewed Jul 17, 2017

View reviewed changes

FloatingGhost added 2 commits July 17, 2017 15:12

mpd: Communication between socket and TCP working

a697b94

mpd: Remove unused result variable

61bdee5

psychon reviewed Aug 29, 2017

View reviewed changes

lcpz force-pushed the master branch 2 times, most recently from cd4f448 to 794f5d3 Compare September 11, 2017 09:08

lcpz force-pushed the master branch 2 times, most recently from b14a2c8 to 055e663 Compare October 5, 2017 09:32

lcpz force-pushed the master branch 5 times, most recently from d2fd3d5 to 24ca585 Compare February 16, 2018 16:52

lcpz force-pushed the master branch 4 times, most recently from 112fcae to cf8ef78 Compare October 28, 2018 12:58

lcpz force-pushed the master branch 2 times, most recently from 1291b1c to 012c6b6 Compare April 9, 2019 11:46

razamatan mentioned this pull request Apr 21, 2020

mpd widget stopped working: N/A #452

Open

lcpz force-pushed the master branch 3 times, most recently from a5b407f to a955bbb Compare March 30, 2021 10:14

lcpz mentioned this pull request Jan 15, 2023

introduce helpers.uri (curl replacement) & migrate widget/weather to it #549

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mpd: Allow mpd connection via UNIX socket #349

mpd: Allow mpd connection via UNIX socket #349

FloatingGhost commented Jul 14, 2017

lcpz commented Jul 14, 2017

FloatingGhost commented Jul 17, 2017

lcpz Jul 17, 2017 •

edited

Loading

lcpz commented Jul 17, 2017 •

edited

Loading

FloatingGhost commented Jul 17, 2017

psychon Aug 29, 2017

psychon Aug 29, 2017

lcpz Aug 29, 2017

FloatingGhost commented Aug 29, 2017

razamatan commented Sep 13, 2019

lcpz commented Sep 16, 2019

psychon commented Sep 16, 2019

lcpz commented Sep 16, 2019

razamatan commented Apr 17, 2020

lcpz commented Apr 17, 2020

razamatan commented Apr 17, 2020

razamatan commented Apr 20, 2020 •

edited

Loading

lcpz commented Apr 21, 2020

razamatan commented Apr 23, 2020

lcpz commented Apr 23, 2020 •

edited

Loading

psychon commented Apr 23, 2020

razamatan commented Apr 23, 2020

psychon commented Apr 23, 2020

razamatan commented May 23, 2020

razamatan commented Jan 13, 2023 •

edited

Loading

lcpz commented Jan 15, 2023

razamatan commented Jan 15, 2023

lcpz commented Jan 15, 2023

razamatan commented Jan 15, 2023 via email

razamatan commented Jan 17, 2023

razamatan commented Jan 18, 2023

razamatan commented Jan 19, 2023

mpd: Allow mpd connection via UNIX socket #349

Are you sure you want to change the base?

mpd: Allow mpd connection via UNIX socket #349

Conversation

FloatingGhost commented Jul 14, 2017

lcpz commented Jul 14, 2017

FloatingGhost commented Jul 17, 2017

lcpz Jul 17, 2017 • edited Loading

Choose a reason for hiding this comment

lcpz commented Jul 17, 2017 • edited Loading

FloatingGhost commented Jul 17, 2017

psychon Aug 29, 2017

Choose a reason for hiding this comment

psychon Aug 29, 2017

Choose a reason for hiding this comment

lcpz Aug 29, 2017

Choose a reason for hiding this comment

FloatingGhost commented Aug 29, 2017

razamatan commented Sep 13, 2019

lcpz commented Sep 16, 2019

psychon commented Sep 16, 2019

lcpz commented Sep 16, 2019

razamatan commented Apr 17, 2020

lcpz commented Apr 17, 2020

razamatan commented Apr 17, 2020

razamatan commented Apr 20, 2020 • edited Loading

lcpz commented Apr 21, 2020

razamatan commented Apr 23, 2020

lcpz commented Apr 23, 2020 • edited Loading

psychon commented Apr 23, 2020

razamatan commented Apr 23, 2020

psychon commented Apr 23, 2020

razamatan commented May 23, 2020

razamatan commented Jan 13, 2023 • edited Loading

lcpz commented Jan 15, 2023

razamatan commented Jan 15, 2023

lcpz commented Jan 15, 2023

razamatan commented Jan 15, 2023 via email

razamatan commented Jan 17, 2023

razamatan commented Jan 18, 2023

razamatan commented Jan 19, 2023

lcpz Jul 17, 2017 •

edited

Loading

lcpz commented Jul 17, 2017 •

edited

Loading

razamatan commented Apr 20, 2020 •

edited

Loading

lcpz commented Apr 23, 2020 •

edited

Loading

razamatan commented Jan 13, 2023 •

edited

Loading