703607 - Mozmill 2.1 builds are failing to start Firefox as .app on OS X

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

There is a regression in Mozmill 2.1 which prevents us from starting the application directly. Using the full path down to the binary works.

$ mozmill -b /Applications/Firefox/Nightly.app/ -t mutt/mutt/tests/js/test_assertions.js 
ERROR | Traceback (most recent call last):
ERROR |   File "/Volumes/data/envs/mozmill-2/bin/mozmill", line 8, in <module>
ERROR | 
ERROR | load_entry_point('mozmill==2.0rc1', 'console_scripts', 'mozmill')()
ERROR |   File "/Volumes/data/code/tools/mozmill-2.0/mozmill/mozmill/__init__.py", line 678, in cli
ERROR | 
ERROR | CLI(args).run()
ERROR |   File "/Volumes/data/code/tools/mozmill-2.0/mozmill/mozmill/__init__.py", line 643, in run
ERROR | 
ERROR | runner = self.create_runner()
ERROR |   File "/Volumes/data/envs/mozmill-2/lib/python2.6/site-packages/mozrunner-4.0-py2.6.egg/mozrunner/runner.py", line 412, in create_runner
ERROR | 
ERROR | return self.runner_class.create(**self.runner_args())
ERROR |   File "/Volumes/data/envs/mozmill-2/lib/python2.6/site-packages/mozrunner-4.0-py2.6.egg/mozrunner/runner.py", line 73, in create
ERROR | 
ERROR | clean_profile=clean_profile, process_class=process_class)
ERROR |   File "/Volumes/data/envs/mozmill-2/lib/python2.6/site-packages/mozrunner-4.0-py2.6.egg/mozrunner/runner.py", line 296, in __init__
ERROR | 
ERROR | Runner.__init__(self, profile, **kwargs)
ERROR |   File "/Volumes/data/envs/mozmill-2/lib/python2.6/site-packages/mozrunner-4.0-py2.6.egg/mozrunner/runner.py", line 85, in __init__
ERROR | 
ERROR | self.binary = self.__class__.get_binary(binary)
ERROR |   File "/Volumes/data/envs/mozmill-2/lib/python2.6/site-packages/mozrunner-4.0-py2.6.egg/mozrunner/runner.py", line 316, in get_binary
ERROR | 
ERROR | return Runner.get_binary(binary)
ERROR |   File "/Volumes/data/envs/mozmill-2/lib/python2.6/site-packages/mozrunner-4.0-py2.6.egg/mozrunner/runner.py", line 129, in get_binary
ERROR | 
ERROR | return os.path.join(binary, 'Contents/MacOS/%s-bin' % cls.names[0])
ERROR | IndexError
ERROR | :
ERROR | list index out of range
ERROR |

Comment 1

[Jeff Hammel]

Attachment: keep proper scope

So I think this should fix this for Mac.  Really we're getting in trouble for having too many default ways of finding the browser and none of the flow is documented.  Henrik, could you test?

Comment 2

[Jeff Hammel]

see also bug 703646

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 575502 [details] [diff] [review]
keep proper scope

>-        elif mozinfo.isMac and binary.find('Contents/MacOS/') == -1:
>+        elif mozinfo.isMac and 'Contents/MacOS/' not in binary and cls.names:
>+            # default to first class name
>             return os.path.join(binary, 'Contents/MacOS/%s-bin' % cls.names[0])

Haven't tested yet. But I also don't think that this is the right way to go. When the users specifies an app bundle we already have a valid path. The only detail we are missing is the binary in the bundle. This one is referenced in 'Contents/Info.plist@binary'. We could use http://docs.python.org/dev/library/plistlib.html to read its value and append it to the path of the bundle path. There would be no need to magically guess how the binary is named, and we could get rid of this strange cls.names thingy.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Here the code-snippet which should work:

import plistlib
info = plistlib.readPlist("%s/Contents/Info.plist" % binary)
return os.path.join(binary, "Contents/MacOS/%s" % info['CFBundleExecutable'])

Comment 5

[Jeff Hammel]

I would recommend we take the fix from https://bug703607.bugzilla.mozilla.org/attachment.cgi?id=575502 since it does keep the proper class scope and avoid sideeffects.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Which side-effects? With the patch you propose, we would still guess how the binary of the application bundle would be named. Means we probably are not able to correctly handle other XULrunner applications.

Comment 7

[Jeff Hammel]

The current architecture will not work as we call the get_binary method as applied to the ABC Runner whereas the logic is contingent on being an implementor class with names.  The patch also generalizes the environment variable pattern.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I'm still puzzled because I probably don't understand all the new logic which has been implemented since Mozmill 1.5. So you should probably ask for review from Clint or someone else. My feedback stays and I would kinda see it discussed in a follow-up review request.

Comment 9

[Jeff Hammel]

I'm not working on this bug at the moment.  We should land whatever we're landing for bug 703646 first.

Comment 10

[Jeff Hammel]

with the elimination of binary finding logic, this is no longer an issue: bug 703646
However, with it we've lost the ability to find binaries in mac subpaths thingies: bug 711520