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

External listener fd #310

Merged
merged 3 commits into from
Jun 14, 2024
Merged

External listener fd #310

merged 3 commits into from
Jun 14, 2024

Conversation

layercak3
Copy link
Contributor

@layercak3 layercak3 commented May 31, 2024

Tested with a systemd .socket unit.

Closes: #227
Requires: any1/neatvnc#111
I have read and understood CONTRIBUTING.md.

@layercak3
Copy link
Contributor Author

Looks like the sr.ht CI is being activated on GitHub PRs somehow but I don't see any indication of it checking out to my commit object. If it needs to be on PRs the linker should be failing on mine.

src/main.c Outdated Show resolved Hide resolved
src/main.c Outdated Show resolved Hide resolved
src/main.c Show resolved Hide resolved
wayvnc.scd Show resolved Hide resolved
@layercak3 layercak3 force-pushed the external-listener-fd branch 2 times, most recently from 96da611 to daec8dc Compare June 1, 2024 23:56
@layercak3
Copy link
Contributor Author

Force-pushed new commits with review addressed.

@any1
Copy link
Owner

any1 commented Jun 2, 2024

For some reason, the integration test is segfaulting on the github CI, but not srht.

@layercak3
Copy link
Contributor Author

layercak3 commented Jun 2, 2024

I deployed this on my server and external listener fd works, but it seems that I broke normal address listening after the review (I think I remember it working while working on the patch initially), maybe due to the atoi/option parser retval after removing the boolean. I'll have to fix that.
As for sr.ht, I stated earlier that I don't see any indication of it checking out to PR heads. Your commit: https://builds.sr.ht/~andri/job/1219324#setup-2245, my commit (not findable from a branch on heads/, but pull/310/head or my fork branch points to it): https://builds.sr.ht/~andri/job/1239409#setup-817

edit: clearly I used the wrong executable to test normal address listening as I forgot to set the default value in the option parser with the option unset, sorry about that

@layercak3 layercak3 marked this pull request as draft June 2, 2024 11:39
@layercak3
Copy link
Contributor Author

I checked on another user's PR and it was doing git checkout (https://builds.sr.ht/~andri/job/1227912), so I was wrong about that, it's only happening to my PRs, probably cause I named the repo "wayvnc-ghfork", maybe a bug in hottub-bridge :)

@layercak3 layercak3 marked this pull request as ready for review June 4, 2024 08:28
@any1
Copy link
Owner

any1 commented Jun 8, 2024

LGTM. There are conflicts. Can you resolve them?

@layercak3
Copy link
Contributor Author

Rebased

Instead of having wayvnc/neatvnc create a socket and listen() on it,
allow listening on an already bound socket file descriptor with the
--external-listener-fd=FD option. This may be used to support any kind of
connection-based socket that isn't explicitly supported by wayvnc in an option,
such as ones using the AF_VSOCK address family, or support cases where wayvnc
is being activated by systemd (via a .socket unit) or a super-server like
inetd.
@any1 any1 merged commit 2d62e12 into any1:master Jun 14, 2024
3 checks passed
@any1
Copy link
Owner

any1 commented Jun 14, 2024

Thanks!

@layercak3 layercak3 deleted the external-listener-fd branch June 16, 2024 04:42
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.

External listener fd
2 participants