387489 - talos should auto-detect the OS

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Talos relies on the "OS" configuration variable in config.py to tell it what OS it's running on, but it's easy to figure that out from sys.platform, os.name, or platform.system().  Talos should automatically detect the OS via one of those.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: replaces config.OS with platform.system()

Here's a patch that replaces references to config.OS with references to platform.system().  

platform.system() seems more compatible with the way we currently differentiate between OSes than sys.platform (which returns "linux2" for my version of Linux and presumably returns "Linux1" for some versions) and os.name (which returns "posix" for both Linux and Mac).

Per section 14.12.1 "Cross Platform" in the Python Library Reference <http://docs.python.org/lib/node442.html>, this method returns "the system/OS name, e.g. 'Linux', 'Windows', or 'Java'".  On my Mac OS X 10.4.10 installation it returns "Darwin".

Section 14.12 "platform" <http://docs.python.org/lib/module-platform.html> says the module is new in Python version 2.3, which is less than (for Windows) or equal to (for Mac, per the patch in bug 384341) the minimum version of Python Talos requires, so it should be available on all systems Talos runs on.

Comment 2

[Robert Helmer [:rhelmer]]

Comment on attachment 271592 [details] [diff] [review]
patch v1: replaces config.OS with platform.system()

this looks good to me; what do you think alice?

Comment 3

[alice nodelman [:alice] [:anode]]

My only concern would be, mostly due to my unfamiliarity with platform.system, that we are using exact equals to identify linux, windows, etc.  Will it always report back as "Linux" for various flavors of linux?  Will Vista and XP report as "Windows"?

The doc states: 
"
The output is intended to be human readable rather than machine parseable. It may look different on different platforms and this is intended.
"

I'm just worried that we could run into some sort of Linux/linux/bsd/Fedora mess.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

> The doc states: 
> "
> The output is intended to be human readable rather than machine parseable. It
> may look different on different platforms and this is intended.
> "

Actually, that's the description of platform.platform().  platform.system(), on the other hand, is described as follows:

"
Returns the system/OS name, e.g. 'Linux', 'Windows', or 'Java'. An empty string is returned if the value cannot be determined.
"

I should note that ffprocess_mac.py and ffprofile_mac.py from the patch in bug 384341 are very similar to the Linux versions of those files.  I wonder whether it would make more sense to use single files for all platforms and implement branching code where necessary within the files themselves.  But that's an issue for another bug.

Comment 5

[alice nodelman [:alice] [:anode]]

Comment on attachment 271592 [details] [diff] [review]
patch v1: replaces config.OS with platform.system()

This looks good to me, after the clarification of my misreading of the docs.

We may want to add some failure case for if/when platform.system() returns  an empty string.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Checking in ffprocess.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess.py,v  <--  ffprocess.py
new revision: 1.3; previous revision: 1.2
done
Checking in ffprofile.py;
/cvsroot/mozilla/testing/performance/talos/ffprofile.py,v  <--  ffprofile.py
new revision: 1.3; previous revision: 1.2
done
Checking in tp.py;
/cvsroot/mozilla/testing/performance/talos/tp.py,v  <--  tp.py
new revision: 1.6; previous revision: 1.5
done
Checking in config.py;
/cvsroot/mozilla/testing/performance/talos/config.py,v  <--  config.py
new revision: 1.5; previous revision: 1.4
done

Comment 7

[(not currently active) Ted Mielczarek]

Mass move of Core:Testing bugs to mozilla.org:Release Engineering:Talos. Filter on RelEngTalosMassMove to ignore.