820285 - bootstrap.py fails on OS X if no point release version

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

STR:

1) Install OS X 10.8.0
2) Run bootstrap.py

Traceback (most recent call last):
  File "python/mozboot/bin/bootstrap.py", line 138, in <module>
    sys.exit(main(sys.argv))
  File "python/mozboot/bin/bootstrap.py", line 129, in main
    dasboot.bootstrap()
  File "python/mozboot/mozboot/bootstrap.py", line 61, in bootstrap
    major, minor, point = map(int, platform.mac_ver()[0].split('.'))
ValueError: need more than 2 values to unpack

unknowne4ce8f42347a:mozilla-central-hg gps$ python
Python 2.7.2 (default, Jun 20 2012, 16:23:33) 
[GCC 4.2.1 Compatible Apple Clang 4.0 (tags/Apple/clang-418.0.60)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform
>>> platform.mac_ver()
('10.8', ('', '', ''), 'x86_64')

Should be a trivial fix.

Comment 1

[Jorge Luis Mendez [:jlmendezbonini]]

Attachment: Keeps OSX version as a string instead of converting it to ints.

Instead of converting the os version to integers, I decided to keep it as a string. All the subsequent version comparisons are also done with strings within osx.py

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 691536 [details] [diff] [review]
Keeps OSX version as a string instead of converting it to ints.

Review of attachment 691536 [details] [diff] [review]:
-----------------------------------------------------------------

String comparison will fail when OS X 10.10 is released because "10.10" < "10.7"

Please retain the integer conversion and deal with the case where len(platform.mac_ver()[0]) < 3.

FWIW, there exists distutils.version.StrictVersion to perform version comparisons. However, I believe this isn't available in Python 2.5 and the bootstrapper currently needs to support 2.5+. If I'm wrong and this is available in Python 2.5 core, I'll accept a patch that uses StrictVersion.

Comment 3

[Jorge Luis Mendez [:jlmendezbonini]]

Hi Greg,

Thanks for the feedback and sorry for that dumb mistake. 

For some reason Python2.5 docs don't mention distutils.version.StrictVersion but it's indeed available (http://pydoc.org/get.cgi/usr/local/lib/python2.5/distutils/version.py). It's even the exact same implementation as Python2.7.3.  Also, I noticed that you were already using distutils.version.StrictVersion within osx.py.

Comment 4

[Gregory Szorc [:gps]]

So, I think the options are:

a) Switch everything to use StrictVersion
b) Handle when len(platform.mac_ver()[0])) < 3

a is more robust but more work.

Your call.

Comment 5

[Jorge Luis Mendez [:jlmendezbonini]]

Attachment: Use distutils.version.StrictVersion in OSX version tests.

I implemented the solution using distutils.version.StrictVersion and I'll appreciate your feedback. However, it's important to note that while testing with Python2.5 I ran into some syntax errors in python/mozboot/bin/bootstrap.py, python/mozboot/mozboot/bootstrap.py, and python/mozboot/mozboot/osx.py that are not part of this patch. I did manage to test with Python2.6 and Python2.7.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 691598 [details] [diff] [review]
Use distutils.version.StrictVersion in OSX version tests.

Review of attachment 691598 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!

I was wrong about Python 2.5. The minimum we need to support is Python 2.6.1 (which is shipped on OS X 10.6).

I'll check this in for you.

Comment 7

[Jorge Luis Mendez [:jlmendezbonini]]

Awesome. Thanks Greg.

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6445a79d9795

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6445a79d9795