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

always submit external commands in list form #2595

Merged
merged 1 commit into from
Jan 25, 2014

Conversation

rolandwalker
Copy link
Contributor

For safety.

  • This is a step toward reworking system_command.rb so that
    invocations are done in list form on the back end, avoiding
    surprises from quoting and shell metacharacters.
  • There is one transitional hack here: the _quote method in
    Cask::SystemCommand is modified to avoid double-quoting. The
    _quote method itself will go away in a future revision when
    only list-forms are used.
  • Casks using system are not touched. It seems natural to
    address that when creating the DSL for after_install/before_install.

For safety.
- This is a step toward reworking system_command.rb so that
invocations are done in list form on the back end, avoiding
surprises from quoting and shell metacharacters.
- There is one transitional hack here: the _quote method in
Cask::SystemCommand is modified to avoid double-quoting.  The
_quote method itself will go away in a future revision when
only list-forms are used.
- Casks using system are not touched. It seems natural to
address that when creating the DSL for after_install/before_install.
rolandwalker added a commit that referenced this pull request Jan 25, 2014
always submit external commands in list form
@rolandwalker rolandwalker merged commit e55533f into Homebrew:master Jan 25, 2014
@rolandwalker rolandwalker deleted the list_args_to_system branch January 25, 2014 12:56
@phinze
Copy link
Contributor

phinze commented Jan 25, 2014

Ah yes I've been moving in this direction with code I've touched too. Glad to see you agree, and good catches on the ones you changed. 🍞

@rolandwalker
Copy link
Contributor Author

good, I have a series of 3 or 4 more PRs that will make us very robust on external commands. looks like @vmrob is also working on some of the same lines.

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

Successfully merging this pull request may close these issues.

2 participants