Last Comment Bug 820285 - fails on OS X if no point release version
: fails on OS X if no point release version
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla20
Assigned To: Jorge Luis Mendez [:jlmendezbonini]
: Gregory Szorc [:gps]
Depends on:
  Show dependency treegraph
Reported: 2012-12-11 01:01 PST by Gregory Szorc [:gps]
Modified: 2012-12-13 08:02 PST (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Keeps OSX version as a string instead of converting it to ints. (6.86 KB, patch)
2012-12-12 14:44 PST, Jorge Luis Mendez [:jlmendezbonini]
gps: review-
Details | Diff | Splinter Review
Use distutils.version.StrictVersion in OSX version tests. (7.21 KB, patch)
2012-12-12 17:05 PST, Jorge Luis Mendez [:jlmendezbonini]
gps: review+
Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2012-12-11 01:01:07 PST

1) Install OS X 10.8.0
2) Run

Traceback (most recent call last):
  File "python/mozboot/bin/", line 138, in <module>
  File "python/mozboot/bin/", line 129, in main
  File "python/mozboot/mozboot/", 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] 2012-12-12 14:44:35 PST
Created attachment 691536 [details] [diff] [review]
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
Comment 2 Gregory Szorc [:gps] 2012-12-12 14:52:55 PST
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] 2012-12-12 16:48:24 PST
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 ( It's even the exact same implementation as Python2.7.3.  Also, I noticed that you were already using distutils.version.StrictVersion within
Comment 4 Gregory Szorc [:gps] 2012-12-12 17:02:50 PST
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] 2012-12-12 17:05:22 PST
Created attachment 691598 [details] [diff] [review]
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/, python/mozboot/mozboot/, and python/mozboot/mozboot/ that are not part of this patch. I did manage to test with Python2.6 and Python2.7.
Comment 6 Gregory Szorc [:gps] 2012-12-12 17:35:07 PST
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] 2012-12-12 17:38:36 PST
Awesome. Thanks Greg.
Comment 9 Ed Morley [:emorley] 2012-12-13 08:02:51 PST

Note You need to log in before you can comment on or make changes to this bug.