Unix sockets (implements #42)

[EliteTK]

I thought I would take a stab at implementing support for unix domain sockets. This involves:

• switching from port numbers to identify clients to using an incremental unique ID (since unix sockets don't have ports, and connections don't even have a named socket address unless you bind them first (which this project doesn't do))
• wrapping TcpListener, TcpStream and tcp::OwnedReadHalf/OwnedWriteHalf such that the code is socket type agnostic
• extending the configuration in a forwards compatible manner to allow specifying a string unix socket path

There's a few things I still want to address:

• conditionally compile all this extra stuff out on windows
• rename ListenAddr to something else as it's used for both listen and connect
• tests, I haven't even looked at whether there are existing tests which relate to this, all the current tests pass but I would feel better about adding a new feature if there were some tests
• revert the change to Cargo.lock (this happened by accident when I was investigating if pin-project-lite would work, but it didn't seem to support enums with tuple variants, and even if the pins could technically be updated, this change has no business being in that commit)
• see if pin-project-lite can be used instead of pin-project (maybe using struct variants rather than enum variants)

Lastly, a few things I'm still considering and wouldn't mind others' opinions on:

• make listen/connect use a scheme like "unix:" and "tcp:" to allow specifying TCP listen/connect addresses using a string (thereby allowing a domain name to be specified)
• remove the need for a separate connect_{tcp,unix} and bind_{tcp,unix} function since it's just an unnecessary bit of noise at every call site at the moment
• change how connect and listen are specified, maybe by adding a third option which feeds into both, it seems weird that you need to change both in order to change the connection type, I think it's good flexibility to be able to set them individually but I think it would be best if for the 99% case only one option needed to be changed
• making unix sockets the default on unix platforms (this would be a breaking change of sorts)
• reconsider exposing the un-projected enums directly because, as I understand it, destructuring and moving out of them would be unsafe and currently there's nothing enforcing that

I've not got that much experience with rust yet so let me know if there's some massively better way of doing anything I've done here. I know macros could be utilised to remove some of the repetition but I haven't really gotten around to learning about macros yet and felt that could be something which could be addressed at a later date.

[pr2502]

hi, thanks! this looks great.

regarding tests.. we don't really have any 😅 we'd either have to puppet some LSP client (like neovim) or mock it, but both of them have been too much effort for me to want to do them, i've instead spent time trying to make logs useful enough to debug problems retroactively when they happen during normal usage. i don't think it's fair to want you to add tests for this work, nor i think it's really necessary, all the transformations you made are really straight forward and i'd expect this to work fine^^

regarding pin-project. i'd prefer to use pin-project-lite considering we already have i in the dependency tree, i'll give it a stab too if there is a way to change it, we could probably just change the tuple variants to struct variants?

[EliteTK]

regarding tests.. we don't really have any 😅 we'd either have to puppet some LSP client (like neovim) or mock it, but both of them have been too much effort for me to want to do them, i've instead spent time trying to make logs useful enough to debug problems retroactively when they happen during normal usage. i don't think it's fair to want you to add tests for this work, nor i think it's really necessary, all the transformations you made are really straight forward and i'd expect this to work fine^^

Alright, I'll leave the to-do item but won't give it much priority then.

regarding pin-project. i'd prefer to use pin-project-lite considering we already have i in the dependency tree, i'll give it a stab too if there is a way to change it, we could probably just change the tuple variants to struct variants?

Nice suggestion, I have added this to my to-dos and will consider it. I think I considered using struct variants temporarily but that was before I discovered that pin-project-lite was already a dependency.

Will try to come up with some updates tonight. Thanks for the quick glance at this, it's encouraging to hear you like the approach at least.

[pr2502]

also ad this

reconsider exposing the un-projected enums directly because, as I understand it, destructuring and moving out of them would be unsafe and currently there's nothing enforcing that

this isn't possible. if the enum is not pinned it's safe to move, and if it is pinned with Pin<&mut Stream> you cannot destructure it anymore without using .project(). it would be a big issue with the pin-project{,-lite} crates if it was possible to use them to create unsafe code without writing unsafe literally somewhere, and since you didn't introduce any unsafe keywords or touched code using unsafe you don't need to worry about that^^

[EliteTK]

The changes to the earlier commits are solely to address some things I had omitted from my previous WIP version.

• I had forgotten to use std::fs in one of the commits when rebasing and it meant that an intermediate commit didn't build, this is addressed now (all commits now build individually without errors and are able to run tests/work)
• Now all commits build on windows with the addition of copious #[cfg(target_family = "unix")].
• Renamed ListenAddr to just Address because it's not listen specific.

I think the rest is superficial.

[pr2502]

just one small code style nitpick and i think this is done, good work :) thank you

[pr2502]

ok, i'm happy with this. if you just run cargo fmt on the repo and push a "rustfmt" commit, we can merge this:)

[EliteTK]

I've pushed to fix the CI failure relating to formatting, now all the individual commits should pass formatting checks. All the remaining things can be done in a later PR.

[EliteTK]

Ah, I almost forgot, this last one is just with the fixup! squashed.

[pr2502]

thank you! :)