ignore `$SHELL` in `shellify`
authorDominik Gabi <dominik@fb.com>
Thu, 15 Sep 2016 14:57:15 +0000 (07:57 -0700)
committerFacebook Github Bot 8 <facebook-github-bot-8-bot@fb.com>
Thu, 15 Sep 2016 15:08:45 +0000 (08:08 -0700)
commit9637519ede462c78b28ca7d2866429bba57a2996
tree43f13835b4b42cd5e5f23fcbc7f3fb30a20724d0
parente69e359c24070c3b4ea2cd25f87b554e33b2a132
ignore `$SHELL` in `shellify`

Summary:
Why `$SHELL` is a bad idea:

- getenv() is not thread safe. (In practice it should work if other threads aren't calling setenv(), but still seems undesirable if we can avoid it.)
- It seems confusing for the program to have different behavior for different developers.
- Other shells besides /bin/sh may have different quoting behaviors. For instance, I don't think csh allows unescaped newlines inside single quotes.
- SHELL might be set to other non-shell-like programs in some cases. (Say, if this gets run from inside a git or mercurial hook it might end up being set to the restricted git or hg shell that only lets you run specific commands.)
Anyway, this isn't related to your diff, so nothing needs to be done for now,
but I would vote for changing this to always use /bin/sh in a future diff.

Both the C system() call and python's subprocess module appear to
always use /bin/sh and ignore $SHELL.

Reviewed By: simpkins

Differential Revision: D3867047

fbshipit-source-id: dab0e6afbe1c20ff13d9a52f212df95af425dd77
folly/Shell.h
folly/test/ShellTest.cpp