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

Standardise pkExpect process execution with shell option #9

Open
emmacasolin opened this issue Aug 10, 2022 · 2 comments
Open

Standardise pkExpect process execution with shell option #9

emmacasolin opened this issue Aug 10, 2022 · 2 comments
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity

Comments

@emmacasolin
Copy link

Specification

Our pkExec and pkSpawn process execution utilities were standardised in MatrixAI/Polykey#436, however, pkExpect was left unchanged due to the lack of shell option for nexpect.spawn. nexpect.spawn always spawns a child process with the default shell: false, however, for this to work with our docker integration tests we need to be able to set the shell option to true. Doing this would require either forking nexpect or switching to a different library.

Additional context

Tasks

  1. Expose the shell option when calling nexpect.spawn or equivalent
  2. Create pkExpectWithoutShell and pkExpectWithShell variants
@emmacasolin emmacasolin added the development Standard development label Aug 10, 2022
@CMCDragonkai
Copy link
Member

A quick PR upstream can also be done as it is unlikely to change many things.

Can you copy paste the code snippet relevant to be changed here.

@emmacasolin
Copy link
Author

The shell option needs to be added to the context object: https://github.com/nodejitsu/nexpect/blob/master/lib/nexpect.js#L365-L376

And this needs to be passed through to spawn: https://github.com/nodejitsu/nexpect/blob/master/lib/nexpect.js#L257-L260

The type of the options here would also need shell to be added: https://github.com/nodejitsu/nexpect/blob/master/lib/nexpect.js#L344

@CMCDragonkai CMCDragonkai changed the title Standardise pkExpect process execution Standardise pkExpect process execution with shell option Aug 11, 2022
@CMCDragonkai CMCDragonkai self-assigned this Jul 10, 2023
@CMCDragonkai CMCDragonkai added the r&d:polykey:supporting activity Supporting core activity label Jul 10, 2023
@CMCDragonkai CMCDragonkai transferred this issue from MatrixAI/Polykey Aug 11, 2023
@CMCDragonkai CMCDragonkai removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity
Development

No branches or pull requests

2 participants