[NaCl SDK] Fix "make debug" and "make run" on Mac.

There were a couple of issues here:
* make doesn't support spaces, so you have to escape them.
* xterm doesn't exist on mac, use osascript to create a new Terminal window instead.

Also, use the correct basename/default paths for Chrome on each platform.

BUG=322549
R=sbc@chromium.org

Review URL: https://codereview.chromium.org/86053005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@238245 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 48 insertions, 23 deletions

diff --git a/native_client_sdk/src/tools/common.mk b/native_client_sdk/src/tools/common.mk
index 9b7d9c2..82c0c97 100644
--- a/native_client_sdk/src/tools/common.mk
+++ b/native_client_sdk/src/tools/common.mk
@@ -473,9 +473,14 @@ CHROME_PATH ?= $(shell $(GETOS) --chrome 2> $(DEV_NULL))
 #
 # Verify we can find the Chrome executable if we need to launch it.
 #
+
+NULL :=
+SPACE := $(NULL) # one space after NULL is required
+CHROME_PATH_ESCAPE := $(subst $(SPACE),\ ,$(CHROME_PATH))
+
 .PHONY: check_for_chrome
 check_for_chrome:
-ifeq (,$(wildcard $(CHROME_PATH)))
+ifeq (,$(wildcard $(CHROME_PATH_ESCAPE)))
 	$(warning No valid Chrome found at CHROME_PATH=$(CHROME_PATH))
 	$(error Set CHROME_PATH via an environment variable, or command-line.)
 else
@@ -487,15 +492,16 @@ PAGE_TC_CONFIG ?= "$(PAGE)?tc=$(TOOLCHAIN)&config=$(CONFIG)"
 .PHONY: run
 run: check_for_chrome all $(PAGE)
 	$(RUN_PY) -C $(CURDIR) -P $(PAGE_TC_CONFIG) \
-	    $(addprefix -E ,$(CHROME_ENV)) -- $(CHROME_PATH) $(CHROME_ARGS) \
-	    --no-sandbox --register-pepper-plugins="$(PPAPI_DEBUG),$(PPAPI_RELEASE)"
+	    $(addprefix -E ,$(CHROME_ENV)) -- $(CHROME_PATH_ESCAPE) \
+	    $(CHROME_ARGS) --no-sandbox \
+	    --register-pepper-plugins="$(PPAPI_DEBUG),$(PPAPI_RELEASE)"
 
 .PHONY: run_package
 run_package: check_for_chrome all
 	@echo "$(TOOLCHAIN) $(CONFIG)" > $(CURDIR)/run_package_config
-	$(CHROME_PATH) --load-and-launch-app=$(CURDIR) $(CHROME_ARGS)
+	$(CHROME_PATH_ESCAPE) --load-and-launch-app=$(CURDIR) $(CHROME_ARGS)
 
-GDB_ARGS += -D $(TC_PATH)/$(OSNAME)_x86_newlib/bin/$(SYSARCH)-nacl-gdb
+GDB_ARGS += -D $(TC_PATH)/$(OSNAME)_x86_newlib/bin/x86_64-nacl-gdb
 GDB_ARGS += -D --eval-command="nacl-manifest $(abspath $(OUTDIR))/$(TARGET).nmf"
 GDB_ARGS += -D $(GDB_DEBUG_TARGET)
 
@@ -503,8 +509,8 @@ GDB_ARGS += -D $(GDB_DEBUG_TARGET)
 debug: check_for_chrome all $(PAGE)
 	$(RUN_PY) $(GDB_ARGS) \
 	    -C $(CURDIR) -P $(PAGE_TC_CONFIG) \
-	    $(addprefix -E ,$(CHROME_ENV)) -- $(CHROME_PATH) $(CHROME_ARGS) \
-	    --enable-nacl-debug \
+	    $(addprefix -E ,$(CHROME_ENV)) -- $(CHROME_PATH_ESCAPE) \
+	    $(CHROME_ARGS) --enable-nacl-debug \
 	    --register-pepper-plugins="$(PPAPI_DEBUG),$(PPAPI_RELEASE)"
 
 .PHONY: serve
diff --git a/native_client_sdk/src/tools/getos.py b/native_client_sdk/src/tools/getos.py
index 70971a6..7a72c80 100755
--- a/native_client_sdk/src/tools/getos.py
+++ b/native_client_sdk/src/tools/getos.py
@@ -20,7 +20,11 @@ import oshelpers
 
 
 SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
-CHROME_EXE_BASENAME = 'google-chrome'
+CHROME_DEFAULT_PATH = {
+  'win': r'c:\Program Files (x86)\Google\Chrome\Application\chrome.exe',
+  'mac': '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome',
+  'linux': '/usr/bin/google-chrome',
+}
 
 
 if sys.version_info < (2, 6, 0):
@@ -102,18 +106,27 @@ def GetSystemArch(platform):
   return arch
 
 
-def GetChromePath():
+def GetChromePath(platform):
+  # If CHROME_PATH is defined and exists, use that.
   chrome_path = os.environ.get('CHROME_PATH')
   if chrome_path:
     if not os.path.exists(chrome_path):
       raise Error('Invalid CHROME_PATH: %s' % chrome_path)
-  else:
-    chrome_path = oshelpers.FindExeInPath(CHROME_EXE_BASENAME)
-    if not chrome_path:
-      raise Error('CHROME_PATH is undefined, and %s not found in PATH.' %
-                  CHROME_EXE_BASENAME)
+    return os.path.realpath(chrome_path)
+
+  # Otherwise look in the PATH environment variable.
+  chrome_path = CHROME_DEFAULT_PATH[platform]
+  basename = os.path.basename(chrome_path)
+  chrome_path = oshelpers.FindExeInPath(basename)
+  if chrome_path:
+    return os.path.realpath(chrome_path)
+
+  # Finally, try the default paths to Chrome.
+  if os.path.exists(chrome_path):
+    return os.path.realpath(chrome_path)
 
-  return os.path.realpath(chrome_path)
+  raise Error('CHROME_PATH is undefined, and %s not found in PATH, nor %s.' % (
+              basename, chrome_path))
 
 
 def GetNaClArch(platform):
@@ -126,7 +139,7 @@ def GetNaClArch(platform):
 
   # On linux the nacl arch matches to chrome arch, so we inspect the chrome
   # binary using objdump
-  chrome_path = GetChromePath()
+  chrome_path = GetChromePath(platform)
 
   # If CHROME_PATH is set to point to google-chrome or google-chrome
   # was found in the PATH and we are running on UNIX then google-chrome
@@ -239,7 +252,7 @@ def main(args):
   elif options.nacl_arch:
     out = GetNaClArch(platform)
   elif options.chrome:
-    out = GetChromePath()
+    out = GetChromePath(platform)
   elif options.helper:
     out = GetHelperPath(platform)
   elif options.irtbin:
diff --git a/native_client_sdk/src/tools/run.py b/native_client_sdk/src/tools/run.py
index e2af359..fb99b6d 100755
--- a/native_client_sdk/src/tools/run.py
+++ b/native_client_sdk/src/tools/run.py
@@ -67,11 +67,15 @@ def main(args):
 
   # If any debug args are passed in, assume we want to debug
   if options.debug:
-    if getos.GetPlatform() != 'win':
+    if getos.GetPlatform() == 'linux':
       cmd = ['xterm', '-title', 'NaCl Debugger', '-e']
+      cmd += options.debug
+    elif getos.GetPlatform() == 'mac':
+      cmd = ['osascript', '-e',
+             'tell application "Terminal" to do script "%s"' %
+                 ' '.join(r'\"%s\"' % x for x in options.debug)]
     else:
       cmd = []
-    cmd += options.debug
     print 'Starting debugger: ' + ' '.join(cmd)
     debug_process = subprocess.Popen(cmd, env=env)
   else:
diff --git a/native_client_sdk/src/tools/tests/getos_test.py b/native_client_sdk/src/tools/tests/getos_test.py
index 99ffb95..b687d91 100755
--- a/native_client_sdk/src/tools/tests/getos_test.py
+++ b/native_client_sdk/src/tools/tests/getos_test.py
@@ -71,21 +71,23 @@ class TestGetos(TestCaseExtended):
     """honors CHROME_PATH environment."""
     with mock.patch.dict('os.environ', {'CHROME_PATH': '/dummy/file'}):
       expect = "Invalid CHROME_PATH.*/dummy/file"
+      platform = getos.GetPlatform()
       if hasattr(self, 'assertRaisesRegexp'):
         with self.assertRaisesRegexp(getos.Error, expect):
-          getos.GetChromePath()
+          getos.GetChromePath(platform)
       else:
         # TODO(sbc): remove this path once we switch to python2.7 everywhere
-        self.assertRaises(getos.Error, getos.GetChromePath)
+        self.assertRaises(getos.Error, getos.GetChromePath, platform)
 
   def testGetChromePathCheckExists(self):
     """checks that existence of explicitly CHROME_PATH is checked."""
     mock_location = '/bin/ls'
-    if getos.GetPlatform() == 'win':
+    platform = getos.GetPlatform()
+    if platform == 'win':
       mock_location = 'c:\\nowhere'
     with mock.patch.dict('os.environ', {'CHROME_PATH': mock_location}):
       with mock.patch('os.path.exists') as mock_exists:
-        chrome = getos.GetChromePath()
+        chrome = getos.GetChromePath(platform)
         self.assertEqual(chrome, mock_location)
         mock_exists.assert_called_with(chrome)