Skip to content

Use Subprocess for process management #410

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

cmcgee1024
Copy link
Member

No description provided.

if let env {
process.environment = env
}
// TODO: figure out how to set the process group
Copy link
Contributor

Choose a reason for hiding this comment

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

The PlatformOptions structure has some APIs you might be able to use for this.

I'm not that familiar with tcsetpgrp -- would setgid be sufficient here? (there's a PlatformOptions.processGroupID API for that)

Alternatively, there's a PlatformOptions.preSpawnProcessConfigurator closure on Linux/Android/BSD which runs between fork and exec, where you could do this. (But on Darwin, posix_spawn is used, and PlatformOptions.preSpawnProcessConfigurator runs before fork so I don't think you'd have a good place to do it)

Worth reaching out to @iCharlesHu to see if tcsetpgrp might be something worth having dedicated API for.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was here because Foundation Process would automatically assign a new process group to child processes, and we wanted to be able to have the terminal control the child process that is in the foreground. With the default behaviour of swift-subprocess being that the child is in the current process's group this is no longer needed, which is a nice bonus of this new API.

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.

2 participants