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

run: when disconnected from PTY forwarder, exit event loop if not --wait #32954

Merged
merged 3 commits into from
May 21, 2024

Conversation

YHNdnzj
Copy link
Member

@YHNdnzj YHNdnzj commented May 21, 2024

Follow-up for ade0789

The change in behavior was partly intentional, as I think if both --wait and --pty are used, manually disconnecting from PTY forwarder should not result in systemd-run exiting with "Finished with ..." log. But we should check for --wait here.

Closes #32953

@YHNdnzj YHNdnzj added the run label May 21, 2024
@github-actions github-actions bot added util-lib please-review PR is ready for (re-)review by a maintainer labels May 21, 2024
@poettering poettering added good-to-merge/with-minor-suggestions and removed please-review PR is ready for (re-)review by a maintainer labels May 21, 2024

This comment was marked as off-topic.

@YHNdnzj YHNdnzj added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels May 21, 2024
src/run/run.c Show resolved Hide resolved
@yuwata yuwata added good-to-merge/with-minor-suggestions and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels May 21, 2024
Follow-up for ade0789

The change in behavior was partly intentional, as I think
if both --wait and --pty are used, manually disconnecting
from PTY forwarder should not result in systemd-run exiting
with "Finished with ..." log. But we should check for
--wait here.

Closes systemd#32953
@YHNdnzj
Copy link
Member Author

YHNdnzj commented May 21, 2024

Beefed up man/systemd-run a bit more, PTAL.

@YHNdnzj YHNdnzj added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels May 21, 2024
Comment on lines +328 to +331
<para>This option will result in <command>systemd-run</command> synchronously waiting for
the transient service to terminate, similar to specifying <option>--wait</option>. If specified
along with <option>--wait</option>, <command>systemd-run</command> won't exit when manually disconnecting
from the pseudo TTY device.</para>
Copy link
Member

Choose a reason for hiding this comment

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

The tenses are messed up here. But the surrounding paragraphs are also not great, so let's merge this, and work on the grammar later.

@bluca bluca merged commit 27b5ac3 into systemd:main May 21, 2024
45 of 49 checks passed
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label May 21, 2024
@YHNdnzj YHNdnzj deleted the run-forwarder-exit branch May 21, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

run/ptyfwd: doesn't exit event loop on ^[ key combo anymore
5 participants