Adding comment to inform the user to set up the sandbox manually on Linux.

Adding comment to inform the user to set up the sandbox manually on Linux.

BUG=None
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220413

[tonyg, 2013-08-29 20:51:28]

lgtm

Adding to the usage comment is great, but users might not check the source
before running the script.

It would really be best if there were a way to check that the sandbox is not set
up and display a helpful error message in that case.

One way to accomplish this would be to capture chrome's stdout to a file and
search it for something about SUID. If it finds that in the stdout, then display
the stdout to the user of the script.

[pshenoy, 2013-08-29 22:17:46]

On 2013/08/29 20:51:28, tonyg wrote:
> lgtm
> 
> Adding to the usage comment is great, but users might not check the source
> before running the script.
> 
> It would really be best if there were a way to check that the sandbox is not
set
> up and display a helpful error message in that case.
> 
> One way to accomplish this would be to capture chrome's stdout to a file and
> search it for something about SUID. If it finds that in the stdout, then
display
> the stdout to the user of the script.

Added a check to see if CHROME_DEVEL_SANDBOX environment variable is set. Else
display an error message and exit the script. Please take a look.

[tonyg, 2013-08-29 22:39:19]

Thanks for making that change. Just two brief nits.

https://codereview.chromium.org/23777002/diff/5001/tools/run-bisect-manual-te...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/23777002/diff/5001/tools/run-bisect-manual-te...
tools/run-bisect-manual-test.py:117: if os.name == 'posix':
os.name == 'posix' includes Mac which does not use the SUID sandbox.

You want:
if sys.platform.startswith('linux'):

https://codereview.chromium.org/23777002/diff/5001/tools/run-bisect-manual-te...
tools/run-bisect-manual-test.py:118: if os.environ.get('CHROME_DEVEL_SANDBOX')
== None:
The style guide says not to compare to None explicitly. This should be:

if not os.environ.get('CHROME_DEVEL_SANDBOX'):

See:
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#True/False_eva...

[pshenoy, 2013-08-29 22:46:23]

https://codereview.chromium.org/23777002/diff/5001/tools/run-bisect-manual-te...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/23777002/diff/5001/tools/run-bisect-manual-te...
tools/run-bisect-manual-test.py:117: if os.name == 'posix':
On 2013/08/29 22:39:19, tonyg wrote:
> os.name == 'posix' includes Mac which does not use the SUID sandbox.
> 
> You want:
> if sys.platform.startswith('linux'):

Done.

https://codereview.chromium.org/23777002/diff/5001/tools/run-bisect-manual-te...
tools/run-bisect-manual-test.py:118: if os.environ.get('CHROME_DEVEL_SANDBOX')
== None:
On 2013/08/29 22:39:19, tonyg wrote:
> The style guide says not to compare to None explicitly. This should be:
> 
> if not os.environ.get('CHROME_DEVEL_SANDBOX'):
> 
> See:
>
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#True/False_eva...

Done.

[tonyg, 2013-08-29 22:53:53]

lgtm

[commit-bot: I haz the power, 2013-08-29 22:56:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pshenoy@chromium.org/23777002/11001

[commit-bot: I haz the power, 2013-08-29 22:58:52]

Change committed as 220413