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

Add option to run vncsession without forking and detaching #1651

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

zmudc
Copy link
Contributor

@zmudc zmudc commented Aug 2, 2023

Option is -D, which is what sshd uses for the same option.

Also added description of the new option to the vncsession man page.

Tested on top of up-to-date version 1.13.80,
commit 094496e (Merge pull request #1648 from TigerVNC/copyright) on Void Linux with runit system service manager.

Also tested on top of Fedora 38 with version 1.13.1 and systemd with Type=exec service and -D option passed to vncsession in vncsession-start.

Resolves #1649

@zmudc
Copy link
Contributor Author

zmudc commented Sep 1, 2023

The updated PR rebases the unmodified, original PR onto the current tip of the master branch.

unix/vncserver/vncsession.c Outdated Show resolved Hide resolved
unix/vncserver/vncsession.c Outdated Show resolved Hide resolved
unix/vncserver/vncsession.man.in Outdated Show resolved Hide resolved
@zmudc
Copy link
Contributor Author

zmudc commented Oct 5, 2023

After digesting everything said here and at #1649 which is the issue this PR resolves, I think the goal of this PR should be to add the -D option to vncsession for runit compatibility. So far, I have tested that it is effective on Void Linux which uses runit as the default init system. I plan to verify it is also effective with Debian's implementation of runit, which has two flavors - runit as the init system replacing systemd, and runit as the process supervisor with either systemd or sysv for the init system. This will take a little while for me to do, and when tests with runit on Debian are complete I will update the PR if this seems to be a useful addition applicable to more than just Void Linux. If this is only needed for Void Linux, I will just close the PR as something I do not plan to pursue further since the maintenance burden probably outweighs the benefit of this PR in that case.

@zmudc zmudc force-pushed the master branch 3 times, most recently from 3b2670b to fda020b Compare October 31, 2023 17:19
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getopt logic still needs some more massaging, but we're close to something ready to be merged.

unix/vncserver/vncsession.c Outdated Show resolved Hide resolved
unix/vncserver/vncsession.c Outdated Show resolved Hide resolved
unix/vncserver/vncsession.man.in Outdated Show resolved Hide resolved
unix/vncserver/vncsession.c Outdated Show resolved Hide resolved
unix/vncserver/vncsession.man.in Outdated Show resolved Hide resolved
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except for one if-clause I don't quite understand.

Option is -D, which is what sshd uses for the same option.

Also add description of the new option to the vncsession
man page.

Tested on Void Linux using the new option, also tested on
Fedora without using the new option.

Resolves TigerVNC#1649
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. This looks good to be merged now. Thanks for all your work!

@CendioOssman CendioOssman merged commit fc066eb into TigerVNC:master Dec 28, 2023
12 checks passed
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.

vncsession: Please follow the recommendations of daemon(7) or at least provide it as an option
2 participants