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

--shell 'busybox sh', 'zsh --emulate posix', 'bash --posix' #410

Closed
mralusw opened this issue Jul 22, 2021 · 5 comments · Fixed by #436
Closed

--shell 'busybox sh', 'zsh --emulate posix', 'bash --posix' #410

mralusw opened this issue Jul 22, 2021 · 5 comments · Fixed by #436
Labels

Comments

@mralusw
Copy link

mralusw commented Jul 22, 2021

It seems that --shell needs to be a single word -- which makes it impossible, amongst others, to test busybox sh or zsh --emulate sh or bash --posix.

Sure, for busybox you can symlink, and for zsh / bash, I suppose you can create a stub script (or small C wrapper) that exec's the real shell with prepended options. But of course that perturbs the measurement.

It might make sense to do simple whitespace splitting of the provided shell string. At most, we might have to handle '\ ' (to allow for shell executables with whitespaces in their paths). Or we could try looking up the shell in $PATH first (like it's currently done), then if that fails, try whitespace splitting.

@sharkdp
Copy link
Owner

sharkdp commented Jul 26, 2021

Thank you for reporting this. This should be supported, I agree.

It might make sense to do simple whitespace splitting of the provided shell string. At most, we might have to handle '\ ' (to allow for shell executables with whitespaces in their paths). Or we could try looking up the shell in $PATH first (like it's currently done), then if that fails, try whitespace splitting.

We can use shell-words (https://crates.io/crates/shell-words). See also: #336

@rhysd
Copy link
Contributor

rhysd commented Oct 4, 2021

I'll work on this.

@sharkdp
Copy link
Owner

sharkdp commented Oct 4, 2021

Please take note of #429 which also touches similar parts of the code base
maybe it makes sense to merge #429 first or to base your changes on this branch.

@rhysd
Copy link
Contributor

rhysd commented Oct 4, 2021

@sharkdp Ah, I've already created a PR #436 before I noticed the PR..

@rhysd
Copy link
Contributor

rhysd commented Oct 4, 2021

Can we merge #436 at first then request #429 author to merge the main branch? Since I'm not understanding what #429 is solving and #429 is in the middle of work, I feel a bit hard to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants