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

Allow arbitrary shells #67

Closed
brejoc opened this issue Jun 7, 2013 · 8 comments
Closed

Allow arbitrary shells #67

brejoc opened this issue Jun 7, 2013 · 8 comments
Labels

Comments

@brejoc
Copy link

@brejoc brejoc commented Jun 7, 2013

Make sure users can determine which local shell is used when executing commands.


Was: Only Bash as shell?

I've only had a quick peek at the code, but I think the bash shell is hardcoded in the runner. Would be nice to let the user define the the shell in tasks.py, since some shells don't share the same syntax.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2013

That's totally on the roadmap, yea. I'll mutate this issue's title/desc to reflect that it's an intended line item, so to speak :) Thanks!

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 8, 2013

N.B. this is only hardcoded in one spot, invoke/runner.py; it shows up in our vendorized pexpect but all of those bash mentions are in docstrings, so not a problem.

@brejoc
Copy link
Author

@brejoc brejoc commented Jun 17, 2013

Yes, that was the place I saw it. The best place to make this editable would be the tasks.py in my opinion. What do you think? Just like make does it:

SHELL := /bin/bash
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 25, 2013

I've been working up to actual config, my general idea is like so:

  • Bottom level is a run kwarg, like echo and friends, so it can be overridden per-execute in any use case.
  • Mid level is a Context object which acts as a config holder & allows one to pass configs around.
    • This is implemented, though the "edit it at the Python level" angle isn't beautiful - it's just dict attributes on Context objects, and I need to figure out how best to 'merge' CLI level settings (below) with in-task-module settings (what you want).
  • Top level is CLI options, i.e. inv --shell=/bin/zsh mytask. These require use of contextualized tasks to take effect - again see echo.

Adding a CLI flag is easy, just follow #32 / #27 re: --echo; having a useful task-module-level way of merging stuff might need more thinking.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Mar 20, 2016

Ran into an internal need for this in #152 (see final comment), so, poking now.

The tl;dr of current state of this issue:

  • Configuration has been in for a long time now (this ticket is old) so not a problem
  • PTY-driven run sets its own shell, which is /bin/bash, so that's easily updated
  • non-PTY-driven run uses subprocess' own hardcoded default, /bin/sh, but that should be overrideable via the executable kwarg
bitprophet added a commit that referenced this issue Mar 20, 2016
bitprophet added a commit that referenced this issue Mar 20, 2016
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Mar 20, 2016

All done. Simply configure run.shell via any config method, or say run('command', shell='/bin/fish') or whatnot at runtime.

@bitprophet bitprophet closed this Mar 20, 2016
@presidento
Copy link
Contributor

@presidento presidento commented Mar 21, 2016

Great, thank you @bitprophet!
(Didn't you want to add it to the 0.12.3 milestone?)

@frol
Copy link

@frol frol commented Apr 11, 2016

Just for your info, the implementation of this feature breaks Invoke on Windows: #345.

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

Successfully merging a pull request may close this issue.

None yet
4 participants