945869 - Provide more granular OS version information in mozinfo

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

It would be nice if we could disable tests on a more granular level (i.e. only Windows 8) instead of across an entire platform. That information is not currently exposed in a usable way.

Comment 1

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

Does mozinfo.version not work for you?

{'version': '6.1.7601', 'os': 'win', 'service_pack': 'Service Pack 1', 'bits': 64, 'processor': 'x86_64'}

We might have to do something like startswith('6.1') but as I can see the information is available in mozinfo.

On the other hand we could split out the build number so we get something like:

{'version': '6.1', 'build': '7601', 'os': 'win', 'service_pack': 'Service Pack 1', 'bits': 64, 'processor': 'x86_64'}

Comment 2

[(not currently active) Ted Mielczarek]

This info is probably not super useful as it is currently formatted, I think we could fix it a little bit and make it useful. I think your suggestion to split out the build number would be helpful for Windows. On Linux we apparently stick the distribution name+version in the version field which seems odd:
>>> mozinfo.version
'Ubuntu 12.04'

and on OS X we do the same, which is odd:
>>> mozinfo.version
'OS X 10.8.5'

If we make these a little more sane I think this could be useful. I might also suggestion renaming "version" to "os_version" to be clearer. Here's my concrete suggestion:

For Windows, drop the build number so we have simply:
{'os': 'win', 'os_version': '6.1', ... }

For Linux, split the distro name off to a separate field so we have:
{'os': 'linux', 'os_version': '12.04', 'linux_distro': 'Ubuntu', ...}

For OS X, split off the patch level and drop the "OS X" bit so we have:
{'os': 'mac', 'os_version': '10.8', ...}

If we need patchlevel information we can split that to a separate field, but that seems less likely to be useful.

Ryan: does that sound sane?

Comment 3

[(not currently active) Ted Mielczarek]

Also, we'll need to be careful about how we treat this info for Android/B2G test harnesses, since this will be the info for the *host* system running the test harness script, not the actual system under test.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

os_version as described sounds perfect, thanks!

Comment 6

[Marco Castelluccio [:marco]]

Attachment: WIP

I haven't tested yet on Mac and Windows and I guess I need to fix all the places where mozinfo.version is used.

Comment 7

[Marco Castelluccio [:marco]]

Looks like the two users of mozinfo.version are:

 - http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/moztest/moztest/results.py#24
   Here I think we can simply use mozinfo.os_version

 - http://mxr.mozilla.org/mozilla-central/source/testing/tps/tps/testrunner.py#456
   Here I'm not sure, because I don't know anything about TPS. I guess it doesn't really need the detailed information contained in mozinfo.version and it could work with mozinfo.os_version.

Comment 8

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

What about keeping mozinfo.version to be backward compatible, and introduce new properties for more detailed information? Thing is we have other customers of this code outside of m-c too.

Comment 9

[Marco Castelluccio [:marco]]

Attachment: WIP v2

(In reply to Henrik Skupin (:whimboo) from comment #8)
> What about keeping mozinfo.version to be backward compatible, and introduce
> new properties for more detailed information? Thing is we have other
> customers of this code outside of m-c too.

Sounds good to me.

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 8405652 [details] [diff] [review]
WIP v2

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

This looks fine. My only reservation is what I said in comment 3: we need to be careful about this because it won't be correct for Android/B2G usage. I suspect we'll always be writing conditions like 'skip-if = os == "mac" && os_version == "10.9"', which will be okay because "os" will be correct at least.

We should get a followup filed to get os_version set correctly for Android/B2G.

::: testing/mozbase/mozinfo/mozinfo/mozinfo.py
@@ +44,5 @@
>          processor = os.environ.get("PROCESSOR_ARCHITEW6432", processor)
>      else:
>          processor = os.environ.get('PROCESSOR_ARCHITECTURE', processor)
>      system = os.environ.get("OS", system).replace('_', ' ')
> +    (mayor, minor, _, _, service_pack, _, _, _, _) = os.sys.getwindowsversion()

spelling nit: "major"

Comment 11

[Marco Castelluccio [:marco]]

Looks like the patch works ok if you run mochitests via make, but not via mach.
For example, I've added a test with: run-if = os == "mac" && os_version == "10.9". The test isn't added to all-tests.json and so mach doesn't run it.

Comment 12

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 13

[Marco Castelluccio [:marco]]

Attachment: Tests

I wrote these tests for local testing purposes, I don't know if we want to check them in.

Comment 14

[Marco Castelluccio [:marco]]

Attachment: Patch

This fixes the problem by using mozinfo in the TreeMetadataEmitter class.

Comment 15

[Marco Castelluccio [:marco]]

I guess I need to pass the json path to find_and_update_from_json, because a try run that I've spun failed with "mozbuild.base.BuildEnvironmentNotFoundException: Could not find Mozilla source tree or build environment."

Comment 16

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 8413932 [details] [diff] [review]
Patch

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

Oh, well that was silly wasn't it?

Comment 18

[Marco Castelluccio [:marco]]

I've spun a try run, it fails only in the "Static Rooting Hazard Analysis" run: https://tbpl.mozilla.org/?tree=Try&rev=15a7a60774b6

The error is still the same ("mozbuild.base.BuildEnvironmentNotFoundException").

I've added a new catch to ignore this exception, now the try run is green: https://tbpl.mozilla.org/?tree=Try&rev=a257740a1778

Comment 19

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 20

[(not currently active) Ted Mielczarek]

Comment on attachment 8415855 [details] [diff] [review]
Patch

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

::: testing/mozbase/mozinfo/mozinfo/mozinfo.py
@@ +154,5 @@
>              update(json_path)
>              return json_path
>      except ImportError:
>          pass
> +    except BuildEnvironmentNotFoundException:

I'm a little worried that we'll paper over real errors with this, but I guess we can deal with this.

Comment 21

[(not currently active) Ted Mielczarek]

sfink: do you know why we'd hit this error on the rooting hazard analysis runs?

Comment 22

[Steve Fink [:sfink] [:s:]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #21)
> sfink: do you know why we'd hit this error on the rooting hazard analysis
> runs?

It uses mach to build. It's the only build job that uses mach.

I haven't really looked at this bug, but there was some problem with mach mentioned in comment 11.

Comment 23

[Steve Fink [:sfink] [:s:]]

Note that the hazard build is currently restricted to linux64 only. Though I hope to be adding a b2g variant before too long.

Comment 24

[Marco Castelluccio [:marco]]

Ok, this is still not correct on build machines.

For example, there were three tests that were supposed to run respectively on Windows XP, Windows 7 and Windows 8. For some reason, just the Windows 7 test was "built" and it was run on the Windows 7 machine and skipped on Windows XP and Windows 8.

Here's the Windows XP log: https://tbpl.mozilla.org/php/getParsedLog.php?id=38845488&tree=Try&full=1
You can see "test_win_61.html" was skipped. "test_win_52.html" should've run.

Any idea about what's happening? Is it possible that we're generating the list of the tests on a machine and then handing it to the other machines?

Comment 25

[(not currently active) Ted Mielczarek]

So yes, we only do one "windows" build, and then run that on the other OS versions. You're running into bug 989583.

Comment 26

[Steve Fink [:sfink] [:s:]]

(I'm not the one to ask. I mostly only know about the hazard builds. I was going to redirect to ted, but you already asked him and he already replied.)

Comment 27

[Marco Castelluccio [:marco]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #25)
> So yes, we only do one "windows" build, and then run that on the other OS
> versions. You're running into bug 989583.

Ok, so let's land this with the understanding that we need to fix bug 989583 to make the feature actually usable on build machines.

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e09e3bce7c0d

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e09e3bce7c0d

Comment 30

[Henri Sivonen (:hsivonen)]

I this patch known to work on Ubuntu 14.04? I get

0:39.31 creating ./config.status
 0:39.73 Traceback (most recent call last):
 0:39.73   File "./config.status", line 978, in <module>
 0:39.73     config_status(**args)
 0:39.73   File "/opt/Projects/mozilla-html5/python/mozbuild/mozbuild/config_status.py", line 134, in config_status
 0:39.74     emitter = TreeMetadataEmitter(env)
 0:39.74   File "/opt/Projects/mozilla-html5/python/mozbuild/mozbuild/frontend/emitter.py", line 74, in __init__
 0:39.74     mozinfo.find_and_update_from_json(config.topobjdir)
 0:39.74 AttributeError: 'module' object has no attribute 'find_and_update_from_json'
 0:39.76 *** Fix above errors and then restart with\
 0:39.76                "/usr/bin/make -f client.mk build"
 0:39.76 make[2]: *** [configure] Error 1
 0:39.76 make[1]: *** [/opt/Projects/mozilla-html5/../obj-debug/Makefile] Error 2
 0:39.76 make: *** [build] Error 2
 0:39.79 0 compiler warnings present.

...even after removing the objdir and trying again.

Comment 31

[Henri Sivonen (:hsivonen)]

Undoing this patch allows me to build again.

Comment 32

[(not currently active) Ted Mielczarek]

Do you have an older version of mozinfo installed globally? That method definitely exists in the in-tree mozinfo:
http://hg.mozilla.org/mozilla-central/annotate/4c4dcf7533cb/testing/mozbase/mozinfo/mozinfo/mozinfo.py#l137

It's possible there's some Python include path ordering problem since this is still during the configure step.

Comment 33

[Henri Sivonen (:hsivonen)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #32)
> Do you have an older version of mozinfo installed globally?

find / | grep mozinfo.py
finds files only inside Mozilla repos and objdirs and I've never knowingly installed mozinfo globally. (I wouldn't even know how.)

Comment 34

[Hubert Figuiere [:hub]]

It breaks here on Fedora 20 as well.

Comment 35

[Hubert Figuiere [:hub]]

Yes I have mozinfo installed globally, thanks pip and other Mozilla tools. *sigh*

And according to pip it is the latest version available.

Comment 36

[Hubert Figuiere [:hub]]

Filed bug 1008648 for that.

Comment 37

[Andrew Halberstadt [:ahal]]

I've heard several people complaining about this. Ted, what's the impact of backing this out while we figure out what's going on here?

Comment 38

[Andrew Halberstadt [:ahal]]

Nvm, we figured out what was going on in https://bugzilla.mozilla.org/show_bug.cgi?id=1008648#c6